[Architecture] Harmonize copying a scene

In the source code I found three different implementations for copying a scene:

In UBDocumentTreeView::dropEvent() a scene is copied by making a clone, copying the dependencies (referenced objects, images, media) without changing the UUID and then saving the clone.
In UBPersistenceManager::copyDocumentScene() the UBForeighnObjectsHandler is used to somehow adjust the copied scene. I still do not really know what this handler does and why. Note: This function is invoked in UBDocumentTreeModel::dropMimeData. But according to #193 that function is unreachable.
In UBPersistenceManager::duplicateDocumentScene first just the SVG file is copied with a new UUID, and then the copied scene is loaded, all dependencies are copied and the UUIDs exchanged. This function is invoked with duplicate actions in Document and Board mode.

Note: Copying the objects in the first and third implementation was introduced with commit 38032fb42dd3d352f66f37dd76ecbedab2a43565 in April 2019 to fix an issue where images could get lost.

Depending on the method, the UUID of copied objects is changed or not. We have also seen, that sceneDeepCopy() fails to update the data property related to z level.

I propose

Analyze all these different implementations and find out what they do differently.
Find out what the UBForeighnObjectsHandler (mind the typo!) actually does. The usage in UBPersistenceManager::copyDocumentScene() is the only reference to this class in the project.
Is copying a referenced object necessary when duplicating a scene? This would only be required when we would also have a "move page" function which transfers a scene with all its objects to another document. Currently, such a function does not exist.
In the end find a single implementation suitable for copying within the same document and to another document and implement it as function of UBDocument.

Copy file or scene?

Is it better to handle the copy at the file level and adjust the SVG then, e.g. changing the UUID?
Or is it better to handle it at the scene level, where we can operate on items?

I think the answer very much depends on how deep we have to go into the details while copying. But finding the referenced objects already needs a deep understanding of the file and so I tend to a scene level approach.

As far as I can see the UBForeighnObjectsHandler tries to stay at the file level for the price, that it implements another SVG parser besides the UBSvgSubsetAdaptor. This is very dangerous when doing updates to the file format which would also affect the foreign objects.

See the note above: as analyzed in #193 this function is currently unreachable and not used.

UBForeighnObjectsHandler

This class and associated helper classes are created to some supporting stuff when dealing with documents. When you look at the functions it has there are only a few:

UBForeighnObjectsHandler::cure() with two signatures to cure a single document or a list of documents. This function is somehow similar to what I already tried to remove unreferenced files from a document. These functions are unused.
UBForeighnObjectsHandler::copyPage(). See above.

copyPage

This function, which is most relevant for us, is first forwarded two times: first to a UBForeighnObjectsHandlerPrivate and then further to a PageCopier: here the function has three steps:

Read the SVG into a DOM document.
Call cureIdsFromSvgDom() on the DOM.
Write the DOM to the target SVG file.

So what does cureIdsFromSvgDom() do?

It loops over the elements of the SVG document and calls cureFromText() on each of them.
cureFromText() again checks the element for one of
- video, audio or image
- foreignObject
- teacherGuide
The last seems to be a remainder of former times.
For each type a specific handling is used, which in the end however calls cureNCopy() for all of them to copy the referenced media file and assign a new UUID. This new UUID is then used in the DOM element to reference the copied media.

Si in the end the result is quite similar to UBPersistenceManager::duplicateDocumentScene(), however with different implementations:

The UBForeighnObjectsHandler stays at the SVG DOM level and does the adaptations there.
The UBPersistenceManager uses the scene level instead and does not need to know anything about SVG.

I tend to use the scene based approach, because then the UBSvgSubsetAdaptor is the only class which needs to know about SVG. This would mean:

Keep the UBPersistenceManager::duplicateDocumentScene() as the only function.
Extend it so that it can create the copy in another document.
Create a function in UBDocument which calls the persistence manager.
Use this function from the other places.
Remove the UBForeighnObjectsHandler.

Together with this I would also like to ask when it is necessary to change the UUID of the copy. I do not see a good reason for this. If I copy a file, why should it not have the same UUID as before? It is still the same file contents!

Change UUID of objects when copying?

There a PROs and CONs for changing the UUID when copying a scene.

PRO changing UUID, CON changing UUID

A new UUID for objects established a 1:1 relationship between scene and object file. This is easier to handle.
When deleting a scene you can delete all referenced object files. Note: This is currently not done, but probably should.
This avoids dangling object files and the necessity for cleanup of documents.
When multiple scenes reference the same object file, it cannot easily be deleted when the scene is deleted. Some kind of object management would be required which overlooks all scenes in a document.

CON changing UUID, PRO keeping UUID
The object file is the same even after copying to another document. Keeping the UUID would reflect this.
When a scene is copied within the same document, the object file does not need to be duplicated.

letsfindaway / OpenBoard