perfact / zodbsync

Dump and restore objects between a ZODB and a serialized file system structure
GNU General Public License v2.0
12 stars 5 forks source link

Simultaneous removal and addition of properties/objects with the same name in the same folder #104

Closed Wiseqube closed 2 years ago

Wiseqube commented 2 years ago

Describe the bug Simultaneous adding/removing of a Property or Object with the same name in the same folder leads to an error on playback!

To Reproduce Steps to reproduce the behavior:

  1. Create a folder test with a subfolder test2 and their appropriate meta-data
  2. Using zodbsync playback playback the folder to the ZODB
  3. In the filesystem remove the subfolder test2 and instead add a property test2 to the folder test
  4. Try zodbsync playback on the folder test which will raise an error 'Invalid or duplicate property id'

Expected behavior The folder test2 is correctly removed and the property test2 correctly added to the folder test

Additional context This is not a big problem when implementing and picking single commits removing and adding the property, but becomes problematic when directly using zodbsync playback or zodbsync reset! The current workaround was to manually remove the object or property in ZODB and then executing zodbsync playback!

viktordick commented 2 years ago

I guess we need to extend the usage of on_return, which is called "on the way up" out of the recursion, after all the children have been handled. Currently, it only handles the reordering of children, but I guess to address this, we need to remove/change properties on the way down and add properties on the way up.

Maybe the current implementation with constructing a partial should be reconsidered in that case, it might be too much overhead. Currently, on_return is only used sparsely, for ordered folders. With this change, it would be the default behavior of everything that can have children.

viktordick commented 2 years ago

It seems to me that this is harder to fix than originally thought.

At the point where we play back one object out of a list of paths, we do not necessarily know if other objects below it are also going to be played back. So we can not at that point start by removing any superfluous children, then adjust the properties as needed, then add any new children. We need to also split the removing and adding of properties, removing them before continuing with paths below and adding them after we return from all paths below.

viktordick commented 2 years ago

Taking a step back.

For the non-recursive case, we have a sorted list of paths for which we expect changes. We should iterate through this list in reverse order to first determine which objects to remove. Then we need to iterate it in regular order to add and remove properties and add objects.

If we are in recursive mode, we also have a sorted list of non-redundant paths, i.e. no path is a subpath of another path. For each object, we need to remove any superfluous children before updating the object itself and returning the list of subpaths that are to be added to the list.

Maybe this will also allow to remove the part with folder_exists and obj_exists, which always was a bit of a hack.