Gaffer
Gaffer copied to clipboard
gchq
Signature of methods in SchemaElementDefinition
In SchemaElementDefinition the signature of the method getGroupBy is to return Set<String>. The actual object that is returned is of type LinkedHashSet. Many callers of this method assume that the results are in a consistent order, e.g. when properties are serialised they need to always be serialised in the same order. As the result is a LinkedHashSet, the results are in a consistent order. But as the signature doesn't guarantee that, in theory the implementation of SchemaElementDefinition could change (to return a HashSet for example) and client code would break as the order would not be consistent.
There are similar issues with getIdentifierMap and getProperties.
EDIT This branch has attempted a workaround. @d21211122 says: For Gaffer 2.0, undo the workaround, deprecate these methods and replace them with ones that guarantee a consistent order and update all the callers of the methods in Gaffer to use the new ones.
We cannot update the signature as the methods are public, so new ordered methods have been created and the old ones deprecated, as mentioned above.
Need to investigate if this is still a problem, then decide what alpha it needs to go into (if any). If it is a breaking change add the label and ensure it's in an appropriate milestone.
The historic comments by @m55624 above (new ordered methods) relate to the branch gh-1804-SchemaElementDefinition-method-signatures, but this was never merged in and these changes have not been applied.
In Gaffer there are many instances of fields declared with the class as an interface from Java Collections and not an implementation. There are also many (200+) public methods returning these classes. Where order is not guaranteed for the Collection's implementation (Set or Map), but calling code relies on order, this problem could occur. However, for this to happen the actual order would need to be lost because of a code change.
In the case of SchemaElementDefinition, there is nothing to suggest that the implementation of groupBy is going to be changed to no longer be ordered. If this change were proposed then method signatures could be altered as part of that.
As the problem of method signatures using interface classes which do not guarantee order is not specific to SchemaElementDefinition and there is nothing to suggest it is a current problem with that class, I'd suggest this ticket be closed. If lack of clarity for ordering of collections in method signatures is considered to be a valid future improvement, then a new issue should be raised for this (such a ticket would include the changes suggested here).
I have added a PR (#2889) which should satisfy this ticket, rather than explicitly changing the types to be ordered, I have added tests that dictate ordered results
