eclipse-rdf4j / rdf4j

Eclipse RDF4J: scalable RDF for Java
https://rdf4j.org/
BSD 3-Clause "New" or "Revised" License
363 stars 164 forks source link

NotifyingSailConnection should distinguish between explicit and inferred statements #5118

Open kenwenzel opened 2 months ago

kenwenzel commented 2 months ago

Problem description

When listening to changes on NotifyingSailConnection the consumer is unable to distinguish between explicit and inferred statements. In scenarios where undo/redo or audit logging should be implemented it would be very helpful if this distinction could be drawn.

Preferred solution

The interface SailConnectionListener could be extended to include an inferred flag as parameter for the notification methods. The default implementation of those methods could just delegate to the existing ones.

public interface SailConnectionListener {
  void statementAdded(Statement st);
  void statementRemoved(Statement st);

  default void statementAdded(Statement st, boolean inferred) {
    statementAdded(st);
  }

  default void statementRemoved(Statement st, boolean inferred) {
    statementRemoved(st);
  }
}

Are you interested in contributing a solution yourself?

None

Alternatives you've considered

The alternative would be to directly include the inferred flag within the statement interface. This is the approach that we have used for KOMMA: https://github.com/komma/komma/blob/master/bundles/core/net.enilink.komma.core/src/main/java/net/enilink/komma/core/IStatement.java

Anything else?

No response

hmottestad commented 2 months ago

Sounds good! We’ll have to be a bit careful that we don’t make any breaking changes.

I think the reason why Statements don’t have an inferred flag is that users can’t insert their own inferred statements. We could introduce an interface StatementWithInferredStatus (or something a bit cleaner) and then we can use instanceOf to check if the underlying store supports that feature (eg. produces statements that implement that interface).

kenwenzel commented 2 months ago

As a first step we could just extend the listener as sketched above. This would save users from false assumptions about the support of inferred states.

At a later point: Instead of StatementWithInferredStatus I would prefer to simply extend the Statement interface with isInferred() and add a default method that just returns false. Within KOMMA I have decided to not include the inferred flag for equals() and hashCode() but I am not sure if this is a good/correct decision.

hmottestad commented 2 months ago

To extend the listener we would have to add two new methods in order to keep it backwards compatible, and both of those methods need a default implementation on the interface side. Those implementations would probably just pass down to the original methods, but for users who implement the methods there would not be a guarantee that the sail actually calls the new methods.

kenwenzel commented 2 months ago

Yes, that is right. But I guess - despite of the SAILs already delivered with RDF4J - there is not much use of this interface. Hence it would be a minimal invasive change.

hmottestad commented 2 months ago

There are some databases that are built on the Sail interfaces, but I don't think this is going to be that big of a deal for those.

Why don't you make a PR and we can see how it looks and functions?