fix: narrow KeyFirstEnvironment to hide only ruamel-specific attrs#794
Closed
fix: narrow KeyFirstEnvironment to hide only ruamel-specific attrs#794
Conversation
Ensure Mapping attribute access prefers keys and returns Undefined on missing keys, preventing ruamel attribute collisions (e.g. .tag) from yielding None. Update integration fixture to use global defaults for fallback values, run the case via --robot for exit-code validation, and add unit tests for the custom Jinja environment behavior.
The original key-first resolution was too broad — it resolved ALL dot-access
on Mappings key-first, causing `{{ obj.items }}` to return a key value instead
of the dict method. This broke the contract that CommentedMap/CommentedSeq
must behave identically to plain dict/list under standard Jinja2.
Narrow the intercept to only ruamel-specific blocklist attrs (_RUAMEL_MAP_ATTRS,
_RUAMEL_SEQ_ATTRS, yaml_* prefix). Everything else falls through to standard
Jinja2 getattr→getitem resolution, preserving .items(), .get(), .keys(),
.values() as callable methods.
Implementation:
- robot_writer.py: Reorder getattr() — check blocklist first, then key-in-obj
inside that branch; add CommentedSeq handling with separate blocklist
- Add blocklist constants with documented rationale
Tests:
- Split KeyFirstEnvironment tests into dedicated test_robot_writer_environment.py
(was mixed into test_robot_writer.py)
- 6 test classes covering: contract equivalence, key precedence, missing-key
behavior, method preservation, attr-getter filters, collision+method interaction
- Integration fixtures expanded: test_methods.robot (dict method calls),
test_selectattr.robot (selectattr/rejectattr/map with collision keys)
- Integration assertions use re.search() with msg= anchors for precise matching
- Collision key templates use bracket notation (child['items']) per contract
oboehmer
changed the title
Collaborator
Author
|
closing this in favor of #796 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fix
KeyFirstEnvironmentto match the contract that CommentedMap/CommentedSeq must behave identically to plain dict/list under standard Jinja2. The original implementation resolved ALL dot-access on Mappings key-first, which broke dict methods like.items(),.get(),.keys(),.values()when those names also existed as YAML keys. The fix narrows the intercept to only ruamel-specific metadata attributes (blocklist +yaml_*prefix), letting everything else follow standard Jinja2 getattr→getitem resolution.Templates that use collision keys like
itemsorkeysmust use bracket notation ({{ obj['items'] }}), which matches standard dict behavior.Closes
Related Issue(s)
Type of Change
Test Framework Affected
Network as Code (NaC) Architecture Affected
Platform Tested
Key Changes
robot_writer.py: RewriteKeyFirstEnvironment.getattr()— blocklist-first resolution with separate_RUAMEL_MAP_ATTRS/_RUAMEL_SEQ_ATTRSconstants; addCommentedSeqhandlingtest_robot_writer_environment.py(new): Dedicated unit tests forKeyFirstEnvironment— 6 test classes covering contract equivalence, key precedence, missing-key behavior, method preservation, attr-getter filters, and collision+method interactiontest_robot_writer.py: Removed oldTestKeyFirstEnvironment(superseded by dedicated test file)test_methods.robot(dict method calls),test_selectattr.robot(selectattr/rejectattr/map); collision key templates use bracket notationre.search()withmsg=anchors for precise rendered-output validationTesting Done
pytest/pre-commit run -a)Test Commands Used
Checklist
pre-commit run -apasses)Additional Notes
This supersedes the narrow fix from #789 with a principled approach: instead of key-first resolution for everything, only ruamel-specific metadata attributes are intercepted. Dict methods resolve normally (matching plain dict behavior), and templates must use bracket notation for keys that collide with dict method names.