nick-boss-tech opened a new pull request, #5085: URL: https://github.com/apache/solr/pull/5085
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)* https://issues.apache.org/jira/browse/SOLR-12703 ## What happens today **A nested atomic operation is stored as a field value instead of being rejected.** An atomic update can put an operation map inside an operation value, for example `{"set": {"set": "bbb"}}`. The `set` case stores the operand as the field's value ([doSet](https://github.com/nick-boss-tech/solr/blob/63c84919c80b15c20d5b93bbacaf3f21835648e5/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L479-L482)). The inner operation is never checked. ## What this change does **The merger rejects a nested operation map with a 400 error. The error names the operation, not its value.** - The check runs for each operation in the update, before its switch ([check](https://github.com/nick-boss-tech/solr/blob/63c84919c80b15c20d5b93bbacaf3f21835648e5/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L162-L175)). - The message names the outer operation, the field, and the nested operation names. It does not include the nested value. - A value counts as a nested operation when it is a non-empty map whose keys are all atomic operation names ([helper](https://github.com/nick-boss-tech/solr/blob/63c84919c80b15c20d5b93bbacaf3f21835648e5/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L710-L720)). ## Proof **The updated test fails on the earlier head and passes with this change.** - `AtomicUpdatesTest` 27 tests, 0 failures, 1 skipped with this change. The new test is `testNestedAtomicOperationIsRejected` ([test](https://github.com/nick-boss-tech/solr/blob/63c84919c80b15c20d5b93bbacaf3f21835648e5/solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java#L1547-L1575)). - Pre-fix proof PASS: the updated test fails on the head before the message change, [`ed95d555e62`](https://github.com/nick-boss-tech/solr/commit/ed95d555e62b030d4aa2c2dc5a3f011642371f9c). - The module check is clean. - Verified at this head. ## Limits - The test calls the merger directly. The SolrJ request path is not exercised, and neither is the partial processing of earlier operations in the same update. - Sequencing with SOLR-6045: both branches change `mergeDocHavingSameId`. SOLR-6045 replaces the loop's source with a flattened list of operations ([loop](https://github.com/nick-boss-tech/solr/blob/e4b77fa7ae53d363e8869e1073a800c324fd6972/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L180)). This change adds its check inside the same loop. The two merge without conflicts in either order, but their combined behavior has not been run. The two should be sequenced, not merged cold. Changelog: `changelog/unreleased/SOLR-12703-nested-atomic-op.yml` ### AI assistance AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
