Skip to content
Permalink
Browse files
Merge pull request #513 from klcodanr/OAK-9717
OAK-9717 - Prevent Merge from Removing nodes
  • Loading branch information
thomasmueller committed Mar 14, 2022
2 parents 24c54e5 + f220ce4 commit ae5ef0a7ea06e5ddefb53cdfea798a31f80a2bce
Showing 19 changed files with 1,439 additions and 599 deletions.
@@ -163,7 +163,9 @@ private static JsonObject mergeChild(String path, String child, int level, JsonO
if (level == 0 && USE_PRODUCT_CHILD_LEVEL_0.contains(child)) {
return p;
}
if (isSameJson(a, p) || isSameJson(c, p)) {
if (c == null && p != null) {
return p; // restore child nodes in product index if removed in custom index
} else if (isSameJson(a, p) || isSameJson(c, p)) {
return c;
} else if (isSameJson(a, c)) {
return p;
@@ -0,0 +1,67 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.jackrabbit.oak.index.merge;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;

import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;

import groovy.json.StringEscapeUtils;

/**
* Test merging conflict detection
*/
@RunWith(Parameterized.class)
public class IndexDefMergerConflictsTest extends ParameterizedMergingTestBase {

@Parameters(name = "{0}")
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][] {
testCase("product and custom properties must equal if not in ancestor",
"conflict-new-properties.json")
});
}

private final String expectedMessage;

public IndexDefMergerConflictsTest(String name, String testCaseFile)
throws IOException {
super(name, testCaseFile);
this.expectedMessage = StringEscapeUtils.unescapeJavaScript(testCase.getProperties().get("expected"));
}

@Test
public void verifyExpectedConflict() {
UnsupportedOperationException exception = assertThrows(
"Expected exception not thrown for test case:" + super.testCaseName,
UnsupportedOperationException.class,
() -> IndexDefMergerUtils.merge(buildIndexes, runIndexes));
assertEquals("Unexpected exception message validating: " + super.testCaseName, "" + expectedMessage,
"\"" + exception.getMessage() + "\"");
}

}
@@ -20,55 +20,68 @@

import static org.junit.Assert.assertEquals;

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collection;

import org.apache.commons.io.IOUtils;
import org.apache.jackrabbit.oak.commons.json.JsonObject;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Test merging index definitions.
*/
@RunWith(Parameterized.class)
public class IndexDefMergerScenariosTest {
@Parameters
public class IndexDefMergerScenariosTest extends ParameterizedMergingTestBase {

private static final Logger log = LoggerFactory.getLogger(IndexDefMergerConflictsTest.class);

@Parameters(name = "{0}")
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][] {
testCase("should use the latest base version for the base in merges", "merges-base"),
testCase("should use the latest base version for the base in merges when updating multiple version numbers", "merges-multi-version")
testCase("should merge custom into new base index", "basic.json"),
testCase("should use the latest base version for the base in merges", "merges-base.json"),
testCase(
"should use the latest base version for the base in merges when updating multiple version numbers",
"merges-multi-version.json"),
testCase("should not remove child nodes from product index missing from custom index",
"missing-child.json"),
testCase("should support removing adding changing properties from product index in custom index",
"removed-property.json")
});
}

public static Object[] testCase(String name, String baseName) {
return new Object[] {
name,
baseName + "_build.json",
baseName + "_run.json",
baseName + "_expected.json"
};
}

private final String name;
private final JsonObject buildIndexes;
private final JsonObject runIndexes;
private final JsonObject expectedIndexes;

public IndexDefMergerScenariosTest(String name, String buildFile, String runFile, String expectedFile)
public IndexDefMergerScenariosTest(String name, String testCaseFile)
throws IOException {
this.name = name;
this.buildIndexes = JsonObject.fromJson(IndexDefMergerTest.readFromResource(buildFile), true);
this.runIndexes = JsonObject.fromJson(IndexDefMergerTest.readFromResource(runFile), true);
this.expectedIndexes = JsonObject.fromJson(IndexDefMergerTest.readFromResource(expectedFile), true);
super(name, testCaseFile);
this.expectedIndexes = super.getTestCaseChild("expected");
}

