This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch branch_10_1 in repository https://gitbox.apache.org/repos/asf/solr.git
commit dcb4f04a0160544a9f1445ee6e34ae5c871d224f Author: David Smiley <[email protected]> AuthorDate: Sat Sep 26 20:28:56 2026 -0400 DocBasedVersionConstraints URP: reject _version_ (#4877) DocBasedVersionConstraintsProcessor now rejects a client-supplied _version_ with a 400 instead of silently ignoring it. And it no longer validates the tombstone document shape when deleteVersionParam isn't configured. (cherry picked from commit ba429f23878ba555f940d1515bd3909b9f127445) (cherry picked from commit c4717bca74ddde64bdeacee4f2dac559f14377c1) --- ...sedversionconstraints-reject-native-version.yml | 9 ++++++ .../DocBasedVersionConstraintsProcessor.java | 24 ++++++++++++++ ...DocBasedVersionConstraintsProcessorFactory.java | 20 ++++++++++-- .../processor/TestDocBasedVersionConstraints.java | 37 ++++++++++++++++++++++ 4 files changed, 87 insertions(+), 3 deletions(-) diff --git a/changelog/unreleased/PR#4877-docbasedversionconstraints-reject-native-version.yml b/changelog/unreleased/PR#4877-docbasedversionconstraints-reject-native-version.yml new file mode 100644 index 00000000000..7f2936c39f3 --- /dev/null +++ b/changelog/unreleased/PR#4877-docbasedversionconstraints-reject-native-version.yml @@ -0,0 +1,9 @@ +title: > + DocBasedVersionConstraintsProcessor now rejects a client-supplied _version_ with a 400 instead of silently ignoring it. + And it no longer validates the tombstone document shape when deleteVersionParam isn't configured +type: changed +authors: + - name: David Smiley +links: + - name: PR#4877 + url: https://github.com/apache/solr/pull/4877 \ No newline at end of file diff --git a/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessor.java b/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessor.java index 7a6dbcde04d..5ffba9f2f12 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessor.java +++ b/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessor.java @@ -416,6 +416,8 @@ public class DocBasedVersionConstraintsProcessor extends UpdateRequestProcessor return; } + rejectClientSuppliedVersion(cmd); + final SolrInputDocument newDoc = cmd.getSolrInputDocument(); Object[] newVersions = getUserVersionsFromDocument(newDoc); validateUserVersions(newVersions, versionFieldNames, "Doc does not have versionField: "); @@ -442,6 +444,28 @@ public class DocBasedVersionConstraintsProcessor extends UpdateRequestProcessor } } + /** + * Rejects an update carrying Solr's native <code>_version_</code>. This processor overwrites that + * field to guard its own read-then-write, so a client precondition could not be honored. Consults + * the same sources, in the same order, as {@link DistributedUpdateProcessor} would. + * + * <p>NOTE: Perhaps we could do either-or, or combine both somehow. + */ + private static void rejectClientSuppliedVersion(AddUpdateCommand cmd) { + if (cmd.getVersion() == 0 + && cmd.getSolrInputDocument().getField(CommonParams.VERSION_FIELD) == null + && cmd.getReq().getParams().get(CommonParams.VERSION_FIELD) == null) { + return; + } + throw new SolrException( + BAD_REQUEST, + "Optimistic concurrency via " + + CommonParams.VERSION_FIELD + + " is not supported when DocBasedVersionConstraintsProcessorFactory is configured," + + " since it uses that field itself. Express version constraints with the configured" + + " versionField(s) instead."); + } + private static void logOverlyFailedRetries(int i, UpdateCommand cmd) { // Log a warning every 256 retries.... even a few retries should normally be very unusual. if ((i & 0xff) == 0xff) { diff --git a/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessorFactory.java b/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessorFactory.java index 90307f9dc38..f70e05cf6c2 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessorFactory.java +++ b/solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessorFactory.java @@ -76,10 +76,20 @@ import org.slf4j.LoggerFactory; * false</code>, but if set to <code>true</code> allows any documents written *before* this * feature is enabled and which are missing the versionField to be overwritten. * <li><code>tombstoneConfig</code> - a list of field names to values to add to the created - * tombstone document. In general is not a good idea to populate tombsone documents with - * anything other than the minimum required fields so that it doean't match queries + * tombstone document. Only has any effect in combination with <code>deleteVersionParam + * </code>, since tombstones are created solely by the Delete By Id handling that param + * enables; without it this init param is inert. In general, it is not a good idea to populate + * tombstone documents with anything beyond the minimum required fields, so that they don't + * match queries. * </ul> * + * <p><b>This processor is incompatible with Solr's native optimistic concurrency.</b> It uses the + * <code>_version_</code> field for itself, setting it on every update so that its read of the + * existing document's version and the subsequent write are atomic, retrying on conflict. A <code> + * _version_</code> supplied by the client — on the document or as a request parameter — could + * therefore not be honored, so an Add carrying one is rejected with a 400. Express ordering through + * the per-document <code>versionField</code> values instead. + * * @since 4.6.0 */ public class DocBasedVersionConstraintsProcessorFactory extends UpdateRequestProcessorFactory @@ -205,7 +215,11 @@ public class DocBasedVersionConstraintsProcessorFactory extends UpdateRequestPro } } - canCreateTombstoneDocument(core.getLatestSchema()); + if (!deleteVersionParamNames.isEmpty()) { + // Tombstones are only produced by the Delete-By-Id handling that deleteVersionParam enables, + // so without it there is no tombstone shape to validate. + canCreateTombstoneDocument(core.getLatestSchema()); + } } /** diff --git a/solr/core/src/test/org/apache/solr/update/processor/TestDocBasedVersionConstraints.java b/solr/core/src/test/org/apache/solr/update/processor/TestDocBasedVersionConstraints.java index 5b69ae0d37c..101cdc6ab20 100644 --- a/solr/core/src/test/org/apache/solr/update/processor/TestDocBasedVersionConstraints.java +++ b/solr/core/src/test/org/apache/solr/update/processor/TestDocBasedVersionConstraints.java @@ -53,6 +53,43 @@ public class TestDocBasedVersionConstraints extends SolrTestCaseJ4 { assertU(commit()); } + /** + * The processor uses {@code _version_} for its own optimistic concurrency, so a client-supplied + * one cannot be honored and must fail rather than be silently ignored. + */ + public void testNativeVersionIsRejected() throws Exception { + try (ErrorLogMuter muter = ErrorLogMuter.regex("Optimistic concurrency via _version_")) { + // on the document + SolrException ex = + expectThrows( + SolrException.class, + () -> + updateJ( + jsonAdd(sdoc("id", "aaa", "my_version_l", "1001", "_version_", "-1")), + params("update.chain", "external-version-constraint"))); + assertEquals(400, ex.code()); + + // as a request param + ex = + expectThrows( + SolrException.class, + () -> + updateJ( + jsonAdd(sdoc("id", "aaa", "my_version_l", "1001")), + params("update.chain", "external-version-constraint", "_version_", "-1"))); + assertEquals(400, ex.code()); + + assertEquals(2, muter.getCount()); + } + + // sanity check: the same add without a version still works + updateJ( + jsonAdd(sdoc("id", "aaa", "my_version_l", "1001")), + params("update.chain", "external-version-constraint")); + assertU(commit()); + assertJQ(req("q", "id:aaa"), "/response/numFound==1"); + } + public void testSimpleUpdates() throws Exception { // skip low version against committed data
