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

tests to reproduce #89 #90

Merged
merged 5 commits into from
Apr 4, 2024
Merged

Conversation

madpah
Copy link
Owner

@madpah madpah commented Apr 4, 2024

Fixes #89

Tests added to reproduce behaviour reported in #89 - now fail:
ValueError: stock_id is not a known Property for tests.model.Book

Signed-off-by: Paul Horton <paul.horton@owasp.org>
@madpah madpah added the bug Something isn't working label Apr 4, 2024
@madpah madpah self-assigned this Apr 4, 2024
@madpah madpah requested a review from jkowalleck as a code owner April 4, 2024 08:08
@madpah madpah marked this pull request as draft April 4, 2024 08:08
madpah added 4 commits April 4, 2024 10:23
…es not conform to current formatter

Signed-off-by: Paul Horton <paul.horton@owasp.org>
Signed-off-by: Paul Horton <paul.horton@owasp.org>
Signed-off-by: Paul Horton <paul.horton@owasp.org>
Signed-off-by: Paul Horton <paul.horton@owasp.org>
@madpah madpah marked this pull request as ready for review April 4, 2024 09:35
@madpah madpah merged commit ade5bd7 into main Apr 4, 2024
18 checks passed
@madpah madpah deleted the fix/deserialization-array-flat-custom-name branch April 4, 2024 11:39
for p, pi in klass_properties.items():
if pi.xml_array_config:
array_type, nested_name = pi.xml_array_config
if nested_name == strip_default_namespace(child_e.tag):
Copy link
Collaborator

Choose a reason for hiding this comment

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

The strip_default_namespace(child_e.tag) is as constant as it gets through all these nested loops.
Better calculate the value upfront, instead of each time the loop is processed.

@@ -557,6 +557,14 @@ def strip_default_namespace(s: str) -> str:
# Handle Sub-Elements
for child_e in data:
decoded_k = CurrentFormatter.formatter.decode(strip_default_namespace(child_e.tag))

if decoded_k not in klass_properties:
Copy link
Collaborator

Choose a reason for hiding this comment

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

possible optimiztion:

why not build up a lookup table, similar to klass_properties?
something that known these XML-serialization field names, and mapps them to the properties.

This might be better than looping through all these things and building a lookup in several looped iterations ....

the mapping would be built during register_xml_property_array_config() calls...

and could be applies here like so:

decoded_k = klass_properties_childnames_map.get(decoded_k, decoded_k)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Deserialization does not work for XmlArraySerializationType.FLAT where XML Name is customised
2 participants