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

Checking differences between file system and zope objects not strict on 'True' and '1' #84

Closed tjastrzembski closed 2 years ago

tjastrzembski commented 3 years ago

Describe the bug I run into the issue that zodbsync playback does not update zope objects, where booleans changed from True to 1 and vice versa.

To Reproduce You can reproduce it by doing following.

  1. Create Test folder in Zope.
  2. Create folder property with boolean set to True.
  3. Change those property in the meta-file in file system to 1.
  4. zodbsync playback with the path to that associated meta file.
  5. Check if changes occured.

Expected behavior I would expect, that the property in zope would be updated to 1. However, it does not.

But if I set the property in meta file to 2 or if I update another property in addition in the meta file ( not True to 1, e.g. title), it recognizes the change and updates every property.

Additional context and analysis results So I tracked this issue to this code line:

https://github.com/perfact/zodbsync/blob/dadd60608ead1089cba7ff902925c3a7336894bd/perfact/zodbsync/zodbsync.py#L622

I researched the normal behavior of comparison operator in python and found this:

https://stackoverflow.com/questions/28033852/strict-comparison

A simple check in python console proves it:

>>> True == 1 True

It seems that for the time being 1 is treated the same as True if compared with each other.

In case of boolean-Properties in Zope-Objects it seems less harmful.

tjastrzembski commented 3 years ago

I would suggest a implementation of a stricter comparison between file system data and zope server data if this issue is a major problem.

viktordick commented 3 years ago

Why would you change a boolean's value from True to 1? A boolean property stores a truthy or falsy value. If you add a property using the ZMI, you initially get a text input field for setting the value, but this immediately gets converted to its boolean counterpart - afterwards you only get a checkbox in the ZMI, allowing you to set it to True or False, but no other value.

What exactly are you trying to do?

tjastrzembski commented 3 years ago

Well, to the background: I was confronted with a Zope-Repository where Zope objects hold boolean properties which seem to be originally initialized as 1 or 0. Now after changing a Zope object they seem to be updated into True or False.

This Repository is just one of three Repositories which are intended to not differ as close as possible.

However those changes are not transmitted when trying them to transmit to other systems if they are the only changes on the zope object.

I hope this current use case explains the issue.

viktordick commented 3 years ago

So this is a question of different Zope versions? Was it the change from Zope 2 to Zope 4/5 that changed the storage from allowing a value like 1 to only allowing really boolean types (i.e., True and False)?

Please test the following: Add a boolean property to some folder and try to set it to a value of 1 in a script, read it out again and check what value you get.

If Zope itself casts boolean property values either while writing or while reading to a bool, I do not think we should circumvent this with zodbsync.

But if I understand your comment correctly, you do not want to change from True to 1, but the other way round, in an object that for some legacy reasons has a value of 1 stored in a boolean property. Your original problem description described the other way.

I guess in that case we could compare the string representations of fs_data and srv_data - one of which is already available since it was just read from the file system, while the other can be obtained from mod_format.

Care to create a PR?

tjastrzembski commented 3 years ago

A bit late, but I was finally able to create this PR in #88.

viktordick commented 2 years ago

As discussed, this would be a workaround of an inconsistency in Zope itself. The attempt to fix it was incomplete and completing it was deemed not worth the hassle - storing values that are not consistent with the type of a given property is simply not supported and should be fixed manually.