rdf4j icon indicating copy to clipboard operation
rdf4j copied to clipboard

NotifyingSailConnection should distinguish between explicit and inferred statements

Open kenwenzel opened this issue 1 year ago • 5 comments

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

kenwenzel avatar Aug 27 '24 09:08 kenwenzel

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).

hmottestad avatar Aug 27 '24 12:08 hmottestad

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.

kenwenzel avatar Aug 27 '24 14:08 kenwenzel

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.

hmottestad avatar Aug 27 '24 14:08 hmottestad

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.

kenwenzel avatar Aug 27 '24 14:08 kenwenzel

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?

hmottestad avatar Aug 28 '24 11:08 hmottestad