Skip to content

Commit

Permalink
Raise an error if data could be lost from singularizing a list
Browse files Browse the repository at this point in the history
When a property is set to `multiple: false` and we find more than one
statement for the predicate, then raise an
ActiveFedora::ConstraintError.

This should help prevent users from losing data that is not written using
ActiveFedora.
  • Loading branch information
jcoyne committed Dec 17, 2014
1 parent 1a8afda commit ae6b480
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/active_fedora/attributes/property_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ def #{name}=(value)
def self.define_singular_readers(mixin, name)
mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1
def #{name}(*args)
get_values(:#{name}).first
vals = get_values(:#{name})
raise ActiveFedora::ConstraintError, "Expected \\"#{name}\\" to have 0-1 statements, but there are \#{vals.size}" if vals.size > 1
vals.first
end
CODE
end
Expand Down
14 changes: 14 additions & 0 deletions lib/active_fedora/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,20 @@ class ReadOnlyRecord < ActiveFedoraError
class IllegalOperation < ActiveFedoraError
end

# Raised when the data has more than one statement for a predicate, but our constraints say it's singular
# This helps to prevent overwriting multiple values with a single value when round tripping:
# class Book < ActiveFedora::Base
# predicate :title, predicate: RDF::DC.title, multiple: false
# end
#
# b = Book.new
# b.resource.title = ['foo', 'bar']
# b.title # Raises ConstraintError
# # which prevents us from doing:
# b.title = b.title
class ConstraintError < ActiveFedoraError
end

# Used to rollback a transaction in a deliberate way without raising an exception.
# Transactions are currently incomplete
class Rollback < ActiveFedoraError
Expand Down
11 changes: 11 additions & 0 deletions spec/unit/attributes_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,17 @@ class BarHistory4 < ActiveFedora::Base
expect(subject[:abstract]).to be_nil
end

context "when there are two assertions for the predicate" do
before do
subject.resource[:abstract] = ['foo', 'bar']
end
it "should raise an error if just returning the first value would cause data loss" do
expect { subject[:abstract] }.to raise_error ActiveFedora::ConstraintError, "Expected \"abstract\" to have 0-1 statements, but there are 2"
end
end
end

context "multiple values" do
it "should return values" do
expect(subject[:title]).to eq ['test1']
end
Expand Down

0 comments on commit ae6b480

Please sign in to comment.