Skip to content

Commit

Permalink
Code review updates
Browse files Browse the repository at this point in the history
  • Loading branch information
maallen committed Aug 25, 2023
1 parent ca4950a commit 5b7fd33
Show file tree
Hide file tree
Showing 22 changed files with 322 additions and 238 deletions.
193 changes: 98 additions & 95 deletions docker/docker-compose-api-worker.yml

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ void startSchedulers() throws SchedulerException {
for (Scheduler scheduler : schedulerManager.getSchedulers()) {
logger.info("Starting scheduler: {}", scheduler.getSchedulerName());
scheduler.startDelayed(delay);
// Increment the delay to avoid lock exceptions being thrown as both schedulers try to start
// concurrently
delay++;
}
}
Expand Down
14 changes: 14 additions & 0 deletions webapp/src/main/java/com/box/l10n/mojito/quartz/QuartzJobInfo.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.box.l10n.mojito.quartz;

import static com.box.l10n.mojito.quartz.QuartzSchedulerManager.DEFAULT_SCHEDULER_NAME;

import java.util.Date;
import org.quartz.JobBuilder;

Expand All @@ -14,6 +16,7 @@ public class QuartzJobInfo<I, O> {
boolean inlineInput;
long timeout;
boolean requestRecovery;
String scheduler;

private QuartzJobInfo(Builder<I, O> builder) {
clazz = builder.clazz;
Expand All @@ -26,6 +29,7 @@ private QuartzJobInfo(Builder<I, O> builder) {
inlineInput = builder.inlineInput;
timeout = builder.timeout;
requestRecovery = builder.requestRecovery;
scheduler = builder.scheduler;
}

public Class<? extends QuartzPollableJob<I, O>> getClazz() {
Expand Down Expand Up @@ -68,6 +72,10 @@ public boolean getRequestRecovery() {
return requestRecovery;
}

public String getScheduler() {
return scheduler;
}

public static <I, O> Builder<I, O> newBuilder(Class<? extends QuartzPollableJob<I, O>> clazz) {
Builder<I, O> builder = new Builder<I, O>();
builder.clazz = clazz;
Expand All @@ -85,6 +93,7 @@ public static final class Builder<I, O> {
private boolean inlineInput = true;
private long timeout = 3600;
private boolean requestRecovery = false;
private String scheduler = DEFAULT_SCHEDULER_NAME;

private Builder() {}

Expand Down Expand Up @@ -128,6 +137,11 @@ public Builder<I, O> withTimeout(long val) {
return this;
}

public Builder<I, O> withScheduler(String val) {
scheduler = val;
return this;
}

/** As defined in {@link JobBuilder#requestRecovery(boolean)} */
public Builder<I, O> withRequestRecovery(boolean requestRecovery) {
this.requestRecovery = requestRecovery;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.box.l10n.mojito.quartz;

import static com.box.l10n.mojito.quartz.QuartzConfig.DYNAMIC_GROUP_NAME;
import static com.box.l10n.mojito.quartz.QuartzSchedulerManager.DEFAULT_SCHEDULER_NAME;

import com.box.l10n.mojito.entity.PollableTask;
import com.box.l10n.mojito.json.ObjectMapper;
Expand Down Expand Up @@ -42,7 +41,7 @@ public <I, O> PollableFuture<O> scheduleJob(
Class<? extends QuartzPollableJob<I, O>> clazz, I input) {
QuartzJobInfo<I, O> quartzJobInfo =
QuartzJobInfo.newBuilder(clazz).withInput(input).withMessage(clazz.getSimpleName()).build();
return scheduleJob(quartzJobInfo, DEFAULT_SCHEDULER_NAME);
return scheduleJob(quartzJobInfo);
}

public <I, O> PollableFuture<O> scheduleJobWithCustomTimeout(
Expand All @@ -54,11 +53,7 @@ public <I, O> PollableFuture<O> scheduleJobWithCustomTimeout(
.withMessage(clazz.getSimpleName())
.build();

return scheduleJob(quartzJobInfo, DEFAULT_SCHEDULER_NAME);
}

public <I, O> PollableFuture<O> scheduleJob(QuartzJobInfo<I, O> quartzJobInfo) {
return scheduleJob(quartzJobInfo, DEFAULT_SCHEDULER_NAME);
return scheduleJob(quartzJobInfo);
}

/**
Expand All @@ -79,12 +74,11 @@ public <I, O> PollableFuture<O> scheduleJob(QuartzJobInfo<I, O> quartzJobInfo) {
* @param <O>
* @return
*/
public <I, O> PollableFuture<O> scheduleJob(
QuartzJobInfo<I, O> quartzJobInfo, String schedulerName) {
public <I, O> PollableFuture<O> scheduleJob(QuartzJobInfo<I, O> quartzJobInfo) {

Scheduler scheduler = schedulerManager.getScheduler(schedulerName);
logger.debug("Scheduling job on queue: {}", quartzJobInfo.getScheduler());

logger.debug("Scheduling job on queue: {}", schedulerName);
Scheduler scheduler = schedulerManager.getScheduler(quartzJobInfo.getScheduler());

String pollableTaskName = getPollableTaskName(quartzJobInfo.getClazz());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

public interface QuartzSchedulerManager {

String DEFAULT_SCHEDULER_NAME = "defaultScheduler";
String DEFAULT_SCHEDULER_NAME = "default";

Scheduler getScheduler(String schedulerName);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@

@Component
@ConditionalOnProperty(name = "l10n.org.multi-quartz.enabled", havingValue = "true")
public class QuartzMultiSchedulerFactory {
public class QuartzMultiSchedulerConfig {

Logger logger = LoggerFactory.getLogger(QuartzMultiSchedulerFactory.class);
Logger logger = LoggerFactory.getLogger(QuartzMultiSchedulerConfig.class);

@Autowired ApplicationContext applicationContext;

@Autowired QuartzMultiSchedulerConfigProperties quartzMultiSchedulerConfigProperties;
@Autowired
QuartzMultiSchedulerConfigurationProperties quartzMultiSchedulerConfigurationProperties;

@Autowired(required = false)
QuartzMetricsReportingJobListener quartzMetricsReportingJobListener;
Expand All @@ -39,7 +40,8 @@ public class QuartzMultiSchedulerFactory {
@PostConstruct
public void createSchedulers() {

for (SchedulerConfig config : quartzMultiSchedulerConfigProperties.getSchedulerConfigs()) {
for (SchedulerConfigurationProperties config :
quartzMultiSchedulerConfigurationProperties.getSchedulerConfigurationProperties()) {
Map<String, String> quartzProps = config.getQuartz();
Properties properties = new Properties();

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package com.box.l10n.mojito.quartz.multi;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.annotation.PostConstruct;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.context.annotation.Configuration;

@Configuration
@ConfigurationProperties("l10n.org.multi-quartz")
@ConditionalOnProperty(name = "l10n.org.multi-quartz.enabled", havingValue = "true")
public class QuartzMultiSchedulerConfigurationProperties {

Map<String, Map<String, String>> schedulers;

List<SchedulerConfigurationProperties> schedulerConfigurationProperties = new ArrayList<>();

public Map<String, Map<String, String>> getSchedulers() {
return schedulers;
}

public void setSchedulers(Map<String, Map<String, String>> schedulers) {
this.schedulers = schedulers;
}

public List<SchedulerConfigurationProperties> getSchedulerConfigurationProperties() {
return schedulerConfigurationProperties;
}

@PostConstruct
public void init() {
schedulerConfigurationProperties.addAll(extractSchedulerConfigs());
}

private List<SchedulerConfigurationProperties> extractSchedulerConfigs() {
List<SchedulerConfigurationProperties> schedulerProperties = new ArrayList<>();

for (Map.Entry<String, Map<String, String>> entry : schedulers.entrySet()) {
SchedulerConfigurationProperties config = new SchedulerConfigurationProperties();
config.setName(entry.getKey());
for (Map.Entry<String, String> quartzEntry : entry.getValue().entrySet()) {
config.getQuartz().put(quartzEntry.getKey().replace("quartz.", ""), quartzEntry.getValue());
}
schedulerProperties.add(config);
}
return schedulerProperties;
}
}

class SchedulerConfigurationProperties {

private String name;

private Map<String, String> quartz = new HashMap<>();

public String getName() {
return name;
}

public void setName(String name) {
this.name = name;
}

public Map<String, String> getQuartz() {
return quartz;
}

public void setQuartz(Map<String, String> quartz) {
this.quartz = quartz;
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.box.l10n.mojito.quartz.multi;

import com.box.l10n.mojito.quartz.QuartzSchedulerManager;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.annotation.PostConstruct;
Expand All @@ -15,28 +16,28 @@

@Component
@ConditionalOnProperty(name = "l10n.org.multi-quartz.enabled", havingValue = "true")
@DependsOn("quartzMultiSchedulerFactory")
@DependsOn("quartzMultiSchedulerConfig")
public class QuartzMultiSchedulerManager implements QuartzSchedulerManager {

Logger logger = LoggerFactory.getLogger(QuartzMultiSchedulerManager.class);

@Autowired List<Scheduler> schedulers;

Map<String, Scheduler> schedulerMap = new java.util.HashMap<>();
Map<String, Scheduler> schedulerByName = new HashMap<>();

@PostConstruct
public void init() throws SchedulerException {
for (Scheduler scheduler : schedulers) {
if (schedulerMap.containsKey(scheduler.getSchedulerName())) {
if (schedulerByName.containsKey(scheduler.getSchedulerName())) {
throw new QuartzMultiSchedulerException(
"Scheduler with name '"
+ scheduler.getSchedulerName()
+ "' already exists. Please configure a unique name for each scheduler");
}
schedulerMap.put(scheduler.getSchedulerName(), scheduler);
schedulerByName.put(scheduler.getSchedulerName(), scheduler);
}

if (!schedulerMap.containsKey(DEFAULT_SCHEDULER_NAME)) {
if (!schedulerByName.containsKey(DEFAULT_SCHEDULER_NAME)) {
throw new QuartzMultiSchedulerException(
"No default scheduler found. Please configure a scheduler with name '"
+ DEFAULT_SCHEDULER_NAME
Expand All @@ -47,13 +48,13 @@ public void init() throws SchedulerException {
@Override
public Scheduler getScheduler(String schedulerName) {

if (!schedulerMap.containsKey(schedulerName)) {
if (!schedulerByName.containsKey(schedulerName)) {
logger.warn(
"Scheduler with name '{}' not found, scheduling job on default scheduler", schedulerName);
schedulerName = DEFAULT_SCHEDULER_NAME;
}

return schedulerMap.get(schedulerName);
return schedulerByName.get(schedulerName);
}

@Override
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public class AssetWS {
@Autowired MeterRegistry meterRegistry;

@Value("${l10n.assetWS.quartz.schedulerName:" + DEFAULT_SCHEDULER_NAME + "}")
String jobSchedulerName;
String schedulerName;

/**
* Gets the list of {@link Asset} for a given {@link Repository} and other optional filters
Expand Down Expand Up @@ -236,9 +236,10 @@ public PollableTask getLocalizedAssetForContentAsync(
QuartzJobInfo.newBuilder(GenerateLocalizedAssetJob.class)
.withInlineInput(false)
.withInput(localizedAssetBody)
.withScheduler(schedulerName)
.build();
PollableFuture<LocalizedAssetBody> localizedAssetBodyPollableFuture =
quartzPollableTaskScheduler.scheduleJob(quartzJobInfo, jobSchedulerName);
quartzPollableTaskScheduler.scheduleJob(quartzJobInfo);
return localizedAssetBodyPollableFuture.getPollableTask();
}

Expand Down
Loading

0 comments on commit 5b7fd33

Please sign in to comment.