gchq / koryphe

A flexible library for writing functional operations in Java
Apache License 2.0
20 stars 15 forks source link

Investigate TupleSignature functionality #225

Closed t92549 closed 3 years ago

t92549 commented 3 years ago

While fixing up https://github.com/gchq/Gaffer/pull/2239, some strange functionality was found with the TupleSignature. When assessing if another type can be assigned to the Tuple type in the TupleSignature, it seems to look at the classes inside the Tuple, not the type itself. This meant that inside a test that should have been outputting a Tuple<String>, expecting a String worked but expecting a Tuple did not.

This is due to the types inside the Tuple being saved to later be checked against: https://github.com/gchq/koryphe/blob/5389edb0415650ddb2d05a9468c8f6e55443e2ca/core/src/main/java/uk/gov/gchq/koryphe/signature/TupleSignature.java#L47-L49 And the assignable function checking them: https://github.com/gchq/koryphe/blob/5389edb0415650ddb2d05a9468c8f6e55443e2ca/core/src/main/java/uk/gov/gchq/koryphe/signature/TupleSignature.java#L71-L73

Is it correct that it works like this? It means that Tuples like this one will be "assignable" from any Java type, due to using Tuple<?>: https://github.com/gchq/koryphe/blob/5389edb0415650ddb2d05a9468c8f6e55443e2ca/core/src/main/java/uk/gov/gchq/koryphe/impl/function/ToTuple.java#L39

m29827 commented 3 years ago

Hi @t92549 I think it's correct, as you've described signatures generated for uk.gov.gchq.koryphe.tuple.Tuple are a special case where the enclosing type is ignored and the declared Tuple types are tested instead. By definition the tuple is a ordered list of elements of known size so it makes sense to ensure we have the expected types in the correct order rather than checking the type of the Tuple itself which acts as a container. For a function returning a Tuple<?> then any java object would be acceptable, for Tuple then we should expect something assignable to a String.

t92549 commented 3 years ago

Okay, this functionality makes sense. I think what is confusing is that in Gaffer there are specific Tuple types created, such as ElementTuple. When these return types are tested for, they skip the TupleSignature, and instead check for an ElementTuple type. Whereas throughout koryphe, TupleSignatures are used. Perhaps this is okay though and I just misunderstood what TupleSignatures are for.

t92549 commented 3 years ago

I think I was just misunderstanding the point of the TupleSignature. It is good that it tests for the declared types, and it is also okay that Gaffer bypasses this by offering specific Tuple implementations like ElementTuple. With concrete implementations, they should then rightfully ignore the TupleSignature and instead test the output type for the implementation instead. Thanks a lot for your hep clarifying this @m29827