You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by an...@apache.org on 2022/06/03 12:24:14 UTC
[sling-org-apache-sling-jcr-resource] branch SLING-11367 created (now 2ee4abf)
This is an automated email from the ASF dual-hosted git repository.
angela pushed a change to branch SLING-11367
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-resource.git
at 2ee4abf SLING-11367 : Potential NPE as ResolveContext.getProviderState() is nullable
This branch includes the following new commits:
new 2ee4abf SLING-11367 : Potential NPE as ResolveContext.getProviderState() is nullable
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[sling-org-apache-sling-jcr-resource] 01/01: SLING-11367 : Potential NPE as ResolveContext.getProviderState() is nullable
Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
angela pushed a commit to branch SLING-11367
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-resource.git
commit 2ee4abf3238ff4c21f20a3eb8ff4277c74ade5b2
Author: angela <an...@adobe.com>
AuthorDate: Fri Jun 3 14:24:01 2022 +0200
SLING-11367 : Potential NPE as ResolveContext.getProviderState() is nullable
---
.../helper/jcr/BasicQueryLanguageProvider.java | 15 +--
.../resource/internal/helper/jcr/ContextUtil.java | 54 ++++++++++
.../internal/helper/jcr/JcrResourceProvider.java | 47 ++++----
.../internal/helper/jcr/ContextUtilTest.java | 120 +++++++++++++++++++++
4 files changed, 208 insertions(+), 28 deletions(-)
diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/BasicQueryLanguageProvider.java b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/BasicQueryLanguageProvider.java
index d3d2118..3d44ff2 100644
--- a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/BasicQueryLanguageProvider.java
+++ b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/BasicQueryLanguageProvider.java
@@ -44,6 +44,9 @@ import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static org.apache.sling.jcr.resource.internal.helper.jcr.ContextUtil.getHelperData;
+import static org.apache.sling.jcr.resource.internal.helper.jcr.ContextUtil.getSession;
+
public class BasicQueryLanguageProvider implements QueryLanguageProvider<JcrProviderState> {
private final Logger logger = LoggerFactory.getLogger(this.getClass());
@@ -65,24 +68,24 @@ public class BasicQueryLanguageProvider implements QueryLanguageProvider<JcrProv
}
@Override
- public String[] getSupportedLanguages(final ResolveContext<JcrProviderState> ctx) {
+ public String[] getSupportedLanguages(final @NotNull ResolveContext<JcrProviderState> ctx) {
try {
- return ctx.getProviderState().getSession().getWorkspace().getQueryManager().getSupportedQueryLanguages();
+ return getSession(ctx).getWorkspace().getQueryManager().getSupportedQueryLanguages();
} catch (final RepositoryException e) {
throw new SlingException("Unable to discover supported query languages", e);
}
}
@Override
- public Iterator<Resource> findResources(final ResolveContext<JcrProviderState> ctx,
+ public Iterator<Resource> findResources(final @NotNull ResolveContext<JcrProviderState> ctx,
final String query,
final String language) {
try {
- final QueryResult res = JcrResourceUtil.query(ctx.getProviderState().getSession(), query, language);
+ final QueryResult res = JcrResourceUtil.query(getSession(ctx), query, language);
return new JcrNodeResourceIterator(ctx.getResourceResolver(),
null, null,
res.getNodes(),
- ctx.getProviderState().getHelperData(),
+ getHelperData(ctx),
this.providerContext.getExcludedPaths());
} catch (final javax.jcr.query.InvalidQueryException iqe) {
throw new QuerySyntaxException(iqe.getMessage(), query, language, iqe);
@@ -98,7 +101,7 @@ public class BasicQueryLanguageProvider implements QueryLanguageProvider<JcrProv
final String queryLanguage = ArrayUtils.contains(getSupportedLanguages(ctx), language) ? language : DEFAULT_QUERY_LANGUAGE;
try {
- final QueryResult result = JcrResourceUtil.query(ctx.getProviderState().getSession(), query, queryLanguage);
+ final QueryResult result = JcrResourceUtil.query(getSession(ctx), query, queryLanguage);
return new ValueMapIterator(result.getColumnNames(), result.getRows());
} catch (final javax.jcr.query.InvalidQueryException iqe) {
throw new QuerySyntaxException(iqe.getMessage(), query, language, iqe);
diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/ContextUtil.java b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/ContextUtil.java
new file mode 100644
index 0000000..b4a7a8b
--- /dev/null
+++ b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/ContextUtil.java
@@ -0,0 +1,54 @@
+/*
+ * 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.sling.jcr.resource.internal.helper.jcr;
+
+import org.apache.sling.jcr.resource.internal.HelperData;
+import org.apache.sling.spi.resource.provider.ResolveContext;
+import org.jetbrains.annotations.NotNull;
+
+import javax.jcr.Session;
+
+final class ContextUtil {
+
+ private ContextUtil() {}
+
+ static @NotNull Session getSession(@NotNull ResolveContext<JcrProviderState> ctx) {
+ return getProviderState(ctx).getSession();
+ }
+
+ static @NotNull JcrItemResourceFactory getResourceFactory(@NotNull ResolveContext<JcrProviderState> ctx) {
+ return getProviderState(ctx).getResourceFactory();
+ }
+
+ static @NotNull HelperData getHelperData(@NotNull ResolveContext<JcrProviderState> ctx) {
+ return getProviderState(ctx).getHelperData();
+ }
+
+ /**
+ * As long as the provider is active there must be a state available.
+ *
+ * @param ctx the {@code ResolveContext}
+ * @return the {@code JcrProviderState} associated with the given context.
+ */
+ private static @NotNull JcrProviderState getProviderState(@NotNull ResolveContext<JcrProviderState> ctx) {
+ JcrProviderState state = ctx.getProviderState();
+ if (state == null) {
+ throw new IllegalStateException("Cannot retrieve JcrProviderState from ResolveContext.");
+ }
+ return state;
+ }
+}
\ No newline at end of file
diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java
index 57713b4..60b6b5a 100644
--- a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java
+++ b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java
@@ -75,6 +75,10 @@ import org.osgi.service.component.annotations.ReferencePolicy;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static org.apache.sling.jcr.resource.internal.helper.jcr.ContextUtil.getHelperData;
+import static org.apache.sling.jcr.resource.internal.helper.jcr.ContextUtil.getResourceFactory;
+import static org.apache.sling.jcr.resource.internal.helper.jcr.ContextUtil.getSession;
+
@Component(name="org.apache.sling.jcr.resource.internal.helper.jcr.JcrResourceProviderFactory",
service = ResourceProvider.class,
property = {
@@ -229,8 +233,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
this.repository);
for (final ObserverConfiguration config : this.getProviderContext().getObservationReporter().getObserverConfigurations()) {
logger.debug("Registering listener for {}", config.getPaths());
- final Closeable listener = new JcrResourceListener(this.listenerConfig,
- config);
+ final Closeable listener = new JcrResourceListener(this.listenerConfig, config);
this.listeners.put(config, listener);
}
} catch (final RepositoryException e) {
@@ -319,7 +322,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
@Override
public boolean isLive(final @NotNull ResolveContext<JcrProviderState> ctx) {
- return ctx.getProviderState().getSession().isLive();
+ return getSession(ctx).isLive();
}
@Override
@@ -327,7 +330,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
public Resource getResource(@NotNull ResolveContext<JcrProviderState> ctx, @NotNull String path,
@NotNull ResourceContext rCtx, @Nullable Resource parent) {
try {
- return ctx.getProviderState().getResourceFactory().createResource(ctx.getResourceResolver(), path, parent, rCtx.getResolveParameters());
+ return getResourceFactory(ctx).createResource(ctx.getResourceResolver(), path, parent, rCtx.getResolveParameters());
} catch (RepositoryException e) {
throw new SlingException("Can't get resource", e);
}
@@ -345,7 +348,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
// try to get the JcrItemResource for the parent path to list
// children
try {
- parentItemResource = ctx.getProviderState().getResourceFactory().createResource(
+ parentItemResource = getResourceFactory(ctx).createResource(
parent.getResourceResolver(), parent.getPath(), null,
parent.getResourceMetadata().getParameterMap());
} catch (RepositoryException re) {
@@ -370,9 +373,9 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
String parentPath = ResourceUtil.getParent(child.getPath());
if (parentPath != null) {
Item childItem = ((JcrItemResource) child).getItem();
- Node parentNode = ctx.getProviderState().getResourceFactory().getParentOrNull(childItem, parentPath);
+ Node parentNode = getResourceFactory(ctx).getParentOrNull(childItem, parentPath);
if (parentNode != null) {
- return new JcrNodeResource(ctx.getResourceResolver(), parentPath, null, parentNode, ctx.getProviderState().getHelperData());
+ return new JcrNodeResource(ctx.getResourceResolver(), parentPath, null, parentNode, getHelperData(ctx));
}
}
}
@@ -385,7 +388,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
@NotNull
public Collection<String> getAttributeNames(final @NotNull ResolveContext<JcrProviderState> ctx) {
final Set<String> names = new HashSet<>();
- final String[] sessionNames = ctx.getProviderState().getSession().getAttributeNames();
+ final String[] sessionNames = getSession(ctx).getAttributeNames();
for (final String name : sessionNames) {
if (isAttributeVisible(name)) {
names.add(name);
@@ -399,9 +402,9 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
public Object getAttribute(final @NotNull ResolveContext<JcrProviderState> ctx, final @NotNull String name) {
if (isAttributeVisible(name)) {
if (ResourceResolverFactory.USER.equals(name)) {
- return ctx.getProviderState().getSession().getUserID();
+ return getSession(ctx).getUserID();
}
- return ctx.getProviderState().getSession().getAttribute(name);
+ return getSession(ctx).getAttribute(name);
}
return null;
}
@@ -422,7 +425,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
final String resourceType = rtObj.toString();
if (resourceType.indexOf(':') != -1 && resourceType.indexOf('/') == -1) {
try {
- ctx.getProviderState().getSession().getWorkspace().getNodeTypeManager().getNodeType(resourceType);
+ getSession(ctx).getWorkspace().getNodeTypeManager().getNodeType(resourceType);
isNodeType = true;
} catch (final RepositoryException ignore) {
// we expect this, if this isn't a valid node type, therefore ignoring
@@ -443,9 +446,9 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
final int lastPos = path.lastIndexOf('/');
final Node parent;
if (lastPos == 0) {
- parent = ctx.getProviderState().getSession().getRootNode();
+ parent = getSession(ctx).getRootNode();
} else {
- parent = (Node) ctx.getProviderState().getSession().getItem(path.substring(0, lastPos));
+ parent = (Node) getSession(ctx).getItem(path.substring(0, lastPos));
}
final String name = path.substring(lastPos + 1);
if (nodeType != null) {
@@ -456,7 +459,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
if (properties != null) {
// create modifiable map
- final JcrModifiableValueMap jcrMap = new JcrModifiableValueMap(node, ctx.getProviderState().getHelperData());
+ final JcrModifiableValueMap jcrMap = new JcrModifiableValueMap(node, getHelperData(ctx));
// check mixin types first
final Object value = properties.get(JcrConstants.JCR_MIXINTYPES);
if (value != null) {
@@ -478,7 +481,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
}
}
- return new JcrNodeResource(ctx.getResourceResolver(), path, null, node, ctx.getProviderState().getHelperData());
+ return new JcrNodeResource(ctx.getResourceResolver(), path, null, node, getHelperData(ctx));
} catch (final RepositoryException e) {
throw new PersistenceException("Unable to create node at " + path, e, path, null);
}
@@ -533,7 +536,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
logger.debug("delete: {} maps to an empty JCR path", resource.getPath());
throw new PersistenceException("Unable to delete resource", null, resource.getPath(), null);
}
- item = ctx.getProviderState().getSession().getItem(jcrPath);
+ item = getSession(ctx).getItem(jcrPath);
}
item.remove();
} catch (final RepositoryException e) {
@@ -544,7 +547,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
@Override
public void revert(final @NotNull ResolveContext<JcrProviderState> ctx) {
try {
- ctx.getProviderState().getSession().refresh(false);
+ getSession(ctx).refresh(false);
} catch (final RepositoryException ignore) {
logger.warn("Unable to revert pending changes.", ignore);
}
@@ -553,7 +556,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
@Override
public void commit(final @NotNull ResolveContext<JcrProviderState> ctx) throws PersistenceException {
try {
- ctx.getProviderState().getSession().save();
+ getSession(ctx).save();
} catch (final RepositoryException e) {
throw new PersistenceException("Unable to commit changes to session.", e);
}
@@ -562,7 +565,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
@Override
public boolean hasChanges(final @NotNull ResolveContext<JcrProviderState> ctx) {
try {
- return ctx.getProviderState().getSession().hasPendingChanges();
+ return getSession(ctx).hasPendingChanges();
} catch (final RepositoryException ignore) {
logger.warn("Unable to check session for pending changes.", ignore);
}
@@ -572,7 +575,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
@Override
public void refresh(final @NotNull ResolveContext<JcrProviderState> ctx) {
try {
- ctx.getProviderState().getSession().refresh(true);
+ getSession(ctx).refresh(true);
} catch (final RepositoryException ignore) {
logger.warn("Unable to refresh session.", ignore);
}
@@ -582,7 +585,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
@Override
public @Nullable <AdapterType> AdapterType adaptTo(final @NotNull ResolveContext<JcrProviderState> ctx,
final @NotNull Class<AdapterType> type) {
- Session session = ctx.getProviderState().getSession();
+ Session session = getSession(ctx);
if (type == Session.class) {
return (AdapterType) session;
} else if (type == Principal.class) {
@@ -619,7 +622,7 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> {
final String srcNodePath = srcAbsPath;
final String dstNodePath = destAbsPath + '/' + ResourceUtil.getName(srcAbsPath);
try {
- ctx.getProviderState().getSession().move(srcNodePath, dstNodePath);
+ getSession(ctx).move(srcNodePath, dstNodePath);
return true;
} catch (final RepositoryException e) {
throw new PersistenceException("Unable to move resource to " + destAbsPath, e, srcAbsPath, null);
diff --git a/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/ContextUtilTest.java b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/ContextUtilTest.java
new file mode 100644
index 0000000..dbf0cf3
--- /dev/null
+++ b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/ContextUtilTest.java
@@ -0,0 +1,120 @@
+/*
+ * 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.sling.jcr.resource.internal.helper.jcr;
+
+import org.apache.sling.jcr.resource.internal.HelperData;
+import org.apache.sling.spi.resource.provider.ResolveContext;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import javax.jcr.Session;
+import java.util.concurrent.atomic.AtomicReference;
+
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.Mockito.when;
+
+public class ContextUtilTest {
+
+ private JcrProviderState state;
+ private ResolveContext<JcrProviderState> ctx;
+
+ @Before
+ public void before() {
+ state = mock(JcrProviderState.class);
+ ctx = when(mock(ResolveContext.class).getProviderState()).thenReturn(state).getMock();
+ }
+
+ @After
+ public void after() {
+ reset(state, ctx);
+ }
+
+ @Test
+ public void testGetHelperData() {
+ HelperData hd = new HelperData(new AtomicReference<>(), new AtomicReference<>());
+ when(state.getHelperData()).thenReturn(hd);
+
+ assertSame(hd, ContextUtil.getHelperData(ctx));
+
+ verify(state).getHelperData();
+ verify(ctx).getProviderState();
+ verifyNoMoreInteractions(ctx, state);
+ }
+
+ @Test
+ public void testGetJcrItemResourceFactory() {
+ JcrItemResourceFactory rf = mock(JcrItemResourceFactory.class);
+ when(state.getResourceFactory()).thenReturn(rf);
+
+ assertSame(rf, ContextUtil.getResourceFactory(ctx));
+
+ verify(state).getResourceFactory();
+ verify(ctx).getProviderState();
+ verifyNoMoreInteractions(ctx, state);
+ }
+
+ @Test
+ public void testGetSession() {
+ Session session = mock(Session.class);
+ when(state.getSession()).thenReturn(session);
+
+ assertSame(session, ContextUtil.getSession(ctx));
+
+ verify(state).getSession();
+ verify(ctx).getProviderState();
+ verifyNoMoreInteractions(ctx, state);
+ }
+
+ @Test
+ public void testNullProviderState() {
+ when(ctx.getProviderState()).thenReturn(null);
+
+ try {
+ ContextUtil.getHelperData(ctx);
+ fail("IllegalStateException expected");
+ } catch (IllegalStateException e) {
+ // success
+ }
+
+ try {
+ ContextUtil.getResourceFactory(ctx);
+ fail("IllegalStateException expected");
+ } catch (IllegalStateException e) {
+ // success
+ }
+
+ try {
+ ContextUtil.getSession(ctx);
+ fail("IllegalStateException expected");
+ } catch (IllegalStateException e) {
+ // success
+ }
+
+ verify(ctx, times(3)).getProviderState();
+ verifyNoMoreInteractions(ctx);
+ verifyZeroInteractions(state);
+ }
+
+}
\ No newline at end of file