Repository navigation
fix(databinding): bind the first of several indexers as the index - #16982
borinquenkid wants to merge 1 commit into
Conversation
A name such as columns[0][data] was split at its last indexer, so it looked for a property named columns[0] and was ignored. The first indexer now selects the element and the rest is a key inside it, as columns[0].data already did. Fixes #15064 Claude-Session: https://claude.ai/code/session_01Dmki4RzWwreEC2GkkyFnvL
jamesfredley
left a comment
jamesfredley
left a comment
There was a problem hiding this comment.
Not approving. A single indexer still binds. Several indexers into an existing map do not. Grouping also turns data-source awareness back on, throws on a name with a space, and drops a typed object that was already supplied.
| if (key.size() > 2 && ((key.startsWith("'") && key.endsWith("'")) || (key.startsWith('"') && key.endsWith('"')))) { | ||
| key = key[1..-2] | ||
| } | ||
| elements.computeIfAbsent(matcher.group(1)) { new LinkedHashMap<String, Object>() }.put(key + rest.group(2), source.getPropertyValue(name)) |
There was a problem hiding this comment.
A list already holds [data: old]. The request is columns[0][data]=new. The selected map should become [data: new]. It stays [data: old]. The grouped value is bound as bean properties, and a map has none named data, so the entry is ignored. Write the grouped entries into the selected map.
| } | ||
| grouped.put(element, entries) | ||
| } | ||
| new SimpleMapDataBindingSource(grouped) |
There was a problem hiding this comment.
The caller set dataSourceAware to false. Adding an unrelated name such as ignored[0][name] replaces the source with one that has it true. A later property can then load and bind an existing instance. Copy the caller's setting onto the grouped source.
| if (matcher) { | ||
| def rest = matcher.group(2) =~ /^\[\s*([^\s\]]*)\s*\](.*)$/ | ||
| rest.matches() | ||
| String key = rest.group(1) |
There was a problem hiding this comment.
A parameter named ignored[0][first name] used to be skipped. It matches the outer pattern and fails the inner one, because the key has a space. group(1) then throws, and the whole bind stops. If the inner pattern does not match, skip that name.
| if (existing instanceof Map) { | ||
| entries.putAll((Map) existing) | ||
| } | ||
| grouped.put(element, entries) |
There was a problem hiding this comment.
The source has factories[0] set to an existing factory, and factories[0][isActive] set to true. The existing factory should keep its identity and its other fields. This replaces it with a new map, so the name and the instance are gone. Keep a supplied object that is not a map, and apply the grouped entries to it.
✅ All tests passed ✅🏷️ Commit: d13dcf1 Learn more about TestLens at testlens.app/docs. |
columns[0][data]was split at its last indexer, so the binder looked for a property namedcolumns[0]and ignored the value without an error. The first indexer now selects the element and the rest is a key inside it, the same ascolumns[0].databinds. Names that share the first index are gathered into one element, and a name with one indexer takes the path it took before.Verified locally: the three new
SimpleDataBinderSpecfeatures fail without the change and pass with it; all 76grails-databinding-coretests pass; codenarc and checkstyle are clean. The data binding guide gains a short section.Fixes #15064
https://claude.ai/code/session_01Dmki4RzWwreEC2GkkyFnvL