ruby-rdf / rdf

RDF.rb is a pure-Ruby library for working with Resource Description Framework (RDF) data.
http://rubygems.org/gems/rdf
The Unlicense
382 stars 92 forks source link

RDF::Changeset does not appear to accept patterns for deletes #403

Closed doriantaylor closed 3 years ago

doriantaylor commented 4 years ago

I am currently trying to port my RDF::KV protocol from Perl to Ruby, and have encountered a problem with RDF::Changeset. Namely, I feel I should be able to add a pattern to the delete side of a changeset, like the docs (eventually) say.

RDF::Changeset includes RDF::Mutable, which is where it gets #insert and #delete from. This is what the documentation for RDF::Mutable#delete says:

Deletes RDF statements from self. If any statement contains a Query::Variable, it is considered to be a pattern, and used to query self to find matching statements to delete.

Doing this, however, raises an exception.

This doesn't look like it would be too hard to patch. I can provide one if you have no objections.

gkellogg commented 4 years ago

Can you provide a test case? Really, variables should only be in an RDF::Query::Pattern, not an RDF::Statement, but one is a subclass of the other.

It may be that this is an area that is not generally exercised, and we'll need to consider test cases across things that implement RDF::Queryable and RDF::Mutable.

The specs for RDF::Mutable have one test "should support wildcard deletions" that should do this, though, but the ChangeSet spec doesn't run this. It's run through RDF::Repository, though.

gkellogg commented 4 years ago

Looking some more, a ChangeSet can't run the Mutable shared tests and really doesn't fully implement Mutable, so this may require some update in documentation, or figure out how to do away with including RDF::Mutable.

Is there some specific reason you need this? Or, is it just part of an overall port? Maybe there's some other way to attack this.

doriantaylor commented 4 years ago

It seems logical that a changeset should be able to have wildcards on the delete side (they of course don't make sense on the insert side). I implemented my own changeset class for the Perl version which accepts wildcards, but from what I read it looked like it wouldn't be necessary with RDF::Changeset.

So I'm looking at either adding useful functionality upstream or otherwise duplicating its functionality.

gkellogg commented 4 years ago

It doesn’t seem like there’s an issue with RDF::ChangeSet, itself, but with the support within the RDF::Repository implementation. Something else using ChangeSet would likely not be an issue.

Testing RDF::ChangeSet against the Mutable shares tests my require an intermediate mock to implement the surrounding tests, but I’ll look into this shortly.

doriantaylor commented 4 years ago

There is an issue in RDF::Repository::Implementation as well, insofar as it short-circuits the process, but I was going to bring that up separately. I only noticed it after I encountered and bandaided the crash scenario.

(That was just to implement this via pry in RDF::Changeset: def query val; [val]; end. Otherwise it crashes with a method missing error.)

I also noticed RDF::Repository will properly execute delete against the wildcarded statement.

https://github.com/ruby-rdf/rdf/compare/develop...doriantaylor:wildcard-changeset is my tentative proposal. I will circle back around with test cases.

gkellogg commented 3 years ago

@doriantaylor I'm going to put out a release soon, if you'd like to get something in.

doriantaylor commented 3 years ago

oof i don't remember what happened to this

doriantaylor commented 3 years ago

…looks like that patch got integrated?

gkellogg commented 3 years ago

Okay, great; wasn't sure if there was something else.