You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by th...@apache.org on 2026/06/16 12:05:28 UTC
(jackrabbit-oak) branch trunk updated: OAK-12125: gracefully handle property index missing propertyNames (#2924) (#2926)
This is an automated email from the ASF dual-hosted git repository.
thomasmueller pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new 27e009ef0e OAK-12125: gracefully handle property index missing propertyNames (#2924) (#2926)
27e009ef0e is described below
commit 27e009ef0ecea5415fb9ec8f0f90bda3c5e6f91e
Author: Thomas Mueller <th...@apache.org>
AuthorDate: Tue Jun 16 14:05:22 2026 +0200
OAK-12125: gracefully handle property index missing propertyNames (#2924) (#2926)
* OAK-12125: gracefully handle property index missing propertyNames (#2924)
A missing 'propertyNames' caused an NPE that escaped IndexUpdate's
IllegalStateException catch and broke the async indexing cycle. Add an
opt-in fallback gated by FT_GRANITE-63829 (registered in @Activate, driven
by LaunchDarkly in production): when on, the editor logs one WARN per
misconfigured index path and uses a sentinel property name so the cycle
continues. Default off preserves the pre-PR behavior.
Co-authored-by: Theia Vlad <tv...@Theias-MacBook-Pro.local>
* OAK-12125: rename feature toggle to use the Oak ticket id (#2929)
Co-authored-by: Theia Vlad <tv...@Theias-MacBook-Pro.local>
---------
Co-authored-by: Theia Vlad <39...@users.noreply.github.com>
Co-authored-by: Theia Vlad <tv...@Theias-MacBook-Pro.local>
---
.../index/property/PropertyIndexEditor.java | 23 +++-
.../property/PropertyIndexEditorProvider.java | 30 ++++-
.../plugins/index/property/PropertyIndexTest.java | 149 +++++++++++++++++++++
3 files changed, 199 insertions(+), 3 deletions(-)
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java
index 69e9f429bc..72acc2937e 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java
@@ -34,6 +34,7 @@ import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;
import java.util.function.Supplier;
@@ -42,6 +43,7 @@ import javax.jcr.PropertyType;
import org.apache.jackrabbit.oak.api.CommitFailedException;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.commons.collections.SetUtils;
+import org.apache.jackrabbit.oak.plugins.index.ContextAwareCallback;
import org.apache.jackrabbit.oak.plugins.index.IndexConstants;
import org.apache.jackrabbit.oak.plugins.index.IndexEditor;
import org.apache.jackrabbit.oak.plugins.index.IndexUpdateCallback;
@@ -53,7 +55,9 @@ import org.apache.jackrabbit.oak.spi.filter.PathFilter;
import org.apache.jackrabbit.oak.spi.mount.MountInfoProvider;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.apache.jackrabbit.oak.spi.toggle.Feature;
import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -67,6 +71,9 @@ class PropertyIndexEditor implements IndexEditor {
private static final Logger log = LoggerFactory.getLogger(PropertyIndexEditor.class);
+ private static final String MISSING_PROPERTY_NAMES_SENTINEL = "propertyNamesIsMissing";
+ private static final Set<String> WARNED_INDEX_PATHS = ConcurrentHashMap.newKeySet();
+
/** Parent editor, or {@code null} if this is the root editor. */
private final PropertyIndexEditor parent;
@@ -119,7 +126,8 @@ class PropertyIndexEditor implements IndexEditor {
private final MountInfoProvider mountInfoProvider;
public PropertyIndexEditor(NodeBuilder definition, NodeState root,
- IndexUpdateCallback updateCallback, MountInfoProvider mountInfoProvider) {
+ IndexUpdateCallback updateCallback, MountInfoProvider mountInfoProvider,
+ @Nullable Feature feature) {
this.parent = null;
this.name = null;
this.path = "/";
@@ -132,7 +140,18 @@ class PropertyIndexEditor implements IndexEditor {
// get property names
PropertyState names = definition.getProperty(PROPERTY_NAMES);
- if (names.count() == 1) {
+ if (names == null && feature != null && feature.isEnabled()) {
+ String indexPath = (updateCallback instanceof ContextAwareCallback)
+ ? ((ContextAwareCallback) updateCallback).getIndexingContext().getIndexPath()
+ : null;
+ String warnKey = indexPath != null ? indexPath : "(unknown index path)";
+ if (WARNED_INDEX_PATHS.add(warnKey)) {
+ log.warn("Property index definition at {} is missing required property '{}'. " +
+ "Falling back to a no-op sentinel; please fix the index definition.",
+ warnKey, PROPERTY_NAMES);
+ }
+ this.propertyNames = singleton(MISSING_PROPERTY_NAMES_SENTINEL);
+ } else if (names.count() == 1) {
// OAK-1273: optimize for the common case
this.propertyNames = singleton(names.getValue(NAME, 0));
} else {
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditorProvider.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditorProvider.java
index 433e6b1ba9..506fd2a940 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditorProvider.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditorProvider.java
@@ -16,6 +16,9 @@
*/
package org.apache.jackrabbit.oak.plugins.index.property;
+import static org.apache.jackrabbit.oak.spi.toggle.Feature.newFeature;
+
+import org.apache.jackrabbit.oak.osgi.OsgiWhiteboard;
import org.apache.jackrabbit.oak.plugins.index.IndexConstants;
import org.apache.jackrabbit.oak.plugins.index.IndexEditorProvider;
import org.apache.jackrabbit.oak.spi.commit.Editor;
@@ -24,8 +27,14 @@ import org.apache.jackrabbit.oak.spi.mount.MountInfoProvider;
import org.apache.jackrabbit.oak.spi.mount.Mounts;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.apache.jackrabbit.oak.spi.toggle.Feature;
+import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard;
import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.osgi.framework.BundleContext;
+import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Deactivate;
import org.osgi.service.component.annotations.Reference;
/**
@@ -42,14 +51,33 @@ public class PropertyIndexEditorProvider implements IndexEditorProvider {
public static final String TYPE = "property";
+ public static final String FT_OAK_12125 = "FT_OAK-12125";
+
@Reference
private MountInfoProvider mountInfoProvider = Mounts.defaultMountInfoProvider();
+ @Nullable
+ private Feature feature;
+
+ @Activate
+ private void activate(BundleContext bundleContext) {
+ Whiteboard whiteboard = new OsgiWhiteboard(bundleContext);
+ this.feature = newFeature(FT_OAK_12125, whiteboard);
+ }
+
+ @Deactivate
+ private void deactivate() {
+ if (feature != null) {
+ feature.close();
+ feature = null;
+ }
+ }
+
@Override
public Editor getIndexEditor(
@NotNull String type, @NotNull NodeBuilder definition, @NotNull NodeState root, @NotNull IndexUpdateCallback callback) {
if (TYPE.equals(type)) {
- return new PropertyIndexEditor(definition, root, callback, mountInfoProvider);
+ return new PropertyIndexEditor(definition, root, callback, mountInfoProvider, feature);
}
return null;
}
diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java
index 7296028873..a9a551c8d5 100644
--- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java
+++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java
@@ -74,8 +74,12 @@ import org.apache.jackrabbit.oak.spi.mount.Mounts;
import org.apache.jackrabbit.oak.spi.query.Filter;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.apache.jackrabbit.oak.spi.toggle.FeatureToggle;
+import org.apache.sling.testing.mock.osgi.MockOsgi;
+import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
import org.hamcrest.core.IsCollectionContaining;
import org.junit.Assert;
+import org.junit.Rule;
import org.junit.Test;
import org.slf4j.LoggerFactory;
@@ -92,6 +96,9 @@ import ch.qos.logback.core.spi.FilterReply;
*/
public class PropertyIndexTest {
+ @Rule
+ public final OsgiContext osgiContext = new OsgiContext();
+
private static final int MANY = 100;
private static final EditorHook HOOK = new EditorHook(
@@ -1187,4 +1194,146 @@ public class PropertyIndexTest {
return PathUtils.concat(path, changeNodeName);
}
}
+
+ /**
+ * @see <a href="https://issues.apache.org/jira/browse/OAK-12125">OAK-12125</a>
+ *
+ * With no Feature wired into the provider (the default), a property index
+ * definition missing the required 'propertyNames' must NPE out of the editor
+ * constructor. IndexUpdate catches IllegalStateException but NOT NullPointerException,
+ * so the NPE propagates and breaks the indexing cycle. This pins the
+ * "default OFF = pre-PR behavior" contract.
+ */
+ @Test
+ public void missingPropertyNames_toggleOff_breaksIndexing() throws Exception {
+ NodeState root = INITIAL_CONTENT;
+ NodeBuilder builder = root.builder();
+
+ NodeBuilder invalidIndex = builder.child(INDEX_DEFINITIONS_NAME).child("invalidIndex_off");
+ invalidIndex.setProperty(JCR_PRIMARYTYPE,
+ IndexConstants.INDEX_DEFINITIONS_NODE_TYPE, Type.NAME);
+ invalidIndex.setProperty(IndexConstants.TYPE_PROPERTY_NAME,
+ PropertyIndexEditorProvider.TYPE);
+
+ NodeState before = builder.getNodeState();
+ builder.child("content").setProperty("foo", "bar");
+ NodeState after = builder.getNodeState();
+
+ try {
+ HOOK.processCommit(before, after, CommitInfo.EMPTY);
+ fail("Expected NullPointerException to propagate from the indexing cycle");
+ } catch (NullPointerException expected) {
+ // success path: NPE bubbled directly
+ } catch (Exception other) {
+ // Some Oak commit-hook layers wrap exceptions; accept NPE anywhere in cause chain
+ Throwable cause = other;
+ boolean foundNpe = false;
+ while (cause != null) {
+ if (cause instanceof NullPointerException) {
+ foundNpe = true;
+ break;
+ }
+ cause = cause.getCause();
+ }
+ assertTrue("Expected NullPointerException in the cause chain, got: " + other, foundNpe);
+ }
+ }
+
+ /**
+ * @see <a href="https://issues.apache.org/jira/browse/OAK-12125">OAK-12125</a>
+ *
+ * With an enabled Feature wired into the provider, a missing 'propertyNames'
+ * must NOT throw. Instead the editor falls back to a sentinel, logs one WARN
+ * naming the misconfigured index path, and lets the indexing cycle complete
+ * so the valid sibling index still works.
+ */
+ @Test
+ public void missingPropertyNames_toggleOn_logsWarnAndContinues() throws Exception {
+ EditorHook hook = hookWithFeatureEnabled(true);
+ LogCustomizer customLogs = LogCustomizer
+ .forLogger(PropertyIndexEditor.class.getName()).enable(Level.WARN).create();
+ try {
+ customLogs.starting();
+
+ NodeState root = INITIAL_CONTENT;
+ NodeBuilder builder = root.builder();
+
+ createIndexDefinition(builder.child(INDEX_DEFINITIONS_NAME), "foo",
+ true, false, Set.of("foo"), null);
+
+ NodeBuilder invalidIndex = builder.child(INDEX_DEFINITIONS_NAME).child("invalidIndex_on");
+ invalidIndex.setProperty(JCR_PRIMARYTYPE,
+ IndexConstants.INDEX_DEFINITIONS_NODE_TYPE, Type.NAME);
+ invalidIndex.setProperty(IndexConstants.TYPE_PROPERTY_NAME,
+ PropertyIndexEditorProvider.TYPE);
+
+ NodeState before = builder.getNodeState();
+ builder.child("content").setProperty("foo", "bar");
+ NodeState after = builder.getNodeState();
+
+ NodeState indexed = hook.processCommit(before, after, CommitInfo.EMPTY);
+
+ assertTrue("Expected a WARN message naming the invalid index path",
+ customLogs.getLogs().stream().anyMatch(
+ msg -> msg.contains("invalidIndex_on") && msg.contains(IndexConstants.PROPERTY_NAMES)));
+
+ FilterImpl f = createFilter(indexed, NT_BASE);
+ PropertyIndexLookup lookup = new PropertyIndexLookup(indexed);
+ assertEquals(Set.of("content"), find(lookup, "foo", "bar", f));
+ } finally {
+ customLogs.finished();
+ }
+ }
+
+ /**
+ * @see <a href="https://issues.apache.org/jira/browse/OAK-12125">OAK-12125</a>
+ *
+ * With the Feature enabled and the same invalid index hit twice, exactly one
+ * WARN must be emitted for that index path. Uses a unique index node name
+ * ("invalidIndex_dedup") so the JVM-wide dedup set doesn't get poisoned by
+ * earlier tests in the suite.
+ */
+ @Test
+ public void missingPropertyNames_toggleOn_warnLoggedOncePerIndexPath() throws Exception {
+ EditorHook hook = hookWithFeatureEnabled(true);
+ LogCustomizer customLogs = LogCustomizer
+ .forLogger(PropertyIndexEditor.class.getName()).enable(Level.WARN).create();
+ try {
+ customLogs.starting();
+
+ NodeState root = INITIAL_CONTENT;
+ NodeBuilder builder = root.builder();
+
+ NodeBuilder invalidIndex = builder.child(INDEX_DEFINITIONS_NAME).child("invalidIndex_dedup");
+ invalidIndex.setProperty(JCR_PRIMARYTYPE,
+ IndexConstants.INDEX_DEFINITIONS_NODE_TYPE, Type.NAME);
+ invalidIndex.setProperty(IndexConstants.TYPE_PROPERTY_NAME,
+ PropertyIndexEditorProvider.TYPE);
+
+ NodeState before = builder.getNodeState();
+ builder.child("content_a").setProperty("foo", "bar");
+ NodeState after1 = builder.getNodeState();
+ hook.processCommit(before, after1, CommitInfo.EMPTY);
+
+ builder.child("content_b").setProperty("foo", "baz");
+ NodeState after2 = builder.getNodeState();
+ hook.processCommit(after1, after2, CommitInfo.EMPTY);
+
+ long warnsForThisIndex = customLogs.getLogs().stream()
+ .filter(msg -> msg.contains("invalidIndex_dedup"))
+ .count();
+ assertEquals("Expected exactly one WARN for invalidIndex_dedup across two cycles, got: "
+ + customLogs.getLogs(), 1, warnsForThisIndex);
+ } finally {
+ customLogs.finished();
+ }
+ }
+
+ private EditorHook hookWithFeatureEnabled(boolean enabled) {
+ PropertyIndexEditorProvider provider = new PropertyIndexEditorProvider();
+ MockOsgi.activate(provider, osgiContext.bundleContext());
+ FeatureToggle toggle = osgiContext.getService(FeatureToggle.class);
+ toggle.setEnabled(enabled);
+ return new EditorHook(new IndexUpdateProvider(provider));
+ }
}