PKP Bugzilla – Bug 6127
OMP clean-up: clean up file and file DAO class hierarchy.
Last modified: 2011-09-06 14:38:51 PDT
We are moving to Git Issues for bug tracking in future releases. During transition, content will be in both tools. If you'd like to file a new bug, please create an issue.
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.
4) Library Files are unecessary -- they can be subsumed by the existing monograph file infrastructure.
Matt, have you seen the additional comments I made about the file class hierarchy in our UML diagram? It's all just first ideas.
Refactoring and clean-up in monograph file handler: https://github.com/pkp/omp/commit/2645eeb17fa22d7f7e233b2fa0ca8d955273471c
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:
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.
Here comes the big file DAO refactoring:
Submitted improvements proposed by Alec in his review (thanks Alec!):
And one more change (multiple delete statements rather than bulk delete):
See https://github.com/pkp/ojs/commit/16f21895b0811f50c460bb742daed0f69316d87d#commitcomment-244078 -- looks like a typo to me.
Fixed typo from comment #9, see https://github.com/pkp/ojs/commit/e7fbbd03f714cb75d214840daad7b55a0d60983f
Major overhaul items complete and the current structure is now baked in. Closing.