bpo-39732: encode UIDs in XML as CF$UID #18622
Conversation
This commit lets the XML writer encode UIDs in XML as a dictionary with a single key CF$UID pointing to an integer value. This is how Apple handles UIDs when writing XMLs. It appears significantly harder to get the parser to replace a value it has just written, so I am not attempting it yet. I mean, I know how to replace it when the parent is a list, but when it is a dictionary it seems that I will need some extra key-saving along the stack.
The apple ref does not mention UID at all. Use a source that does.
|
Uhh, hello? @serhiy-storchaka care to take a look? Just in case someone quotes "NSKeyedArchiver only does binary" - it doesn't. The swift version accepts both XML and Binary as output (https://github.com/apple/swift-corelibs-foundation/blob/master/Sources/Foundation/NSKeyedArchiver.swift) and they got plenty of test cases with XML CF$UID at https://github.com/apple/swift-corelibs-foundation/tree/2a5bc4d8a0b073532e60410682f5eb8f00144870/Tests/Foundation/Resources. |
Consider this more advise than a review, but since I wrote the PR for plistlib.UID support I thought I might chime in.
Just in case someone quotes "NSKeyedArchiver only does binary" - it doesn't
Yes I realize this now and regret the error, I just recently hit the problem this PR is trying to address.
| @@ -130,7 +133,7 @@ The following classes are available: | |||
| .. class:: UID(data) | |||
|
|
|||
| Wraps an :class:`int`. This is used when reading or writing NSKeyedArchiver | |||
| encoded data, which contains UID (see PList manual). | |||
| encoded data, which contains UID (see Wikipedia). | |||
There was a problem hiding this comment.
It's not preferable to refer to Wikipedia.
There was a problem hiding this comment.
I know, but Apple doesn't document the archivers. Should I just link to the Swift foundation source?
There was a problem hiding this comment.
Swift foundation source sounds reasonable to me
| @@ -130,7 +133,7 @@ The following classes are available: | |||
| .. class:: UID(data) | |||
|
|
|||
| Wraps an :class:`int`. This is used when reading or writing NSKeyedArchiver | |||
| encoded data, which contains UID (see PList manual). | |||
| encoded data, which contains UID (see Wikipedia). | |||
There was a problem hiding this comment.
Swift foundation source sounds reasonable to me
| @@ -105,6 +105,9 @@ def __hash__(self): | |||
| <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> | |||
| """ | |||
|
|
|||
| # CF$UID KEY | |||
| CFUID = "CF$UID" | |||
There was a problem hiding this comment.
This might deserve documentation
Also should this be CFUID_KEY?
| @@ -225,7 +225,9 @@ def test_indentation_dict_mix(self): | |||
|
|
|||
| def test_uid(self): | |||
| data = UID(1) | |||
| self.assertEqual(plistlib.loads(plistlib.dumps(data, fmt=plistlib.FMT_BINARY)), data) | |||
| for fmt in ALL_FORMATS: | |||
There was a problem hiding this comment.
Maybe add another test case to verify dict format is as expected? (existing tests just assert that loads/dumps are inverses of each other, not necessarily the correct format)
| @@ -44,10 +44,16 @@ or :class:`datetime.datetime` objects. | |||
| .. versionchanged:: 3.9 | |||
| Old API removed. | |||
|
|
|||
| .. versionchanged:: 3.10 | |||
There was a problem hiding this comment.
This should be bumped to 3.11 (or based on how slowly this is getting reviewed, maybe 3.12
This commit lets the XML writer encode UIDs in XML as a dictionary with a single key CF$UID pointing to an integer value. This is how Apple handles UIDs when writing XMLs.
It appears significantly harder to get the parser to replace a value it has just written, so I am not attempting it yet. I mean, I know how to replace it when the parent is a list, but when it is a dictionary it seems that I will need some extra key-saving along the stack.An update has added a dict parsing replacer. Since there is always only one layer of key->dict involved, a single instance var will do.
https://bugs.python.org/issue39732