SLING-7037 : Scheduler does not retain provided name. Fix handling of name and provided name, add tests including new test case from Marcel Reutegger
git-svn-id: https://svn.apache.org/repos/asf/sling/trunk@1804434 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/pom.xml b/pom.xml
index cfe2787..e3ff731 100644
--- a/pom.xml
+++ b/pom.xml
@@ -23,7 +23,7 @@
<parent>
<groupId>org.apache.sling</groupId>
<artifactId>sling</artifactId>
- <version>30</version>
+ <version>31</version>
<relativePath />
</parent>
@@ -50,9 +50,6 @@
<extensions>true</extensions>
<configuration>
<instructions>
- <Private-Package>
- org.apache.sling.commons.scheduler.impl
- </Private-Package>
<Import-Package>
!commonj.work,
!com.mchange.v2.c3p0,
diff --git a/src/main/java/org/apache/sling/commons/scheduler/impl/InternalScheduleOptions.java b/src/main/java/org/apache/sling/commons/scheduler/impl/InternalScheduleOptions.java
index cdf58f0..a5e67c9 100644
--- a/src/main/java/org/apache/sling/commons/scheduler/impl/InternalScheduleOptions.java
+++ b/src/main/java/org/apache/sling/commons/scheduler/impl/InternalScheduleOptions.java
@@ -30,6 +30,10 @@
*/
public class InternalScheduleOptions implements ScheduleOptions {
+ public final TriggerBuilder<? extends Trigger> trigger;
+
+ public final IllegalArgumentException argumentException;
+
public String providedName;
public String name;
@@ -40,10 +44,6 @@
public Map<String, Serializable> configuration;
- public final TriggerBuilder<? extends Trigger> trigger;
-
- public final IllegalArgumentException argumentException;
-
public String[] runOn;
public InternalScheduleOptions(final TriggerBuilder<? extends Trigger> trigger) {
@@ -71,6 +71,7 @@
@Override
public ScheduleOptions name(final String name) {
this.name = name;
+ this.providedName = name;
return this;
}
diff --git a/src/main/java/org/apache/sling/commons/scheduler/impl/QuartzScheduler.java b/src/main/java/org/apache/sling/commons/scheduler/impl/QuartzScheduler.java
index 8e05f1c..795d530 100644
--- a/src/main/java/org/apache/sling/commons/scheduler/impl/QuartzScheduler.java
+++ b/src/main/java/org/apache/sling/commons/scheduler/impl/QuartzScheduler.java
@@ -75,7 +75,7 @@
static final String DATA_MAP_OBJECT = "QuartzJobScheduler.Object";
/** Map key for the provided job name */
- static final String DATA_MAP_PROVIDED_NAME = "QuartzJobScheduler.JobName";
+ static final String DATA_MAP_PROVIDED_NAME = "QuartzJobScheduler.ProvidedJobName";
/** Map key for the job name */
static final String DATA_MAP_NAME = "QuartzJobScheduler.JobName";
@@ -97,7 +97,7 @@
/** Map key for the quartz scheduler */
static final String DATA_MAP_QUARTZ_SCHEDULER = "QuartzJobScheduler.QuartzScheduler";
-
+
static final String DATA_MAP_THREAD_POOL_NAME = "QuartzJobScheduler.threadPoolName";
static final String METRICS_NAME_RUNNING_JOBS = "commons.scheduler.running.jobs";
@@ -112,11 +112,11 @@
@Reference
private ThreadPoolManager threadPoolManager;
- @Reference
+ @Reference
MetricsService metricsService;
-
+
ConfigHolder configHolder;
-
+
/** The quartz schedulers. */
private final Map<String, SchedulerProxy> schedulers = new HashMap<>();
@@ -146,7 +146,7 @@
ctx.addBundleListener(this);
this.configHolder = new ConfigHolder(configuration);
-
+
this.active = true;
}
@@ -615,7 +615,6 @@
}
synchronized ( proxy ) {
- opts.providedName = opts.name;
final String name;
if ( opts.name != null ) {
// if there is already a job with the name, remove it first
diff --git a/src/main/java/org/apache/sling/commons/scheduler/impl/WhiteboardHandler.java b/src/main/java/org/apache/sling/commons/scheduler/impl/WhiteboardHandler.java
index ed02e48..4d7c5bb 100644
--- a/src/main/java/org/apache/sling/commons/scheduler/impl/WhiteboardHandler.java
+++ b/src/main/java/org/apache/sling/commons/scheduler/impl/WhiteboardHandler.java
@@ -247,7 +247,6 @@
}
private void scheduleJob(final ServiceReference<?> ref, final Object job, final ScheduleOptions scheduleOptions) {
- ((InternalScheduleOptions)scheduleOptions).providedName = getStringProperty(ref, Scheduler.PROPERTY_SCHEDULER_NAME);
final String name = getServiceIdentifier(ref);
final Boolean concurrent = getBooleanProperty(ref, Scheduler.PROPERTY_SCHEDULER_CONCURRENT);
final String[] runOnOpts = getRunOpts(ref);
@@ -265,6 +264,7 @@
.canRunConcurrently((concurrent != null ? concurrent : true))
.threadPoolName(poolName)
.onInstancesOnly(runOnOpts);
+ ((InternalScheduleOptions)scheduleOptions).providedName = getStringProperty(ref, Scheduler.PROPERTY_SCHEDULER_NAME);
final long bundleId = ref.getBundle().getBundleId();
final Long serviceId = getLongProperty(ref, Constants.SERVICE_ID);
diff --git a/src/test/java/org/apache/sling/commons/scheduler/impl/QuartzSchedulerTest.java b/src/test/java/org/apache/sling/commons/scheduler/impl/QuartzSchedulerTest.java
index e05e5ec..4905672 100644
--- a/src/test/java/org/apache/sling/commons/scheduler/impl/QuartzSchedulerTest.java
+++ b/src/test/java/org/apache/sling/commons/scheduler/impl/QuartzSchedulerTest.java
@@ -25,9 +25,12 @@
import java.io.Serializable;
import java.lang.reflect.Field;
+import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
+import java.util.Set;
import org.apache.sling.commons.scheduler.Job;
import org.apache.sling.testing.mock.osgi.MockOsgi;
@@ -42,9 +45,11 @@
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
import org.osgi.framework.BundleEvent;
+import org.quartz.JobDetail;
import org.quartz.JobKey;
import org.quartz.SchedulerException;
import org.quartz.TriggerBuilder;
+import org.quartz.impl.matchers.GroupMatcher;
@RunWith(MockitoJUnitRunner.class)
public class QuartzSchedulerTest {
@@ -327,4 +332,39 @@
sField.set(quartzScheduler, this.proxies);
}
}
+
+ @SuppressWarnings("rawtypes")
+ @Test
+ public void testNameAndProvidedName() throws SchedulerException {
+ final Date future = new Date(System.currentTimeMillis() + 1000 * 60 * 60);
+ quartzScheduler.schedule(1L, 1L, new Thread(), quartzScheduler.AT(future).name("j1").threadPoolName("tp1"));
+ quartzScheduler.schedule(1L, 1L, new Thread(), quartzScheduler.AT(future)
+ .config(Collections.singletonMap("key", (Serializable)"value")).threadPoolName("tp1"));
+ assertNull(proxies.get("tp1"));
+ // j1 is scheduled named, so both name and provided name should be set to j1
+ JobDetail jobDetail = proxies.get("testName").getScheduler().getJobDetail(JobKey.jobKey("j1"));
+ assertEquals("j1", jobDetail.getJobDataMap().get(QuartzScheduler.DATA_MAP_NAME));
+ assertEquals("j1", jobDetail.getJobDataMap().get(QuartzScheduler.DATA_MAP_PROVIDED_NAME));
+
+ // search job detail for job without name
+ jobDetail = null;
+ org.quartz.Scheduler scheduler = quartzScheduler.getSchedulers().get("testName").getScheduler();
+ final List<String> groups = scheduler.getJobGroupNames();
+ for(final String group : groups) {
+ final Set<JobKey> keys = scheduler.getJobKeys(GroupMatcher.jobGroupEquals(group));
+ for(final JobKey key : keys) {
+ final JobDetail detail = scheduler.getJobDetail(key);
+ if ( detail != null
+ && detail.getJobDataMap().get(QuartzScheduler.DATA_MAP_CONFIGURATION) != null
+ && ((Map)detail.getJobDataMap().get(QuartzScheduler.DATA_MAP_CONFIGURATION)).get("key").equals("value")) {
+ jobDetail = detail;
+ break;
+ }
+ }
+ }
+ // provided name should be null, name is generated
+ assertNotNull(jobDetail);
+ assertNull(jobDetail.getJobDataMap().get(QuartzScheduler.DATA_MAP_PROVIDED_NAME));
+ assertNotNull(jobDetail.getJobDataMap().get(QuartzScheduler.DATA_MAP_NAME));
+ }
}
diff --git a/src/test/java/org/apache/sling/commons/scheduler/impl/WhiteboardHandlerTest.java b/src/test/java/org/apache/sling/commons/scheduler/impl/WhiteboardHandlerTest.java
index d490d8a..742a73e 100644
--- a/src/test/java/org/apache/sling/commons/scheduler/impl/WhiteboardHandlerTest.java
+++ b/src/test/java/org/apache/sling/commons/scheduler/impl/WhiteboardHandlerTest.java
@@ -16,12 +16,15 @@
*/
package org.apache.sling.commons.scheduler.impl;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import java.lang.reflect.Field;
import java.util.Dictionary;
import java.util.Hashtable;
+import java.util.List;
+import java.util.Set;
import org.apache.sling.commons.scheduler.Scheduler;
import org.apache.sling.testing.mock.osgi.MockOsgi;
@@ -32,8 +35,10 @@
import org.osgi.framework.Constants;
import org.osgi.framework.ServiceReference;
import org.osgi.framework.ServiceRegistration;
+import org.quartz.JobDetail;
import org.quartz.JobKey;
import org.quartz.SchedulerException;
+import org.quartz.impl.matchers.GroupMatcher;
public class WhiteboardHandlerTest {
private WhiteboardHandler handler;
@@ -113,6 +118,71 @@
assertNull(quartzScheduler.getSchedulers().get("testName").getScheduler().getJobDetail(jobKey));
}
+ // SLING-7037
+ @Test
+ public void testAddingServiceWithProvidedName() throws SchedulerException {
+ Thread service = new Thread();
+ String schedulerName = "testScheduler";
+ Long period = 1L;
+ Integer times = 2;
+
+ Dictionary<String, Object> serviceProps = new Hashtable<>();
+ serviceProps.put(Scheduler.PROPERTY_SCHEDULER_RUN_ON, Scheduler.VALUE_RUN_ON_LEADER);
+ serviceProps.put(Scheduler.PROPERTY_SCHEDULER_CONCURRENT, Boolean.FALSE);
+ serviceProps.put(Scheduler.PROPERTY_SCHEDULER_IMMEDIATE, Boolean.FALSE);
+ serviceProps.put(Scheduler.PROPERTY_SCHEDULER_NAME, schedulerName);
+ serviceProps.put(Scheduler.PROPERTY_SCHEDULER_PERIOD, period);
+ serviceProps.put(Scheduler.PROPERTY_SCHEDULER_TIMES, times);
+ serviceProps.put(Constants.SERVICE_PID, "1");
+
+ final ServiceRegistration<?> reg = context.registerService(Runnable.class.getName(), service, serviceProps);
+ final ServiceReference<?> reference = reg.getReference();
+ handler.register(reference, service);
+ JobKey jobKey = JobKey.jobKey(schedulerName + "." + reference.getProperty(Constants.SERVICE_ID));
+
+ JobDetail jobDetail = quartzScheduler.getSchedulers().get("testName").getScheduler().getJobDetail(jobKey);
+ assertNotNull(jobDetail);
+ assertEquals(schedulerName, jobDetail.getJobDataMap().getString(QuartzScheduler.DATA_MAP_PROVIDED_NAME));
+ assertEquals(schedulerName + "." + reference.getProperty(Constants.SERVICE_ID),
+ jobDetail.getJobDataMap().getString(QuartzScheduler.DATA_MAP_NAME));
+ }
+
+ @Test
+ public void testAddingServiceWithoutProvidedName() throws SchedulerException {
+ Thread service = new Thread();
+ Long period = 1L;
+ Integer times = 2;
+
+ Dictionary<String, Object> serviceProps = new Hashtable<>();
+ serviceProps.put(Scheduler.PROPERTY_SCHEDULER_RUN_ON, Scheduler.VALUE_RUN_ON_LEADER);
+ serviceProps.put(Scheduler.PROPERTY_SCHEDULER_CONCURRENT, Boolean.FALSE);
+ serviceProps.put(Scheduler.PROPERTY_SCHEDULER_IMMEDIATE, Boolean.FALSE);
+ serviceProps.put(Scheduler.PROPERTY_SCHEDULER_PERIOD, period);
+ serviceProps.put(Scheduler.PROPERTY_SCHEDULER_TIMES, times);
+ serviceProps.put(Constants.SERVICE_PID, "1");
+
+ final ServiceRegistration<?> reg = context.registerService(Runnable.class.getName(), service, serviceProps);
+ final ServiceReference<?> reference = reg.getReference();
+ handler.register(reference, service);
+
+ JobDetail jobDetail = null;
+ org.quartz.Scheduler scheduler = quartzScheduler.getSchedulers().get("testName").getScheduler();
+ final List<String> groups = scheduler.getJobGroupNames();
+ for(final String group : groups) {
+ final Set<JobKey> keys = scheduler.getJobKeys(GroupMatcher.jobGroupEquals(group));
+ for(final JobKey key : keys) {
+ final JobDetail detail = scheduler.getJobDetail(key);
+ if ( detail != null && detail.getJobDataMap().get(QuartzScheduler.DATA_MAP_SERVICE_ID).equals(reg.getReference().getProperty(Constants.SERVICE_ID))) {
+ jobDetail = detail;
+ break;
+ }
+ }
+ }
+ assertNotNull(jobDetail);
+ assertNull(jobDetail.getJobDataMap().getString(QuartzScheduler.DATA_MAP_PROVIDED_NAME));
+ assertNotNull(jobDetail.getJobDataMap().getString(QuartzScheduler.DATA_MAP_NAME));
+ }
+
@After
public void deactivateScheduler() {
quartzScheduler.deactivate(context);
