Skip to content

Conversation

@no-reply
Copy link
Member

@no-reply no-reply commented Jan 3, 2016

For initial review. This gets changesets and transactions in a state that I think they are usable.

Before these changes, Transaction fails regularly with no method errors on nil #changes. Instead of "talking to" a nil on changes we introduce Changeset#empty? and give each transaction a changeset on initialization.

This also implements Transaction#execute, pushing responsibility for transactionality down to the Repository, via Changeset#apply. Enumerable#supports?(:transactions) is introduced; and changesets can now be applied to any Mutable. Transaction is still a Repository only concept.

@bendiken, @gkellogg this could use an initial review. In the meanwhile, I'm picking up work on transactionality tests & support for the default repository.

@no-reply
Copy link
Member Author

no-reply commented Jan 3, 2016

Note that class level documentation needs a rework before this is merged.

@no-reply
Copy link
Member Author

no-reply commented Jan 4, 2016

A dumb transactional repository implementation lives at https://github.com/ruby-rdf/rdf/tree/feature/repository-transactions

I think some tests should be put in place before something with better memory efficiency happens.

@gkellogg
Copy link
Member

gkellogg commented Jan 4, 2016

Looks good to me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Changeset is solid, now.

Note that #apply_changeset is a method on Mutable, so Changesets can be applied more generally than just to repositories.

artob and others added 9 commits January 23, 2016 12:46
Not every `Transaction` needs to keep an up-to-date `Changeset` in
buffer. This change documents that and shifts tests that were in
`rdf-spec` to `RDF::Transaction`s default implementation tests.

Some clarification of the looseness of ACID guarantees and the minimal
requirements for transaction support are added, as well.
Defines transaction rollback requirements and implements them for the
base class.

Adds support for passing a isolated snapshot of the repository. The
default is to use an unisolated repository for query; but this supports
using any Queryable as a snapshot.

Further documentation for adding transaction support to a Repository
using the base class.
Implements non-mutating versions of `#insert_statement`,
`#delete_statement` and `#has_statement?` which can run over a
`Hamster::Hash` passed in, returning the new hash. Adds a custom
`#apply_changeset` for the default `Repository`, which captures the
current `@data` in a separate variable, updates the snapshot inline,
then replaces `@data`.

Refactors the default `#insert_statement` and `#delete_statement`
implementations to use the non-mutating versions.
@no-reply
Copy link
Member Author

I rebased this on the Hamster repository changes, implemented snapshots and atomic application of changesets, which I think completes the work.

I'm getting some RDF::Vocabulary test failures. These seem to be unrelated, but might be my fault due to merge conflict resolution. I'm looking into this.

In the meanwhile, feedback on this and ruby-rdf/rdf-spec#44 is welcome. Again, I understand these to be a complete implementation of the desired interface and behavior. If something is missing, that should be called out ASAP to keep the 2.0 release on track.

cc: @gkellogg @bendiken

@gkellogg
Copy link
Member

See ruby-rdf/rdf-spec#44 (comment). We should probably merge these PRs (modulo the #each implementation) and pick up the pieces.

no-reply pushed a commit that referenced this pull request Jan 28, 2016
@no-reply no-reply merged commit ac6ad27 into develop Jan 28, 2016
@no-reply no-reply deleted the transaction branch January 28, 2016 00:21
@no-reply no-reply mentioned this pull request Jan 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants