codeconsole commented on PR #16732:
URL: https://github.com/apache/grails-core/pull/16732#issuecomment-6377229599
Rather than wait for the next sequence that makes the two representations
disagree, I checked them mechanically: every ordered sequence of up to three
writes drawn from ten write paths (a flat dotted key, a nested map, a dotted
key inside a nested map, a map under a dotted key, dot-notation set of a
scalar, of a map and of `null`, an indexed key at the root and inside a nested
map, and subscript assignment) over the keys `a`, `a.b`, `a.b.c`, `a.b.d` and
`a.e`, 75k sequences in all, each followed by a walk of the result that
requires every nested block and list to be the value of its flat alias, every
nested scalar to equal its flat key, every dotted flat key to lead to its
nested entry, and a clone written after each prefix to leave the original
unchanged. The probe is a throwaway spec and is not in the PR.
What it found, besides the states the layering rule leaves on purpose (the
flat key of a block that a later scalar replaced stays readable), were three
pre-existing bug classes, each fixed in its own commit with failing-first tests:
- d1914b1279: setting a nested property to `null` removed every flat key
under its parent, so `config.a.e = null` took `config['a.b.c']` with it, while
setting a root property to `null` removed no flat key below it, so `config.a =
null` left `config['a.b']` for the flat-first read to answer. The removal now
takes the flat key of the property, the keys below it and its indexed aliases,
wherever the property sits, and nothing else.
- d25bee0f5a: the clone shared the lists that indexed keys create, so an
indexed merge into a copy wrote the original's list in place. Lists and the
plain maps inside them are copied as the nested maps are, each once.
- 6d3ba8d01e: a dotted key inside a nested map, `merge([a: ['b.c': v]])`,
was also written as `b.c` into the `a` sub-map, a copy no other write kept
current, so after `merge(['a.b.c': w])` `getProperty('a', Map)` showed the old
value under `b.c` beside the new one under `b`. The first pass over a dotted
key now runs at the root only, where the dotted key is the flat key itself.
After the three fixes the probe reports only the layering states, the flat
key of a block that a scalar replaced, both while the scalar is there and after
a later merge has recreated the block without that key, as in the reply above.
grails-bootstrap and grails-core suites pass locally; the description lists the
fixes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
