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.

Sign up for GitHub

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

Jump to bottom

Don't prefix a property value that is already prefixed #313

Merged
merged 2 commits into from
Aug 17, 2023
Merged

Don't prefix a property value that is already prefixed #313

merged 2 commits into from
Aug 17, 2023

Conversation

fperrin
Copy link
Contributor

@fperrin fperrin commented Aug 14, 2023 •
edited by xavier-contreras
Loading

This is a rebase of @markuscraig's PR #213 over the most recent master (original PR has a conflict when rebasing), plus a testcase that exhibits the issue.

Without the fix, testcase test_json_ietf_serialise_namespace_handling_remote fails with:

>               self.assertEqual(
                    data["identityref:ietfint"]["ref"],
                    "remote-two:remote-id",
                )
E               AssertionError: 'remote-two:remote-two:remote-id' != 'remote-two:remote-id'
E               - remote-two:remote-two:remote-id
E               ? -----------
E               + remote-two:remote-id

Reference: https://www.ietf.org/rfc/rfc6020.txt sections 9.10.*

fperrin and others added 2 commits August 14, 2023 20:46
@fperrin
Add test coverage for JSON serialisation double namespace
d40bd21
@markuscraig @fperrin
Updates json ietf serialization to not prefix a property value that i…
06636bc 
…s already prefixed. This can occur when ietf serializing an instance that was deserialized from a json ietf string.
@fperrin fperrin mentioned this pull request Aug 14, 2023
Closed
xavier-contreras
xavier-contreras approved these changes Aug 17, 2023
View reviewed changes
Copy link
Collaborator

@xavier-contreras xavier-contreras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, LGTM

@xavier-contreras xavier-contreras merged commit 6b85c2b into robshakir:master Aug 17, 2023
JoseIgnacioTamayo pushed a commit that referenced this pull request Dec 11, 2023
@fperrin @markuscraig @JoseIgnacioTamayo
Don't prefix a property value that is already prefixed (#313)
6595e8a 
* Add test coverage for JSON serialisation double namespace

* Updates json ietf serialization to not prefix a property value that is already prefixed. This can occur when ietf serializing an instance that was deserialized from a json ietf string.

---------

Co-authored-by: Mark Craig <mark@mistsys.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xavier-contreras xavier-contreras xavier-contreras approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fperrin @xavier-contreras @markuscraig