Permalink
Browse files

Bug fix related to cleanup of indices / uniques.

For code doing virtual index / unique, there was
a bug where cleanup failed mainly because there
was no attribute to compare the index / unique
with.
  • Loading branch information...
1 parent ebc8432 commit 28f3813fd3c50267b93b66e643f0c8739f08b57d @cyx cyx committed May 25, 2012
Showing with 79 additions and 20 deletions.
  1. +45 −19 lib/ohm.rb
  2. +32 −0 test/indices.rb
  3. +2 −1 test/model.rb
View
@@ -1286,27 +1286,34 @@ def save!
def __save__
Transaction.new do |t|
t.watch(*_unique_keys)
- t.watch(key) if not new?
+
+ if not new?
+ t.watch(key)
+ t.watch(key[:_indices]) if model.indices.any?
+ t.watch(key[:_uniques]) if model.uniques.any?
+ end
t.before do
_initialize_id if new?
end
- existing = nil
+ _uniques = nil
uniques = nil
+ _indices = nil
indices = nil
t.read do
_verify_uniques
- existing = db.hgetall(key)
+ _uniques = db.hgetall(key[:_uniques])
+ _indices = db.smembers(key[:_indices])
uniques = _read_index_type(:uniques)
indices = _read_index_type(:indices)
end
t.write do
db.sadd(model.key[:all], id)
- _delete_uniques(existing)
- _delete_indices(existing)
+ _delete_uniques(_uniques)
+ _delete_indices(_indices)
_save
_save_indices(indices)
_save_uniques(uniques)
@@ -1324,13 +1331,23 @@ def __save__
#
def delete
transaction do |t|
- t.read do |store|
- store[:existing] = db.hgetall(key)
+ _uniques = nil
+ _indices = nil
+
+ t.watch(*_unique_keys)
+
+ t.watch(key)
+ t.watch(key[:_indices]) if model.indices.any?
+ t.watch(key[:_uniques]) if model.uniques.any?
+
+ t.read do
+ _uniques = db.hgetall(key[:_uniques])
+ _indices = db.smembers(key[:_indices])
end
- t.write do |store|
- _delete_uniques(store[:existing])
- _delete_indices(store[:existing])
+ t.write do
+ _delete_uniques(_uniques)
+ _delete_indices(_indices)
model.collections.each { |e| db.del(key[e]) }
db.srem(model.key[:all], id)
db.del(key[:counters])
@@ -1470,28 +1487,37 @@ def _read_index_type(type)
def _save_uniques(uniques)
uniques.each do |att, val|
- db.hset(model.key[:uniques][att], val, id)
+ unique = model.key[:uniques][att]
+
+ db.hset(unique, val, id)
+ db.hset(key[:_uniques], unique, val)
end
end
- def _delete_uniques(atts)
- model.uniques.each do |att|
- db.hdel(model.key[:uniques][att], atts[att.to_s])
+ def _delete_uniques(_uniques)
+ _uniques.each do |unique, val|
+ db.hdel(unique, val)
+ db.hdel(key[:_uniques], unique)
end
end
- def _delete_indices(atts)
- model.indices.each do |att|
- val = atts[att.to_s]
-
- db.srem(model.key[:indices][att][val], id)
+ def _delete_indices(_indices)
+ _indices.each do |index|
+ db.srem(index, id)
+ db.srem(key[:_indices], index)
end
+ # model.indices.each do |att|
+ # val = atts[att.to_s]
+
+ # db.srem(model.key[:indices][att][val], id)
+ # end
end
def _save_indices(indices)
indices.each do |att, val|
model.toindices(att, val).each do |index|
db.sadd(index, id)
+ db.sadd(key[:_indices], index)
end
end
end
View
@@ -95,3 +95,35 @@ def before_save
assert [@user1, @user2] == gmail.sort_by { |u| u.id }
assert [@user3] == User.find(:email_provider => "yahoo.com").to_a
end
+
+scope do
+ # Just to give more context around this bug, basically it happens
+ # when you define a virtual unique or index.
+ #
+ # Previously it was unable to cleanup the indices mainly because
+ # it relied on the attributes being set.
+ class Node < Ohm::Model
+ index :available
+ attribute :capacity
+
+ unique :available
+
+ def available
+ capacity.to_i <= 90
+ end
+ end
+
+ test "index bug" do
+ n = Node.create
+ n.update(capacity: 91)
+
+ assert_equal 0, Node.find(available: true).size
+ end
+
+ test "uniques bug" do
+ n = Node.create
+ n.update(capacity: 91)
+
+ assert_equal nil, Node.with(:available, true)
+ end
+end
View
@@ -347,7 +347,8 @@ class ::Foo < Ohm::Model
assert_equal [], Ohm.redis.keys("*")
Foo.create(:name => "Bar")
- expected = %w[Foo:1 Foo:all Foo:id Foo:indices:name:Bar]
+ expected = %w[Foo:1 Foo:1:_indices Foo:all Foo:id Foo:indices:name:Bar]
+
assert expected.sort == Ohm.redis.keys("*").sort
Foo[1].delete

0 comments on commit 28f3813

Please sign in to comment.