Mailinglist Archive: opensuse-buildservice (273 mails)

< Previous Next >
[opensuse-buildservice] Can't delete two projects that a project lists as path elements (w/ fix)
A project lists multiple projects a path elements:

<project name="depends_on_many">
...
<repository name="SLE_10">
<path repository="SLE_10" project="dependency_foo"/>
<path repository="SLE_10" project="dependency_bar"/>
<arch>x86_64</arch>
</repository>
</project>

Now, if I delete dependency_foo, depends_on_many is updated with

<project name="depends_on_many">
...
<repository name="SLE_10">
<path repository="standard" project="deleted"/>
<path repository="SLE_10" project="dependency_bar"/>
<arch>x86_64</arch>
</repository>
</project>

Now, if I delete dependency_bar, the update of depends_on_many is trying
to do

<project name="depends_on_many">
...
<repository name="SLE_10">
<path repository="standard" project="deleted"/>
<path repository="standard" project="deleted"/>
<arch>x86_64</arch>
</repository>
</project>

But that gives a DuplicateKey MySQL error since there are two
deleted/standard path elements then, so I got around this with:

Index: buildservice/src/frontend/app/controllers/source_controller.rb
===================================================================
--- buildservice/src/frontend/app/controllers/source_controller.rb
(revision 1668)
+++ buildservice/src/frontend/app/controllers/source_controller.rb
(working copy)
@@ -59,8 +59,20 @@
del_repo = DbProject.find_by_name("deleted").repositories[0]
lreps.each do |link_rep|
pe = link_rep.path_elements.find(:first, :include =>
["link"], :conditions => ["db_project_id = ?", pro.id])
- pe.link = del_repo
- pe.save
+ logger.debug "del_repo.id: #{del_repo.id}"
+ del_pe = link_rep.path_elements.find(:first, :include =>
["link"], :conditions => ["db_project_id = ?", del_repo.id])
+ logger.debug "del_pe: #{del_pe}"
+
+ # if a dep_repo link already exists, then we can't added
another
+ # one, so just destroy the pe instead of replacing it with
deleted
+ # project
+ if del_pe.nil?
+ pe.link = del_repo
+ pe.save
+ else
+ pe.destroy
+ end
+
#update backend
link_prj = link_rep.db_project
logger.info "updating project '#{link_prj.name}'"

So, if a deleted/standard is already a path_element, it just deletes the
path element instead of replacing it. This should have left
depends_on_many like this:

<project name="depends_on_many">
...
<repository name="SLE_10">
<path repository="standard" project="deleted"/>
<arch>x86_64</arch>
</repository>
</project>

This did not work though, the pe.destroy line caused another
DuplicateKey error, this time because 2 path_elements got the same
position during the destroy. I traced this to remove_from_list in
acts_as_list. The problem is the positions of the lower items are
updated before the item being deleted has it's position set to nil. It
appears that acts_as_list is written assuming position will not be a
unique key. I made this change to get around that position is part of a
multicolumn unique key for path_elements:

Index:
buildservice/src/frontend/vendor/plugins/acts_as_list/lib/active_record/acts/list.rb
===================================================================
---
buildservice/src/frontend/vendor/plugins/acts_as_list/lib/active_record/acts/list.rb
(revision 1659)
+++
buildservice/src/frontend/vendor/plugins/acts_as_list/lib/active_record/acts/list.rb
(working copy)
@@ -104,7 +104,7 @@
def move_to_bottom
return unless in_list?
acts_as_list_class.transaction do
- decrement_positions_on_lower_items
+
decrement_positions_on_lower_items(self.send(position_column).to_i)
assume_bottom_position
end
end
@@ -122,8 +122,12 @@
# Removes the item from the list.
def remove_from_list
if in_list?
- decrement_positions_on_lower_items
+ # Set self's position to nil first so duplicate key doesn't
happen
+ # when item directly below self is updated to have same
position
+ # as self.
+ old_position = self.send(position_column).to_i
update_attribute position_column, nil
+ decrement_positions_on_lower_items(old_position)
end
end

@@ -216,10 +220,9 @@
end

# This has the effect of moving all the lower items up one.
- def decrement_positions_on_lower_items
- return unless in_list?
+ def decrement_positions_on_lower_items(position)
acts_as_list_class.update_all(
- "#{position_column} = (#{position_column} - 1)",
"#{scope_condition} AND #{position_column} >
#{send(position_column).to_i}"
+ "#{position_column} = (#{position_column} - 1)",
"#{scope_condition} AND #{position_column} > #{position}"
)
end

Now, dependency_bar deletes cleanly. The attached file has both
patches.

I've sent in patches to this mailing list before and I never know if
they get accepted. Can I get some feedback if this is going to be
accepted or not or should I be submitting these somewhere else?
Index: buildservice/src/frontend/app/controllers/source_controller.rb
===================================================================
--- buildservice/src/frontend/app/controllers/source_controller.rb
(revision 1668)
+++ buildservice/src/frontend/app/controllers/source_controller.rb
(working copy)
@@ -59,8 +59,20 @@
del_repo = DbProject.find_by_name("deleted").repositories[0]
lreps.each do |link_rep|
pe = link_rep.path_elements.find(:first, :include => ["link"],
:conditions => ["db_project_id = ?", pro.id])
- pe.link = del_repo
- pe.save
+ logger.debug "del_repo.id: #{del_repo.id}"
+ del_pe = link_rep.path_elements.find(:first, :include => ["link"],
:conditions => ["db_project_id = ?", del_repo.id])
+ logger.debug "del_pe: #{del_pe}"
+
+ # if a dep_repo link already exists, then we can't added another
+ # one, so just destroy the pe instead of replacing it with deleted
+ # project
+ if del_pe.nil?
+ pe.link = del_repo
+ pe.save
+ else
+ pe.destroy
+ end
+
#update backend
link_prj = link_rep.db_project
logger.info "updating project '#{link_prj.name}'"
Index:
buildservice/src/frontend/vendor/plugins/acts_as_list/lib/active_record/acts/list.rb
===================================================================
---
buildservice/src/frontend/vendor/plugins/acts_as_list/lib/active_record/acts/list.rb
(revision 1659)
+++
buildservice/src/frontend/vendor/plugins/acts_as_list/lib/active_record/acts/list.rb
(working copy)
@@ -104,7 +104,7 @@
def move_to_bottom
return unless in_list?
acts_as_list_class.transaction do
- decrement_positions_on_lower_items
+ decrement_positions_on_lower_items(self.send(position_column).to_i)
assume_bottom_position
end
end
@@ -122,8 +122,12 @@
# Removes the item from the list.
def remove_from_list
if in_list?
- decrement_positions_on_lower_items
+ # Set self's position to nil first so duplicate key doesn't happen
+ # when item directly below self is updated to have same position
+ # as self.
+ old_position = self.send(position_column).to_i
update_attribute position_column, nil
+ decrement_positions_on_lower_items(old_position)
end
end

@@ -216,10 +220,9 @@
end

# This has the effect of moving all the lower items up one.
- def decrement_positions_on_lower_items
- return unless in_list?
+ def decrement_positions_on_lower_items(position)
acts_as_list_class.update_all(
- "#{position_column} = (#{position_column} - 1)",
"#{scope_condition} AND #{position_column} > #{send(position_column).to_i}"
+ "#{position_column} = (#{position_column} - 1)",
"#{scope_condition} AND #{position_column} > #{position}"
)
end

< Previous Next >
This Thread
  • No further messages