SLING-11367 : Potential NPE as ResolveContext.getProviderState() is nullable
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.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 @@
}
@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 @@
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.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 @@
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 @@
@Override
public boolean isLive(final @NotNull ResolveContext<JcrProviderState> ctx) {
- return ctx.getProviderState().getSession().isLive();
+ return getSession(ctx).isLive();
}
@Override
@@ -327,7 +330,7 @@
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 @@
// 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 @@
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 @@
@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 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 @@
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 @@
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 @@
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 @@
}
}
- 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 @@
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 @@
@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 @@
@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 @@
@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 @@
@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 @@
@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 @@
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
