diff --git a/solr/core/src/java/org/apache/solr/update/processor/ConditionalUpsertProcessorFactory.java b/solr/core/src/java/org/apache/solr/update/processor/ConditionalUpsertProcessorFactory.java index b8c8278afcb..063d3002afd 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/ConditionalUpsertProcessorFactory.java +++ b/solr/core/src/java/org/apache/solr/update/processor/ConditionalUpsertProcessorFactory.java @@ -20,7 +20,6 @@ import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.List; -import java.util.Map; import org.apache.lucene.util.BytesRef; import org.apache.solr.common.SolrException; diff --git a/solr/core/src/java/org/apache/solr/update/processor/UpsertCondition.java b/solr/core/src/java/org/apache/solr/update/processor/UpsertCondition.java index 65074d6b4c5..4af24872ff3 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/UpsertCondition.java +++ b/solr/core/src/java/org/apache/solr/update/processor/UpsertCondition.java @@ -140,6 +140,7 @@ ActionType run(SolrInputDocument oldDoc, SolrInputDocument newDoc) { boolean matches(SolrInputDocument oldDoc, SolrInputDocument newDoc) { Docs docs = new Docs(oldDoc, newDoc); boolean atLeastOneMatched = false; + boolean hasPositive = false; for (FieldRule rule: rules) { boolean ruleMatched = rule.matches(docs); switch(rule.getOccur()) { @@ -148,27 +149,28 @@ boolean matches(SolrInputDocument oldDoc, SolrInputDocument newDoc) { return false; } atLeastOneMatched = true; + hasPositive = true; break; case MUST_NOT: if (ruleMatched) { return false; } - atLeastOneMatched = true; break; default: atLeastOneMatched = ruleMatched || atLeastOneMatched; + hasPositive = true; break; } } - return atLeastOneMatched; + return atLeastOneMatched || !hasPositive; } enum ActionType { UPSERT, // copy some/all fields from the OLD doc (when they don't exist on the new doc) RETAIN, // copy some/all fields from the OLD doc always NULLIFY, // make sure specific fields are null before doc written - CONCAT, // set a field to be the concatenation of other fields from NEW or OLD doc - CONCAT_LC, // set a field to be the lowercase concatenation of other fields from NEW or OLD doc + CONCAT, // attempt to set a field to be the concatenation of other fields from NEW or OLD doc + CONCAT_LC, // attempt to set a field to be the lowercase concatenation of other fields from NEW or OLD doc INSERT, // just do a regular insert as normal SKIP; // entirely skip inserting the doc } @@ -371,7 +373,7 @@ void run(SolrInputDocument oldDoc, SolrInputDocument newDoc) { // One of the required fields is not present, so we can't set the target field return; } - builder.append(type == ActionType.CONCAT_LC ? fieldValue.toLowerCase() : fieldValue); + builder.append(type == ActionType.CONCAT_LC ? fieldValue.toLowerCase(Locale.ROOT) : fieldValue); } newDoc.setField(target, builder.toString()); } diff --git a/solr/core/src/test/org/apache/solr/update/processor/UpsertConditionTest.java b/solr/core/src/test/org/apache/solr/update/processor/UpsertConditionTest.java index 2b8bc881705..ad6dee1f1f0 100644 --- a/solr/core/src/test/org/apache/solr/update/processor/UpsertConditionTest.java +++ b/solr/core/src/test/org/apache/solr/update/processor/UpsertConditionTest.java @@ -442,6 +442,64 @@ public void givenMultipleShouldClauses_whenMatching() { assertFalse(condition.matches(oldDoc, newDoc)); } + @Test + public void givenMultipleShouldAndMustNotClauses_whenMatching() { + NamedList args = namedList(ImmutableListMultimap.of( + "should", "NEW.field1:*", + "should", "NEW.field2:*", + "must_not", "NEW.field3:*", + "action", "skip" + )); + + UpsertCondition condition = UpsertCondition.parse("skip-it", args); + + assertThat(condition.getName(), is("skip-it")); + + { + SolrInputDocument newDoc = new SolrInputDocument(); + assertFalse(condition.matches(null, newDoc)); + } + + { + SolrInputDocument newDoc = new SolrInputDocument(); + newDoc.setField("field3", "anything"); + assertFalse(condition.matches(null, newDoc)); + } + + { + SolrInputDocument newDoc = new SolrInputDocument(); + newDoc.setField("field2", "anything"); + assertTrue(condition.matches(null, newDoc)); + } + + { + SolrInputDocument newDoc = new SolrInputDocument(); + newDoc.setField("field1", "anything"); + newDoc.setField("field2", "anything-else"); + assertTrue(condition.matches(null, newDoc)); + } + + { + SolrInputDocument newDoc = new SolrInputDocument(); + newDoc.setField("field2", "anything"); + newDoc.setField("field3", "stuff"); + assertFalse(condition.matches(null, newDoc)); + } + + { + SolrInputDocument newDoc = new SolrInputDocument(); + newDoc.setField("field99", "anything"); + assertFalse(condition.matches(null, newDoc)); + } + + { + SolrInputDocument newDoc = new SolrInputDocument(); + newDoc.setField("field99", "anything"); + newDoc.setField("field3", "stuff"); + assertFalse(condition.matches(null, newDoc)); + } + } + @Test public void givenMustAndMustNotClauses_whenMatching() { NamedList args = namedList(ImmutableListMultimap.of( @@ -854,6 +912,20 @@ public void givenConcatWithOldDoc_whenRunning() { // fallback to new.product_range since model_name unavailable in old and new } + { + SolrInputDocument newDoc = new SolrInputDocument(); + newDoc.setField("colour", "Grey"); + SolrInputDocument oldDoc = new SolrInputDocument(); + oldDoc.setField("product_range", "Laptop"); + oldDoc.setField("colour", "Silver"); + oldDoc.setField("size", "17in"); + oldDoc.setField("sku", "PowerbookSilver17in"); + assertTrue(condition.matches(oldDoc, newDoc)); + assertThat(condition.run(oldDoc, newDoc), is(UpsertCondition.ActionType.CONCAT)); + assertThat(newDoc.getFieldValue("sku"), is("LaptopGrey17in")); + // fallback to old.product_range since model_name unavailable in old and new + } + { SolrInputDocument newDoc = new SolrInputDocument(); newDoc.setField("size", "16in"); @@ -870,10 +942,15 @@ public void givenConcatWithOldDoc_whenRunning() { @Test public void givenConcatWithAtomicUpdates_whenRunning() { - NamedList args = namedList(ImmutableListMultimap.of( - "must_not", "NEW.sku:*", - "action", "concat:sku:model_name|product_range,colour,size?" - )); + NamedList args = namedList(ImmutableListMultimap.builder() + .put("should", "NEW.model_name:*") + .put("should", "NEW.product_range:*") + .put("should", "NEW.colour:*") + .put("should", "NEW.size:*") + .put("must_not", "NEW.sku:*") + .put("action", "concat:sku:model_name|product_range,colour,size?") + .build() + ); UpsertCondition condition = UpsertCondition.parse("concat", args); @@ -902,6 +979,16 @@ public void givenConcatWithAtomicUpdates_whenRunning() { assertThat(newDoc.getFieldValue("sku"), is("PowerbookSilver12in")); } + { + SolrInputDocument newDoc = new SolrInputDocument(); + newDoc.setField("unrelated_field", Collections.singletonMap("set", "English")); + SolrInputDocument oldDoc = new SolrInputDocument(); + oldDoc.setField("model_name", "Powerbook"); + oldDoc.setField("colour", "Silver"); + oldDoc.setField("sku", "PowerbookSilver"); + assertFalse(condition.matches(oldDoc, newDoc)); + } + { SolrInputDocument newDoc = new SolrInputDocument(); newDoc.setField("size", Collections.singletonMap("set", "12in")); @@ -914,6 +1001,58 @@ public void givenConcatWithAtomicUpdates_whenRunning() { } } + @Test + public void givenConcatWithMultipleConditions_whenRunning() { + NamedList args = namedList(ImmutableListMultimap.>builder() + .put("modelBased", namedList(ImmutableListMultimap.of( + "must_not", "NEW.sku:*", + "action", "concat:sku:model_name,colour,size?" + ))) + .put("productBased", namedList(ImmutableListMultimap.of( + "must_not", "NEW.sku:*", + "action", "concat:sku:product_range,colour,size?" + ))) + .build() + ); + + final List conditions = UpsertCondition.readConditions(args); + + { + SolrInputDocument newDoc = new SolrInputDocument(); + newDoc.setField("model_name", Collections.singletonMap("set", "Macbook")); + newDoc.setField("product_range", Collections.singletonMap("set", "Laptop")); + SolrInputDocument oldDoc = new SolrInputDocument(); + oldDoc.setField("model_name", "Powerbook"); + oldDoc.setField("colour", "Silver"); + oldDoc.setField("sku", "PowerbookSilver"); + assertThat(UpsertCondition.shouldInsertOrUpsert(conditions, oldDoc, newDoc), is(true)); + assertThat(newDoc.getFieldValue("sku"), is("MacbookSilver")); + // does not go on to set sku using product_range because new.sku is set from first condition + } + + { + SolrInputDocument newDoc = new SolrInputDocument(); + newDoc.setField("product_range", Collections.singletonMap("set", "Laptop")); + SolrInputDocument oldDoc = new SolrInputDocument(); + oldDoc.setField("colour", "Silver"); + oldDoc.setField("sku", "PowerbookSilver"); + assertThat(UpsertCondition.shouldInsertOrUpsert(conditions, oldDoc, newDoc), is(true)); + assertThat(newDoc.getFieldValue("sku"), is("LaptopSilver")); + // first condition is not able to actually set new.sku so second condition matches + } + + { + SolrInputDocument newDoc = new SolrInputDocument(); + newDoc.setField("colour", Collections.singletonMap("set", "Black")); + SolrInputDocument oldDoc = new SolrInputDocument(); + oldDoc.setField("colour", "Silver"); + oldDoc.setField("sku", "PowerbookSilver"); + assertThat(UpsertCondition.shouldInsertOrUpsert(conditions, oldDoc, newDoc), is(true)); + assertThat(newDoc.getFieldValue("sku"), nullValue()); + // neither condition was able to actually set new.sku + } + } + @Test public void givenExistingPermanentDelete_whenCheckingShouldInsertOrUpsert() { List conditions = givenMultipleConditions();