thomasmueller commented on code in PR #2751:
URL: https://github.com/apache/jackrabbit-oak/pull/2751#discussion_r2851365598
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/diff/DiffIndexMerger.java:
##########
@@ -669,33 +712,68 @@ private static void addPrimaryType(String path,
JsonObject json) {
/**
* Merge a JSON diff into a target index definition.
*
- * @param path the path
+ * @param indexName the index name (for logging)
+ * @param path the path (relative to the index)
* @param diff the diff (what to merge)
* @param target where to merge into
+ * @param isNew whether the target node is newly created (didn't exist
before)
*/
- private void mergeInto(String path, JsonObject diff, JsonObject target) {
+ private void mergeInto(String indexName, String path, JsonObject diff,
JsonObject target,
+ boolean isNewNode) {
+ String pathForLogging = path.isEmpty() ? "the root" : "relative path "
+ path;
for (String p : diff.getProperties().keySet()) {
if (path.isEmpty()) {
if ("jcr:primaryType".equals(p)) {
continue;
}
}
+ if (!isNewNode) {
+ // for existing nodes, we do a few more checks before the merge
+ if (path.isEmpty() &&
REJECTED_TOP_LEVEL_PROPS_FOR_EXISTING_INDEX.contains(p)
+ && !target.getProperties().containsKey(p)) {
+ // at the top level, some properties (eg. selectionPolicy)
are not allowed to be added
+ // to an existing index
+ logWarn("{}: Ignoring new top-level property {} at {} for
existing index", indexName, p, pathForLogging);
+ continue;
+ }
+ if (REJECTED_ADDING_TO_EXISTING_PROPERTY.contains(p) &&
!target.getProperties().containsKey(p)) {
+ // some properties are not allowed to be added if the node
already exists
+ logWarn("{}: Ignoring new property \"{}\" at {} for
existing child", indexName, p, pathForLogging);
+ continue;
+ }
+ }
if (target.getProperties().containsKey(p)) {
- // we do not currently allow to overwrite most existing
properties
- if (p.equals("boost")) {
- // allow overwriting the boost value
- LOG.info("Overwrite property {} value at {}", p, path);
+ // we do not currently allow to overwrite most existing
properties,
+ // except for:
+ if (!path.isEmpty() &&
ALLOW_CHANGING_IN_EXISTING_PROPERTY.contains(p)) {
+ // allow overwriting the (eg.) boost value
target.getProperties().put(p, diff.getProperties().get(p));
+ } else if (path.isEmpty() &&
MERGE_MULTI_VALUES_STRINGS.contains(p)) {
+ // merge includedPaths, queryPaths, and tags,
+ // such that it contains more entries
+ // (if the property is not set, we would make it more
restrictive,
+ // which is not allowed)
+ TreeSet<String> oldSet =
JsonNodeUpdater.getStringSet(target.getProperties().get(p));
+ TreeSet<String> newSet =
JsonNodeUpdater.getStringSet(diff.getProperties().get(p));
+ TreeSet<String> mergedSet = new TreeSet<String>(oldSet);
+ mergedSet.addAll(newSet);
Review Comment:
User can not remove properties, and that's by design. (That would break
existing queries.)
--
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]
