PKP Bugzilla – Full Text Bug Listing
|Summary:||OMP clean-up: clean up file and file DAO class hierarchy.|
|Component:||General||Assignee:||PKP Support <pkp-support>|
|Version Reported In:||Also Affects:|
|Bug Depends on:|
Description jerico 2010-11-03 17:10:00 PDT
There are some inconsistencies in the SubmissionFile class hierarchy. 1) MonographFile and ArtworkFile/MonographGalley inherit from SubmissionFile but their DAOs are separate from each other. I think it would be correct to have their DAOs also inherit from each other. 2) ArtworkFile is a specialization of Monograph file and therefore the database table artwork_monograph_file should have a 1:1 relation with monograph_file which is not the case, though. IMO an entry into artwork_monograph_file should also result in an entry into monograph_file. The DAO is not implemented like that, though. And uploading an artwork file does not seem to populate the fields required for a monograph file. IMO the artwork_id column should be removed and file_id should become the primary of artwork_monograph_file. Then ArtworkDAO should use functionality from MonographDAO to populate fields of it's superclasses (see #1 above). 3) Implementing a new table for every book file type category is not very scalable. We should be very sure that we only have to book file type categories (artwork and document) otherwise we'll soon have to implement further tables if we get files with different meta-data requirements. If that happens we better switch to an implementation that uses the _settings table for additional/differing meta-data.
Comment 1 Matthew Crider 2010-11-19 17:37:01 PST
4) Library Files are unecessary -- they can be subsumed by the existing monograph file infrastructure.
Comment 2 jerico 2010-11-19 18:43:50 PST
Matt, have you seen the additional comments I made about the file class hierarchy in our UML diagram? It's all just first ideas.
Comment 3 jerico 2010-11-21 20:36:12 PST
Refactoring and clean-up in monograph file handler: https://github.com/pkp/omp/commit/2645eeb17fa22d7f7e233b2fa0ca8d955273471c
Comment 4 jerico 2010-11-21 23:24:43 PST
Renaming monograph file type and publication format type IDs to conform with our database nomenclature, further refactored/simplified monograph file manager, moved file path generation for monographs from the monograph file manager to the monograph class to remove code duplication and inconsistency, coding style improvements, rename get/setMonographType() to ...Id() and change its column type from varchar to int, remove superfluous indexes that duplicate the primary key, see: https://github.com/pkp/omp/commit/ae8b0337aab1ef37140db5f632a43325a717c798 https://github.com/pkp/pkp-lib/commit/8c9daf181fd4c68fe3faf0f1dd686d47a4100fca
Comment 5 jerico 2010-11-27 11:53:25 PST
We should also abstract file access away from the actual file system implementation. This will allow us to base file access on any document repository we like, e.g. a system like Fedora or DuraSpace, a document management system, WebDav folders, etc.
Comment 6 jerico 2010-12-05 14:47:07 PST
Here comes the big file DAO refactoring: https://github.com/pkp/omp/commit/c4b51ea6646ae0183c66bc3d2bd67a9b7430fbec https://github.com/pkp/pkp-lib/commit/626eeb1803deffc647c70fc7d7d55d832afa3c16
Comment 7 jerico 2010-12-08 08:54:58 PST
Submitted improvements proposed by Alec in his review (thanks Alec!): https://github.com/pkp/omp/commit/f635fb0fb4a9cf8b4e8f06e8170032f2004d7e25 https://github.com/pkp/pkp-lib/commit/1151eb5ff62d91456c747f5c83a935f79d928ec0
Comment 8 jerico 2010-12-08 09:19:34 PST
And one more change (multiple delete statements rather than bulk delete): https://github.com/pkp/pkp-lib/commit/abb7624eb4b4cf619733e530ad312d649ab9c408 https://github.com/pkp/omp/commit/286a2b8faba7f5f808f47dc789957728dc3a8d5d
Comment 9 Alec Smecher 2011-01-18 10:36:39 PST
See https://github.com/pkp/ojs/commit/16f21895b0811f50c460bb742daed0f69316d87d#commitcomment-244078 -- looks like a typo to me.
Comment 10 jerico 2011-01-23 05:53:58 PST
Fixed typo from comment #9, see https://github.com/pkp/ojs/commit/e7fbbd03f714cb75d214840daad7b55a0d60983f
Comment 11 Alec Smecher 2011-09-06 14:38:51 PDT
Major overhaul items complete and the current structure is now baked in. Closing.