...
- A mock of BrowseCreateDAOOracle has been done due to an incompatibility between H2 and the "upper" function call. This will affect tests related to case sensitivity in indexes.
- Many objects (like SupervisedItem) lack a proper definition of the "equals" method, which makes comparison between objects and unit testing harder
- Update method of many objects doesn't provide any feedback, we can only test if it raises an exception or not, but we can't be 100% sure if it workedÂ
- Many objects have methods to change the policies related to the object or children objects (like Item), it would be good to have some methods to retrieve these policies also in the same object (code coherence)
- There are some inconsistencies in the calls to certain methods. For example getName returns an empty String in a Collection where the name is not set, but a null in an Item without name
- DCDate: the tests raise many errors. I can't be sure if it's due to misunderstanding of the purpose of the methods or due to faulty implementation (probably the previous). In some cases extra encapsulation of the internals of the class would be advisable, to hide the complexities of the Calendars (months starting by 0, etc)
- The Authorization system gets a bit confusing. We have AuthorizationManager, AuthorizationUtils, methods that raise exceptions and methods that return booleans. Given the number of checks we have to do for permissions, and that some classes call methods that require extra permissions not declared or visible at first, this makes creation of tests (and usage of the API) a bit complex. I know we can ignore all permissions via context (turning on and off authorizations) but usually we don't want that
- Community: set logo checks for authorization, but set metadata doesn't. It's in purpose?
- Collection: methods create and delete don't check for authorization
- Item: there is no authorization check for changing policies, no need to be an administrator
- ItemIterator: it uses ArrayLists in the methods instead of List
- ItemIterator: we can't verify if the Iterator has been closed
- Metadata classes: usually most classes have a static method to create a new instance of the class. Instead, for the metadata classes (Schema, Field and Value) the method create is part of the object, thus requiring you to first create an instance via new and then calling create. This should be changed to follow the convention established in other objects (or the other objects should be amended to behave like the Metadata classes)
- Site: this class extends DSpaceObject. With it being a Singleton, it creates potential problems, for example when we use DSpaceObject methods to store details in the object. It's this relation necessary?
...