@Test
public void testMerge() {
public void verifyExpectedMergeResult() {
IndexDefMergerUtils.merge(buildIndexes, runIndexes);
assertEquals("Failed to execute test: " + name, expectedIndexes.toString(), buildIndexes.toString());
File output = new File("target" + File.separator + "surefire-output" + File.separator
+ getClass().getCanonicalName().replace(".", "-") + File.separator + testCaseFile);
try {
if (!output.getParentFile().exists()) {
output.getParentFile().mkdirs();
}
IOUtils.write(buildIndexes.toString(), new FileOutputStream(output),
StandardCharsets.UTF_8);
} catch (IOException e) {
log.warn("Failed to write merged index definition to: {}", output, e);
}
assertEquals("Failed to execute test: " + testCaseName, expectedIndexes.toString(), buildIndexes.toString());
}

}
@@ -44,25 +44,6 @@ public void merge() throws IOException, CommitFailedException {
}
}

@Test
public void mergeIndexes() throws IOException, CommitFailedException {
String s = readFromResource("mergeIndexes.txt");
JsonObject json = JsonObject.fromJson(s, true);
for(JsonObject e : array(json.getProperties().get("tests"))) {
mergeIndexes(e);
}
}

private void mergeIndexes(JsonObject e) {
JsonObject all = e.getChildren().get("all");
JsonObject newDefs = e.getChildren().get("new");
JsonObject expectedNew = e.getChildren().get("expectedNew");
IndexDefMergerUtils.merge(newDefs, all);
assertEquals(
expectedNew.toString(),
newDefs.toString());
}

private void merge(JsonObject e) {
JsonObject ancestor = e.getChildren().get("ancestor");
JsonObject custom = e.getChildren().get("custom");
@@ -0,0 +1,54 @@
package org.apache.jackrabbit.oak.index.merge;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Optional;

import org.apache.commons.io.IOUtils;
import org.apache.jackrabbit.oak.commons.json.JsonObject;

public class ParameterizedMergingTestBase {

protected final String testCaseFile;
protected final String testCaseName;
protected final JsonObject buildIndexes;
protected final JsonObject runIndexes;
protected final JsonObject testCase;

public static Object[] testCase(String name, String testCaseFile) {
return new Object[] {
name,
testCaseFile
};
}

public ParameterizedMergingTestBase(String name, String testCaseFile)
throws IOException {
this.testCaseName = name;
this.testCaseFile = testCaseFile;
this.testCase = readTestCaseFile(testCaseFile);
this.buildIndexes = getTestCaseChild("build");
this.runIndexes = getTestCaseChild("run");
}

private JsonObject readTestCaseFile(String testCaseFileName) {
return Optional.ofNullable(IndexDefMergerConflictsTest.class.getResourceAsStream(testCaseFileName))
.map(in -> {
try {
return IOUtils.toString(in, StandardCharsets.UTF_8.toString());
} catch (IOException e) {
throw new IllegalArgumentException(
"Unexpected IOException reading test case file: " + testCaseFileName, e);
}
})
.map(s -> JsonObject.fromJson(s, true))
.orElseThrow(() -> new IllegalArgumentException("Unable to read test case file: " + testCaseFileName));
}

protected JsonObject getTestCaseChild(String fieldName) {
return Optional.ofNullable(testCase.getChildren().get(fieldName))
.orElseThrow(() -> new IllegalArgumentException(
"Unable to run test: " + testCaseName + ", Expected field " + fieldName + " not set"));
}

}
@@ -0,0 +1,19 @@
{
"run": {
"/oak:index/lucene": { "a": 1, "b": 10, "x": 1, "z": 2 },
"/oak:index/lucene-custom-1": { "a": 2, "b": 10, "c": 1, "x": 1 }
},
"build": {
"/oak:index/lucene-2": { "a": 1, "b": 11, "d": 100, "z": 2 }
},
"expected": {
"/oak:index/lucene-2": { "a": 1, "b": 11, "d": 100, "z": 2 },
"/oak:index/lucene-2-custom-1": {
"a": 2,
"b": 11,
"c": 1,
"d": 100,
"merges": ["/oak:index/lucene-2", "/oak:index/lucene-custom-1"]
}
}
}

0 comments on commit ae5ef0a

Please sign in to comment.