Skip to content

Store a UID in one type and refuse what is not one - #3045

Open
ramonski wants to merge 2 commits into
2.xfrom
fix/one-uid-field-type
Open

ramonski wants to merge 2 commits into
2.xfrom
fix/one-uid-field-type

Conversation

@ramonski

@ramonski ramonski commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

A UID is 32 hex characters, and the code declares that in two incompatible ways. The same value is therefore right in one place and wrong in the next, and the complaint arrives as a WrongContainedType on the whole object, far from the value that caused it.

Description of the issue/feature this PR addresses

Two declarations for one thing:

# senaite/core/schema/uidreferencefield.py
value_type = ASCIILine(title=u"UID")         # native str, under Python 2

# senaite/core/content/analysisprofile.py
uid = schema.TextLine(title=u"Profile UID")  # unicode

SampleTemplate and WorksheetTemplate declare their rows like AnalysisProfile.

Which spelling arrives depends on nothing but the route. A JSON body carries unicode; code running in the process carries what the catalog holds, which is a native string. Both directions have been hit: a spec composed in process stored native strings into the profile rows and the object stopped validating, and a UID that came in over the JSON API as unicode did the same to an analysis category.

And nothing checked that a UID in a datagrid row is a UID at all.

Current behavior before PR

>>> IAnalysisProfileRecord["uid"].validate(str(uid))
WrongType: ('...', <type 'unicode'>, 'uid')

>>> UIDReferenceField().value_type.validate(six.text_type(uid))
WrongType: (u'...', <type 'str'>, None)

A row with a made-up UID passes the setter's type check and is stored, and the API reports success:

POST /create {"portal_type": "AnalysisProfile", ...,
              "services": [{"uid": "not-a-uid", "hidden": false}]}

200 {"count": 1, "items": [{..., "services": [{"hidden": false,
                                              "uid": "not-a-uid"}]}]}

Desired behavior after PR is merged

One field type, UIDField, exported from senaite.core.schema like its neighbours and used for the value type of UIDReferenceField and for the uid of each of those rows. It takes both spellings, because both are already stored, and refuses a value that is not a UID:

    _type = six.string_types

    def _validate(self, value):
        super(UIDField, self)._validate(value)
        # The reference field checks this for the UIDs it holds itself.
        # A UID written into a datagrid row had nothing checking it, so
        # anything shaped like a line of ASCII was stored and only came
        # up later, as a row pointing at nothing.
        if value and not api.is_uid(value):
            raise InvalidValue(value)

What is stored is the native string, and the writers normalize to it:

def to_uid(value):
    """Normalize a UID to the native string it is stored as

    A UID is made by plone.uuid as a native string, kept on the object
    as one under `_plone.uuid`, handed out by `UID()` as one and
    indexed in the uid catalog as one. A reference stored as text would
    differ in type from the thing it points at, for no reason beyond
    how it happened to arrive.
    ...
    """

The same request as above is now refused with {"services": "WrongContainedType"}.

Migration

None, and deliberately so.

The field only widens what it accepts, so everything that validated before still validates and no stored value becomes invalid. The two spellings compare and hash alike under Python 2, and the uid catalog finds an object under either, which was checked rather than assumed:

uid_catalog str=1 unicode=1
get_object_by_uid str=True unicode=True
equal=True same_hash=True

An object written again is written in the one shape, so the mixture clears itself as sites are used. There is nothing for an upgrade step to convert and the profile version is untouched.

Verification

  • bin/test-senaite -s senaite.core: 229 tests, 0 failures, 0 errors.
  • bin/test-senaite -s senaite.jsonapi: 41 tests, 0 failures, 0 errors.
  • uidreferencefield.txt gained a section covering both spellings for the reference field and for a datagrid row, the native string that comes back out, the portal's own 0, a value that is not a UID, and one that is not ASCII at all.
  • Checked by taking each part back out on its own: the section fails without the widened type, without the normalization, and the made-up UID is stored again without the check.

I confirm I have tested the PR thoroughly and coded it according to PEP8 standards.

