search

arrowhead-f / arrowhead-kalix

The Arrowhead Kalix Libraries
https://arkalix.se
Eclipse Public License 2.0
2 stars 5 forks source link

Service Query throws UnsupportedOperationException #36

Closed awoSYS closed 3 years ago

awoSYS commented 3 years ago

I'm using the kalix lib v0.5, in which I found a bug, which I think still exists in the current lib version. I'm doing this:

final var consumer = system.consume().name(provSrvName).encodings(provEnc).transports(provTransp)
    .oneUsing(...

Where provEnc is of type EncodingDescriptor and provTransp is of type TransportDescriptor. This results in the following error:

java.lang.UnsupportedOperationException: remove
    at java.base/java.util.Iterator.remove(Iterator.java:102)
    at java.base/java.util.AbstractCollection.retainAll(AbstractCollection.java:420)
    at se.arkalix.query.ServiceQuery.updateAndValidateUsing(ServiceQuery.java:296)
    at se.arkalix.query.ServiceQuery.oneUsing(ServiceQuery.java:256)

As far as I understand it, this is a result of trying to do retainAll() on a List that's created by the ServiceQuery#encodings(final EncodingDescriptor...) method via this line: this.encodings = Arrays.asList(encodings);. The corresponding line in the current lib version seems to be this one. The List created that way doesn't seem to support retainAll() and remove(), see also this Stackoverflow post.

Hope this helps.

emanuelpalm commented 3 years ago

@awoSYS Well, this is a documentation problem more than it is a bug. I should have used List#of() rather than Arrays#asList(), because the former provides stronger immutability guarantees than the latter (the List<T> returned by Arrays#asList() allows in-place modification via #set(index, newValue)). Unless explicitly noted, all collections (List, Map, etc.) returned by Kalix methods are meant to always be immutable. That means that #retainAll() will never work on a list returned by a generated or normal Kalix method. You should rather use list.stream().filter(..).toList() if you want to filter a list.

Immutability is a way of reducing program complexity. If mutable references are never shared between objects, you never have to wonder what objects are able to modify them.

awoSYS commented 3 years ago

@emanuelpalm Yeah, good pointer to the advantages of immutability. However, in this case it's the ServiceQuery.oneUsing() method (via ServiceQuery#updateAndValidateUsing()) that tries to do retainAll() on the list that's supposed to be immutable, right?

emanuelpalm commented 3 years ago

@awoSYS I didn't look carefully enough at the stack trace. You are right. I will fix it.

emanuelpalm commented 3 years ago

This bug is fixed in version 0.5.3, which will be on Maven Central in 10 minutes or so. Please reopen the issue if the problem would remain in the new version.