Wrong annotations on the ServiceProvider class about lyo.core HOT 10 CLOSED

cc @jamsden @jadelkhoury

from lyo.core.

Interesting. I guess marshalling of a ServiceProvider is done quite often, but no one has yet encoutered this problem before, since it is not often one needs to unmarshall a ServiceProvider.

Have a fix in mind? It should be easy to find and fix.

from lyo.core.

I will investigate. This is really strange b/c the getCreated returns a single Date anyway. I suspect the about setter is not called properly and it creates a subsequent mess in the persisted graph.

from lyo.core.

Got it. The error is triggered by me trying to add the same SP to a Set multiple times:

@Override
public void addServiceProvider(final ServiceProvider sp) {
    final Collection<ServiceProvider> serviceProviders = getServiceProviders();
    serviceProviders.add(sp);
    try {
        store.putResources(twinsGraphURI, serviceProviders);
    } catch (StoreAccessException e) {
        throw new IllegalStateException("Unable to persist the new SP in the KB");
    }
}

I was expecting the Set implementation to catch that but the problem is that it relies on equals and hashCode implementations. This brings us back to an old question. I wanted for a long time to define the equals and hashCode just in terms of the about URI. I think both @jadelkhoury and @jamsden were uneasy about ignoring all other properties.

I think it makes sense to expect the check

if(serviceProviders.contains(sp)) {
    
}

to return true if a Set already contains a resource with the same URI. If we don't agree to add such equality implementations, I would have to reimplement my Set as a Map<URI, SP> and then do a check on the about URIs. I am not sure if that's the better way or just a hack. The Map would also make it harder to work with the collections returned by Lyo Store.

P.S. Apologies for a duplicate.

from lyo.core.

Just to add some material for thinking: formally speaking, the equals method has to satisfy the equivalence relations described here: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/Object.html#equals(java.lang.Object)

The one I am proposing will satisfy all the rules, it will just not be as discriminating as possible.

from lyo.core.

An about URI in this case is a perfect example of a Natural identifier as explained here: https://vladmihalcea.com/how-to-implement-equals-and-hashcode-using-the-jpa-entity-identifier/

from lyo.core.

@jadelkhoury @jamsden @yashkhatri what's your opinion?

from lyo.core.

I think it makes sense to define equal in terms of the about URI.

from lyo.core.

I definitely agree, AbstractResource should implements equals and hashcode based on the about URI. Sometimes I had have to implement my own Comparator in order to keep unique resources into a Set... I suggest to modify Link class also to implement equals and hashcode in terms of the value property.

from lyo.core.

Nothing wrong with the annotations on the ServiceProvider class, closing.

Equality checking will be handled in #82.

from lyo.core.