Skip to content

Commit 67a216f

Browse files
committed
Prevent NPE in async installation
This happened if the job was scheduled on another server. This closes #820
1 parent db925a4 commit 67a216f

File tree

2 files changed

+52
-3
lines changed

2 files changed

+52
-3
lines changed

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/impl/AcInstallationServiceImpl.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ public class AcInstallationServiceImpl implements AcInstallationService, AcInsta
147147
private ExtendedSlingSettingsService slingSettingsService;
148148

149149
@Reference(policyOption = ReferencePolicyOption.GREEDY)
150-
private JobManager jobManager;
150+
JobManager jobManager;
151151

152152
private PersistableInstallationLogger asyncInstallLog;
153153

@@ -210,6 +210,10 @@ public String applyAsynchronously(InstallationOptions options) {
210210
public JobResult process(Job job) {
211211
// cannot use deserialization due to https://issues.apache.org/jira/browse/SLING-12745
212212
Map<String, Object> jobProperties = job.getPropertyNames().stream().collect(Collectors.toMap(Function.identity(), job::getProperty));
213+
if (asyncInstallLog == null) {
214+
LOG.warn("Job {} was scheduled on another instance, no async install log available. Creating a new one.", job.getId());
215+
asyncInstallLog = new PersistableInstallationLogger();
216+
}
213217
InstallationOptionsBuilder optionsBuilder = new InstallationOptionsBuilder(jobProperties);
214218
InstallationOptions options = optionsBuilder.build();
215219
apply(options, asyncInstallLog);

accesscontroltool-bundle/src/test/java/biz/netcentric/cq/tools/actool/impl/AcInstallationServiceImplTest.java

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package biz.netcentric.cq.tools.actool.impl;
22

3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
35
/*-
46
* #%L
57
* Access Control Tool Bundle
@@ -17,23 +19,31 @@
1719
import static org.mockito.Mockito.when;
1820

1921
import java.util.Arrays;
22+
import java.util.HashMap;
2023
import java.util.Iterator;
2124
import java.util.List;
25+
import java.util.Map;
2226

2327
import javax.jcr.RepositoryException;
2428

2529
import org.apache.jackrabbit.api.security.user.Authorizable;
2630
import org.apache.jackrabbit.api.security.user.Group;
2731
import org.apache.jackrabbit.api.security.user.User;
32+
import org.apache.sling.event.jobs.Job;
33+
import org.apache.sling.event.jobs.JobBuilder;
34+
import org.apache.sling.event.jobs.JobManager;
2835
import org.junit.jupiter.api.Test;
2936
import org.junit.jupiter.api.extension.ExtendWith;
3037
import org.mockito.Mock;
38+
import org.mockito.Mockito;
3139
import org.mockito.invocation.InvocationOnMock;
3240
import org.mockito.junit.jupiter.MockitoExtension;
3341
import org.mockito.stubbing.Answer;
3442

43+
import biz.netcentric.cq.tools.actool.api.InstallationOptionsBuilder;
44+
3545
@ExtendWith(MockitoExtension.class)
36-
public class AcInstallationServiceImplTest {
46+
class AcInstallationServiceImplTest {
3747

3848
AcInstallationServiceImpl acInstallationServiceImpl = new AcInstallationServiceImpl();
3949

@@ -65,7 +75,7 @@ public class AcInstallationServiceImplTest {
6575
User user2;
6676

6777
@Test
68-
public void testSortAuthorizablesForDeletion() throws RepositoryException {
78+
void testSortAuthorizablesForDeletion() throws RepositoryException {
6979

7080
when(group1.getID()).thenReturn("group1");
7181
when(group2.getID()).thenReturn("group2");
@@ -129,4 +139,39 @@ public Iterator<Group> answer(InvocationOnMock invocation) throws Throwable {
129139
return groups.iterator();
130140
}
131141
}
142+
143+
@Test
144+
void testAsynchronousInstallationInDistributedSetups() {
145+
JobManager jobManager = Mockito.mock(JobManager.class);
146+
Job job = Mockito.mock(Job.class);
147+
JobBuilder jobBuilder = Mockito.mock(JobBuilder.class);
148+
Map<String, Object> jobProps = new HashMap<String, Object>();
149+
150+
when(jobManager.createJob(Mockito.anyString())).thenReturn(jobBuilder);
151+
when(jobBuilder.add()).thenReturn(job);
152+
when(job.getId()).thenReturn("id");
153+
when(jobBuilder.properties(Mockito.any())).thenAnswer(new Answer<JobBuilder>() {
154+
@Override
155+
public JobBuilder answer(InvocationOnMock invocation) throws Throwable {
156+
Map<String, String> props = invocation.getArgument(0);
157+
jobProps.putAll(props);
158+
return jobBuilder;
159+
}
160+
});
161+
when(jobManager.createJob(Mockito.anyString())).thenReturn(jobBuilder);
162+
when(job.getPropertyNames()).thenReturn(jobProps.keySet());
163+
when(job.getProperty(Mockito.anyString())).thenAnswer(new Answer<Object>() {
164+
@Override
165+
public Object answer(InvocationOnMock invocation) throws Throwable {
166+
return jobProps.get(invocation.getArgument(0));
167+
}
168+
});
169+
170+
acInstallationServiceImpl.jobManager = jobManager;
171+
// schedule in one server
172+
assertEquals("id", acInstallationServiceImpl.applyAsynchronously(new InstallationOptionsBuilder().withConfigurationRootPath("/apps").build()));
173+
// and process in another server (with another instance of the service)
174+
AcInstallationServiceImpl acInstallationServiceImpl2 = new AcInstallationServiceImpl();
175+
acInstallationServiceImpl2.process(job);
176+
}
132177
}

0 commit comments

Comments
 (0)