@ramonski
ramonski force-pushed the fix/one-uid-field-type branch from b0f3e13 to 8eddc7e Compare October 6, 2026 22:13
@ramonski ramonski changed the title Store a UID in one field type, whatever spelling it arrives in Use one field type for a UID, whatever spelling it arrives in Oct 6, 2026
@ramonski
ramonski force-pushed the fix/one-uid-field-type branch from 8eddc7e to 9daa784 Compare October 7, 2026 06:25
@ramonski ramonski changed the title Use one field type for a UID, whatever spelling it arrives in Store a UID in one type and refuse what is not one Oct 7, 2026
@ramonski
ramonski force-pushed the fix/one-uid-field-type branch from 9daa784 to a844295 Compare October 7, 2026 06:41
@ramonski
ramonski requested a review from xispa October 7, 2026 07:17
@ramonski ramonski added the Enhancement ✨ Improvement to existing functionality label Oct 7, 2026
A UID is 32 hex characters, and the code declared that in two
incompatible ways. UIDReferenceField used ASCIILine, which under
Python 2 means a native string, while the datagrid rows of
AnalysisProfile, SampleTemplate and WorksheetTemplate used TextLine,
which means unicode. The same value was therefore right in one place
and wrong in the next, and which of the two arrived depended on
nothing but the route: a JSON body carries unicode, code running in
the process carries what the catalog holds.

One field type now, UIDField, used for the value type of
UIDReferenceField and for the uid of each of those rows. It takes
both, because both are already stored and because refusing one would
turn how a value arrived into a WrongContainedType on a field the
caller never mentioned.

What is stored is the native string. That is what plone.uuid makes, it
is what the object keeps under _plone.uuid, it is what UID() hands out
and what the uid catalog is indexed with, so a reference stored as
text would differ in type from the thing it points at. Nothing has to
be converted: the two compare and hash alike under Python 2, and an
object written again is written in the one shape.

Both fields normalize where they store, not where they are called
from. zope.schema never asks a subfield to store anything, so the rows
of a datagrid go in as they came; the three fields holding them are
DataGridField now, which has somewhere to put that.

The field also refuses a value that is not a UID, which is what
UIDReferenceField has always done for the UIDs it holds itself. A UID
inside a datagrid row had nothing checking it: a record with a made-up
uid passed the setter's type check and was stored, and the API
answered that the object had been created. It is refused now.
@ramonski
ramonski force-pushed the fix/one-uid-field-type branch from a844295 to a99dbf9 Compare October 8, 2026 08:13
@ramonski

ramonski commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Force-pushed a correction.

UIDField._type was six.string_types, which is a tuple on Python 2. z3c.form builds the value of a collection by calling its value type's _type on every line of the form, taking valueType[0] when it is a tuple, so it reached the form as an attempt to instantiate basestring:

Module senaite.core.z3cform.datagridfield.datagridfield, line 394, in _validate
Module senaite.core.z3cform.widgets.queryselect.widget, line 76, in toFieldValue
Module z3c.form.converter, line 356, in toFieldValue
TypeError: The basestring type cannot be instantiated

That broke the edit form of every Dexterity type holding a UIDReferenceField, which I found on a SampleTemplate with partitions.

_type now stays the single type the field stores, and the tolerance for text moved into _validate, which normalises before it checks. The doctest gained the assertion that would have caught it:

>>> field.value_type._type(uid) == uid
True

@ramonski

ramonski commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Cross-reference: #3049 touches the same two files and is independent of this one, in either merge order.

This pull request changes the field types of a sample template's service record; #3049 changes what the setters write into it. Between them a record has two sub-fields in the wrong type, and they fix one each: uid here, part_id there. ISampleTemplateSchema["services"] therefore validates only once both are merged, and neither is wrong on its own in the meantime.

The to_uid added here and the to_uid_list added there end up side by side in uidreferencefield.py without overlapping: the native string a UID is stored as, against the list of one that a reference field holds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement ✨ Improvement to existing functionality

Development

Successfully merging this pull request may close these issues.

1 participant