Write layer-source of object into Data.FS

[Wiseqube]

Well, does what is says on the tin (atleast proof of concept wise)! Basically I had the idea that the Data.FS should just know from which layer an object was uploaded so we can make use of this information inside of the application (for example the ZMI). For this we just store the layer-ident in the zodbsync_layer property of an object (not the PropertySheet functionality, but just actual python properties), which are just as happily written into the Data.FS!

Whats currently missing is the "update" of this property in case the file is changed in Zope and therefor changes it`s source-layer! But I wanted to wait to implement something for this until I got feedback on whether this is actually something we would like to pursue further.

For testing purposes I actually patched the ZMI to display the layer-ident, which is really working like a charm:

diff --git a/src/OFS/zpt/main.zpt b/src/OFS/zpt/main.zpt
index e3d4c814b..83543f89a 100644
--- a/src/OFS/zpt/main.zpt
+++ b/src/OFS/zpt/main.zpt
@@ -91,6 +91,9 @@
                   <span class="zmi-object-title hidden-xs" tal:condition="ob/title|nothing">
                     &nbsp;(<span tal:replace="ob/title"></span>)
                   </span>
+                  <span class="zmi-object-title hidden-xs" tal:condition="ob/zodbsync_layer|nothing">
+                    &nbsp;(<span tal:replace="ob/zodbsync_layer"></span>)
+                  </span>
                 </a>
               </td>
               <td class="text-right zmi-object-size hidden-xs" tal:content="python:here.compute_size(ob)">

[jan-jockusch]

As long as the Python property zodbsync_layer is managed completely by this tool, i.e. if updates to the property are also cleanly managed in one place, I believe this is a sound approach to communicating this important information to the user.

Since the ZMI patch just does nothing on objects which do not have the property, it's also okay to just to incorporate a patch in perfact-dbutils-zope4.

So, I believe we should use this idea, unless @viktordick or @gandie have objections…

[viktordick]

No objections per se, though the mentioned missing update might be a bit difficult (this operation usually writes from the Data.FS into the file system, and would now require a response being written back) and without that this feature might sometimes give erroneous information. And implementing this would also break the assumption that a zodbsync record / is a one-way operation (though since this is not part of the "recorded" properties, this will probably be OK).

[Wiseqube]

@viktordick I've added a "writing"-back of the zodbsync_layer to the Data.FS and a Test to prove it working correctly and clearing back to the custom layer if the file is there! But I'm not entirely sure about my latest commit. Somehow the usage of a Transaction to writeback the zodbsync_layer has broken the test test_layer_remove_subfolder, although I'm not certain why it was working in the first place or why other tests are not showing the same behaviour although they are designed similarly!

[Wiseqube]

@viktordick sorry, I think my test was just poorly written so the transaction usage in the record was totally not necessary and therfor the fix on the test was also not required. Basically ignore the whole part of the last message about this topic

[viktordick]

I'm pretty sure this will have no effect on most record runs since they don't commit their transaction. In particular, the watch command explicitly aborts its transaction (https://github.com/perfact/zodbsync/blob/main/perfact/zodbsync/commands/watch.py#L357) since it is used to only make sure a consistent state is viewed and it does not expect that anything is changed in the Data.FS. And a regular record run uses an implicit transaction that is neither commited nor aborted, which I am pretty sure means an implicit abort.