Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Senseful "OFS.SimpleItem.Item_w__name__.id" definition #906

Merged
merged 2 commits into from
Oct 7, 2020

Conversation

d-maurer
Copy link
Contributor

@d-maurer d-maurer commented Oct 6, 2020

Override id in OFS.SimpleItem.Item_w__name__ to avoid bugs by use of deprecated direct id access (--> #903 ).

In ancient days, all Zope objects had an id attribute. With the advent of Zope 3 technologies, __name__ seemed a better name for the name/id of a Zope object: direct id access was deprecated in favor of getId() and getId was defined to switch between id and __name__, as necessary. For backward compatibility __name__ was defined as alias for id, overridable on the instance level. This allowed for a graceful migration from the old id to the new __name__ use in the context of existing Zope object without __name__ but with id. However, newly created Item_w__name__ instances had a wrong id: for them, the deprecated direct access to id led to bugs (e.g. #903 ).

This PR defines Item_w__name__.id as follows:

  • if the instance has instance attribute id, it is used (this handles the case of an old Zope object still using id in place of __name__)
  • else if the instance has instance attribute __name__, its getId() it called; the precondition is necessary to avoid an infinite recursion (as __name__ is otherwise defined referring to id)
  • else "" is used as id

This definition is not perfect: it fails for example,

  • should a derived class decide to implement __name__ not via an instance attribute
  • for objects with both id and __name__; for them obj.id could be different from obj.getId(). Such a case could result from the copying or renaming of an old Zope object (with id, without __name__).

However, if seems considerable better then the current state.

This PR would fix #903; however, it is not the best fix possible. This PR tries to avoid problems with the deprecated direct id access in general, not particularly for #903.

@d-maurer d-maurer requested a review from dataflake October 6, 2020 09:11
@d-maurer d-maurer merged commit 80dfab9 into 4.x Oct 7, 2020
@d-maurer d-maurer deleted the item_w__name__#903 branch October 7, 2020 13:10
dataflake added a commit that referenced this pull request Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants