Move returns wrong object when the id already exists

[mauritsvanrees]

Seen on Plone 4.3.18 with plone.api 1.8.4, but I don't see anything in later releases that would fix this.

As example, you have three folders and two documents with the same id:

• folder1/doc
• folder2/doc
• folder3

You move both documents to the third folder:

• api.content.move(source=folder1.doc, target=folder3)
• api.content.move(source=folder2.doc, target=folder3)

The second call moves folder2/doc to folder3/copy_of_doc, but it returns folder3/doc, which is the original folder1/doc.

In other words: the move works, but you get the wrong object back.

This is with the default safe_id=False, so I thought this would raise an error. The documentation says about this keyword argument: "When False, the given id will be enforced. If the id is conflicting with another object in the target container, raise a InvalidParameterError. When True, choose a new, non-conflicting id."

The problem is that this safe_id parameter is only checked if you pass an explicit id parameter. I think it should also be used without this id.

So in the above case:

• api.content.move(source=folder2.doc, target=folder3, safe_id=False): fail with InvalidParameterError, because another item with id doc already exists in the target.
• api.content.move(source=folder2.doc, target=folder3, safe_id=True): rename the item to copy_of_doc (the OFS package does that for us), and return this item.

Does that sound good?

[fulv]

I just stumbled into this, as well. Not only does the move method never raise an InvalidParameterError and return the wrong object, but it also always renames the colliding object regardless of safe_id.

The reason for this is that it does the move in two steps:

1. cut source object to clipboard and paste to destination.
2. rename the pasted object to the specified id and delegate the raising of any exception to the rename method.

The problem is that the colliding object has already been renamed after step 1 by the manage_pasteObjects method.
Another problem is that the rename refers to target[source_id], which, in case of a collision, is the object that already existed in the destination, not the moved object. So the rename is essentially a no-op. (In fact, if you look at the destination container after the move, you'll see that both the original id and the copy_of_id objects now have the exact same modification datetime.)

Interestingly, unlike move, the copy method explicitly raises the exception, so I expect it would work as documented, although I have not tried.

[szakitibi]

The bug still exists with latest Plone 5.2.7and Plone 1.11.1.

I had a simple case, where custom reports had to be moved between report folders based on selected report type. Naturally choose plone.dexterity.insterfaces.IEditFinishedEvent and handler used api.content.move with safe_id=True.

Comment #434 (comment) explains the problem quite clear, regarding the wrong object return part. But additionally when the manage_pasteObjects is attempted here, there is an indexing error raised:

2022-03-25 14:06:50,586 ERROR   [Products.ZCatalog:98][waitress-2] A different document with value '954bfb356c8b4e5b8a11dae6c505d8c0' already exists in the index.

Which ends up in a broken catalog brain resulting:

• ValueError: The object with the id "copy_of_<id>" does not exist. when rendering plone.global_sections viewlet.
• KeyError: 'copy_of_<id>' for folder_contents view rename or remove attempts.

To fix the original issue api.content.move should handle paste similarly to copy - see content.py#L269-L273.

Regarding the broken catalog brain, I had to copy move inner parts and tweak it:

    # manually uncatalog source object before move
    brain = uuidToCatalogBrain(source.UID())
    source.portal_catalog.uncatalog_object(brain.getPath())
    copy_info = target.manage_pasteObjects(
        source.aq_parent.manage_cutObjects(source.id)
        )
    # update source id from copy info
    source_id = copy_info[0]['new_id']

Which still results on manage_pasteObject a catalog error:

2022-03-25 14:08:27,103 ERROR   [Zope.ZCatalog:405][waitress-1] uncatalogObject unsuccessfully attempted to uncatalog an object with a uid of /Plone/test. 

But this one is at least not harmful, and there is no broken brain in portal catalog.