Skip to content

Commit

Permalink
Add further tests for UpsertCondition and fix matching when should cl…
Browse files Browse the repository at this point in the history 
…auses mix with must_not clause
  • Loading branch information
@timatbw
timatbw committed Mar 11, 2024
1 parent fc391eb commit 0cedbed
Show file tree
Hide file tree
Showing 3 changed files with 150 additions and 10 deletions.
1 change: 0 additions & 1 deletion solr/core/src/java/org/apache/solr/update/processor/ConditionalUpsertProcessorFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 7 additions & 5 deletions solr/core/src/java/org/apache/solr/update/processor/UpsertCondition.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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
}
Expand Down Expand Up @@ -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());
}
Expand Down
147 changes: 143 additions & 4 deletions solr/core/src/test/org/apache/solr/update/processor/UpsertConditionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,64 @@ public void givenMultipleShouldClauses_whenMatching() {
assertFalse(condition.matches(oldDoc, newDoc));
}

@Test
public void givenMultipleShouldAndMustNotClauses_whenMatching() {
NamedList<String> 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<String> args = namedList(ImmutableListMultimap.of(
Expand Down Expand Up @@ -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");
Expand All @@ -870,10 +942,15 @@ public void givenConcatWithOldDoc_whenRunning() {

@Test
public void givenConcatWithAtomicUpdates_whenRunning() {
NamedList<String> args = namedList(ImmutableListMultimap.of(
"must_not", "NEW.sku:*",
"action", "concat:sku:model_name|product_range,colour,size?"
));
NamedList<String> args = namedList(ImmutableListMultimap.<String, String>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);

Expand Down Expand Up @@ -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"));
Expand All @@ -914,6 +1001,58 @@ public void givenConcatWithAtomicUpdates_whenRunning() {
}
}

@Test
public void givenConcatWithMultipleConditions_whenRunning() {
NamedList<?> args = namedList(ImmutableListMultimap.<String, NamedList<String>>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<UpsertCondition> 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<UpsertCondition> conditions = givenMultipleConditions();
Expand Down

0 comments on commit 0cedbed

Please sign in to comment.