tmvlad commented on code in PR #2926:
URL: https://github.com/apache/jackrabbit-oak/pull/2926#discussion_r3346860365
##########
oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java:
##########
@@ -1187,4 +1194,146 @@ private String getCommitPath(String changeNodeName) {
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();
Review Comment:
The provider class doesn't get the toggle from the constructor (only the
editor does). It has the toggle name hardcoded in the `activate` method, where
only the osgiContext is needed, for the creating the whiteboard with the shared
state.
--
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]
