Merge pull request #117 from jmmut/feature/dont-reuse-parameters

EVA-741 The parameters from completed executions should not be reused

src/main/java/uk/ac/ebi/eva/pipeline/jobs/AggregatedVcfJob.java

@@ -35,6 +35,7 @@
 import uk.ac.ebi.eva.pipeline.jobs.flows.AnnotationFlowOptional;
 import uk.ac.ebi.eva.pipeline.jobs.steps.LoadFileStep;
 import uk.ac.ebi.eva.pipeline.jobs.steps.VariantLoaderStep;
+import uk.ac.ebi.eva.pipeline.parameters.NewJobIncrementer;
 import uk.ac.ebi.eva.pipeline.parameters.validation.job.AggregatedVcfJobParametersValidator;
 
 import static uk.ac.ebi.eva.pipeline.configuration.BeanNames.AGGREGATED_VCF_JOB;
@@ -76,7 +77,7 @@ public Job aggregatedVcfJob(JobBuilderFactory jobBuilderFactory) {
 
         JobBuilder jobBuilder = jobBuilderFactory
                 .get(AGGREGATED_VCF_JOB)
-                .incrementer(new RunIdIncrementer())
+                .incrementer(new NewJobIncrementer())
                 .validator(new AggregatedVcfJobParametersValidator());
         FlowJobBuilder builder = jobBuilder
                 .flow(variantLoaderStep)

src/main/java/uk/ac/ebi/eva/pipeline/jobs/AnnotationJob.java

@@ -23,7 +23,6 @@
 import org.springframework.batch.core.configuration.annotation.JobBuilderFactory;
 import org.springframework.batch.core.job.builder.JobBuilder;
 import org.springframework.batch.core.job.flow.Flow;
-import org.springframework.batch.core.launch.support.RunIdIncrementer;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.context.annotation.Bean;
@@ -32,6 +31,7 @@
 import org.springframework.context.annotation.Scope;
 
 import uk.ac.ebi.eva.pipeline.jobs.flows.AnnotationFlow;
+import uk.ac.ebi.eva.pipeline.parameters.NewJobIncrementer;
 
 import static uk.ac.ebi.eva.pipeline.configuration.BeanNames.ANNOTATE_VARIANTS_JOB;
 import static uk.ac.ebi.eva.pipeline.configuration.BeanNames.VEP_ANNOTATION_FLOW;
@@ -67,7 +67,7 @@ public Job annotateVariantsJob(JobBuilderFactory jobBuilderFactory) {
 
         JobBuilder jobBuilder = jobBuilderFactory
                 .get(ANNOTATE_VARIANTS_JOB)
-                .incrementer(new RunIdIncrementer());
+                .incrementer(new NewJobIncrementer());
         return jobBuilder.start(annotation).build().build();
     }
 

src/main/java/uk/ac/ebi/eva/pipeline/jobs/DatabaseInitializationJob.java

@@ -32,6 +32,7 @@
 
 import uk.ac.ebi.eva.pipeline.jobs.steps.CreateDatabaseIndexesStep;
 import uk.ac.ebi.eva.pipeline.jobs.steps.GeneLoaderStep;
+import uk.ac.ebi.eva.pipeline.parameters.NewJobIncrementer;
 
 import static uk.ac.ebi.eva.pipeline.configuration.BeanNames.CREATE_DATABASE_INDEXES_STEP;
 import static uk.ac.ebi.eva.pipeline.configuration.BeanNames.GENES_LOAD_STEP;
@@ -67,7 +68,7 @@ public Job initDatabaseJob(JobBuilderFactory jobBuilderFactory) {
 
         JobBuilder jobBuilder = jobBuilderFactory
                 .get(INIT_DATABASE_JOB)
-                .incrementer(new RunIdIncrementer());
+                .incrementer(new NewJobIncrementer());
 
         return jobBuilder
                 .start(createDatabaseIndexesStep)

src/main/java/uk/ac/ebi/eva/pipeline/jobs/DropStudyJob.java

@@ -35,6 +35,7 @@
 import uk.ac.ebi.eva.pipeline.jobs.steps.DropFilesByStudyStep;
 import uk.ac.ebi.eva.pipeline.jobs.steps.DropSingleStudyVariantsStep;
 import uk.ac.ebi.eva.pipeline.jobs.steps.PullFilesAndStatisticsByStudyStep;
+import uk.ac.ebi.eva.pipeline.parameters.NewJobIncrementer;
 import uk.ac.ebi.eva.pipeline.parameters.validation.job.DropStudyJobParametersValidator;
 
 import static uk.ac.ebi.eva.pipeline.configuration.BeanNames.DROP_FILES_BY_STUDY_STEP;
@@ -73,7 +74,7 @@ public Job dropStudyJob(JobBuilderFactory jobBuilderFactory) {
 
         JobBuilder jobBuilder = jobBuilderFactory
                 .get(DROP_STUDY_JOB)
-                .incrementer(new RunIdIncrementer())
+                .incrementer(new NewJobIncrementer())
                 .validator(new DropStudyJobParametersValidator());
 
         SimpleJobBuilder builder = jobBuilder

src/main/java/uk/ac/ebi/eva/pipeline/jobs/GenotypedVcfJob.java

@@ -35,6 +35,7 @@
 import uk.ac.ebi.eva.pipeline.jobs.flows.ParallelStatisticsAndAnnotationFlow;
 import uk.ac.ebi.eva.pipeline.jobs.steps.LoadFileStep;
 import uk.ac.ebi.eva.pipeline.jobs.steps.VariantLoaderStep;
+import uk.ac.ebi.eva.pipeline.parameters.NewJobIncrementer;
 import uk.ac.ebi.eva.pipeline.parameters.validation.job.GenotypedVcfJobParametersValidator;
 
 import static uk.ac.ebi.eva.pipeline.configuration.BeanNames.GENOTYPED_VCF_JOB;
@@ -77,7 +78,7 @@ public Job genotypedVcfJob(JobBuilderFactory jobBuilderFactory) {
 
         JobBuilder jobBuilder = jobBuilderFactory
                 .get(GENOTYPED_VCF_JOB)
-                .incrementer(new RunIdIncrementer())
+                .incrementer(new NewJobIncrementer())
                 .validator(new GenotypedVcfJobParametersValidator());
         FlowJobBuilder builder = jobBuilder
                 .flow(variantLoaderStep)

src/main/java/uk/ac/ebi/eva/pipeline/jobs/PopulationStatisticsJob.java

@@ -31,6 +31,7 @@
 import org.springframework.context.annotation.Scope;
 
 import uk.ac.ebi.eva.pipeline.jobs.flows.PopulationStatisticsFlow;
+import uk.ac.ebi.eva.pipeline.parameters.NewJobIncrementer;
 
 import static uk.ac.ebi.eva.pipeline.configuration.BeanNames.CALCULATE_STATISTICS_FLOW;
 import static uk.ac.ebi.eva.pipeline.configuration.BeanNames.CALCULATE_STATISTICS_JOB;
@@ -58,7 +59,7 @@ public Job calculateStatisticsJob(JobBuilderFactory jobBuilderFactory) {
 
         JobBuilder jobBuilder = jobBuilderFactory
                 .get(CALCULATE_STATISTICS_JOB)
-                .incrementer(new RunIdIncrementer());
+                .incrementer(new NewJobIncrementer());
 
         return jobBuilder
                 .start(optionalStatisticsFlow)

src/main/java/uk/ac/ebi/eva/pipeline/parameters/NewJobIncrementer.java

@@ -0,0 +1,50 @@
+/*
+ * Copyright 2015-2017 EMBL - European Bioinformatics Institute
+ *
+ * Licensed 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 uk.ac.ebi.eva.pipeline.parameters;
+
+import org.springframework.batch.core.JobParameters;
+import org.springframework.batch.core.JobParametersBuilder;
+import org.springframework.batch.core.JobParametersIncrementer;
+
+/**
+ * Incrementer that does not reuse parameters from previous completed executions.
+ */
+public class NewJobIncrementer implements JobParametersIncrementer {
+
+    private static String RUN_ID_KEY = "run.id";
+
+    private String key = RUN_ID_KEY;
+
+    /**
+     * The name of the run id in the job parameters.  Defaults to "run.id".
+     *
+     * @param key the key to set
+     */
+    public void setKey(String key) {
+        this.key = key;
+    }
+
+    /**
+     * Increment the run.id parameter (starting with 1), and ignore the other parameters.
+     */
+    @Override
+    public JobParameters getNext(JobParameters parameters) {
+        JobParameters params = (parameters == null) ? new JobParameters() : parameters;
+
+        long id = params.getLong(key, 0L) + 1;
+        return new JobParametersBuilder().addLong(key, id).toJobParameters();
+    }
+}

src/test/java/uk/ac/ebi/eva/pipeline/jobs/AggregatedVcfJobTest.java

@@ -59,6 +59,8 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static uk.ac.ebi.eva.test.utils.JobTestUtils.assertCompleted;
+import static uk.ac.ebi.eva.test.utils.JobTestUtils.assertFailed;
 import static uk.ac.ebi.eva.utils.FileUtils.getResource;
 
 /**
@@ -111,11 +113,8 @@ public void aggregatedTransformAndLoadShouldBeExecuted() throws Exception {
                 .toJobParameters();
         JobExecution jobExecution = jobLauncherTestUtils.launchJob(jobParameters);
 
-        assertEquals(ExitStatus.COMPLETED, jobExecution.getExitStatus());
-        assertEquals(BatchStatus.COMPLETED, jobExecution.getStatus());
-
         // check execution flow
-        assertEquals(ExitStatus.COMPLETED, jobExecution.getExitStatus());
+        assertCompleted(jobExecution);
 
         Collection<StepExecution> stepExecutions = jobExecution.getStepExecutions();
         Set<String> names = stepExecutions.stream().map(StepExecution::getStepName)
@@ -161,7 +160,6 @@ public void aggregationNoneIsNotAllowed() throws Exception {
                 .toJobParameters();
         JobExecution jobExecution = jobLauncherTestUtils.launchJob(jobParameters);
 
-        assertEquals(ExitStatus.FAILED, jobExecution.getExitStatus());
-        assertEquals(BatchStatus.FAILED, jobExecution.getStatus());
+        assertFailed(jobExecution);
     }
 }

src/test/java/uk/ac/ebi/eva/pipeline/jobs/AnnotationJobTest.java

@@ -41,6 +41,7 @@
 import uk.ac.ebi.eva.test.configuration.BatchTestConfiguration;
 import uk.ac.ebi.eva.test.rules.PipelineTemporaryFolderRule;
 import uk.ac.ebi.eva.test.rules.TemporaryMongoRule;
+import uk.ac.ebi.eva.test.utils.JobTestUtils;
 import uk.ac.ebi.eva.utils.EvaJobParameterBuilder;
 import uk.ac.ebi.eva.utils.URLHelper;
 
@@ -54,6 +55,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static uk.ac.ebi.eva.test.utils.JobTestUtils.assertCompleted;
 import static uk.ac.ebi.eva.test.utils.TestFileUtils.getResourceUrl;
 import static uk.ac.ebi.eva.utils.FileUtils.getResource;
 
@@ -114,8 +116,7 @@ public void allAnnotationStepsShouldBeExecuted() throws Exception {
 
         JobExecution jobExecution = jobLauncherTestUtils.launchJob(jobParameters);
 
-        assertEquals(ExitStatus.COMPLETED, jobExecution.getExitStatus());
-        assertEquals(BatchStatus.COMPLETED, jobExecution.getStatus());
+        assertCompleted(jobExecution);
 
         assertEquals(3, jobExecution.getStepExecutions().size());
         List<StepExecution> steps = new ArrayList<>(jobExecution.getStepExecutions());
@@ -176,8 +177,7 @@ public void noVariantsToAnnotateOnlyGenerateAnnotationStepShouldRun() throws Exc
 
         JobExecution jobExecution = jobLauncherTestUtils.launchJob(jobParameters);
 
-        assertEquals(ExitStatus.COMPLETED, jobExecution.getExitStatus());
-        assertEquals(BatchStatus.COMPLETED, jobExecution.getStatus());
+        assertCompleted(jobExecution);
 
         assertEquals(1, jobExecution.getStepExecutions().size());
         StepExecution findVariantsToAnnotateStep = new ArrayList<>(jobExecution.getStepExecutions()).get(0);

src/test/java/uk/ac/ebi/eva/pipeline/jobs/DropStudyJobTest.java

@@ -42,6 +42,7 @@
 import static uk.ac.ebi.eva.test.utils.DropStudyJobTestUtils.assertDropFiles;
 import static uk.ac.ebi.eva.test.utils.DropStudyJobTestUtils.assertDropSingleStudy;
 import static uk.ac.ebi.eva.test.utils.DropStudyJobTestUtils.assertPullStudy;
+import static uk.ac.ebi.eva.test.utils.JobTestUtils.assertCompleted;
 
 /**
  * Test for {@link PopulationStatisticsJob}
@@ -99,8 +100,7 @@ public void fullDropStudyJob() throws Exception {
                 .toJobParameters();
 
         JobExecution jobExecution = jobLauncherTestUtils.launchJob(jobParameters);
-        assertEquals(ExitStatus.COMPLETED, jobExecution.getExitStatus());
-        assertEquals(BatchStatus.COMPLETED, jobExecution.getStatus());
+        assertCompleted(jobExecution);
 
         assertDropSingleStudy(variantsCollection, STUDY_ID_TO_DROP, EXPECTED_VARIANTS_AFTER_DROP_STUDY);
         assertPullStudy(variantsCollection, STUDY_ID_TO_DROP, EXPECTED_FILE_COUNT, EXPECTED_STATS_COUNT);

src/test/java/uk/ac/ebi/eva/pipeline/jobs/GenotypedVcfJobTest.java

@@ -36,11 +36,14 @@
 import uk.ac.ebi.eva.test.rules.PipelineTemporaryFolderRule;
 import uk.ac.ebi.eva.test.rules.TemporaryMongoRule;
 import uk.ac.ebi.eva.test.utils.GenotypedVcfJobTestUtils;
+import uk.ac.ebi.eva.test.utils.JobTestUtils;
 import uk.ac.ebi.eva.utils.EvaJobParameterBuilder;
 
 import java.io.File;
 
 import static org.junit.Assert.assertEquals;
+import static uk.ac.ebi.eva.test.utils.JobTestUtils.assertCompleted;
+import static uk.ac.ebi.eva.test.utils.JobTestUtils.assertFailed;
 
 /**
  * Test for {@link GenotypedVcfJob}
@@ -108,8 +111,7 @@ public void fullGenotypedVcfJob() throws Exception {
 
         JobExecution jobExecution = jobLauncherTestUtils.launchJob(jobParameters);
 
-        assertEquals(ExitStatus.COMPLETED, jobExecution.getExitStatus());
-        assertEquals(BatchStatus.COMPLETED, jobExecution.getStatus());
+        assertCompleted(jobExecution);
 
         GenotypedVcfJobTestUtils.checkLoadStep(databaseName);
 
@@ -163,7 +165,6 @@ public void aggregationIsNotAllowed() throws Exception {
                 .toJobParameters();
         JobExecution jobExecution = jobLauncherTestUtils.launchJob(jobParameters);
 
-        assertEquals(ExitStatus.FAILED, jobExecution.getExitStatus());
-        assertEquals(BatchStatus.FAILED, jobExecution.getStatus());
+        assertFailed(jobExecution);
     }
 }

src/test/java/uk/ac/ebi/eva/pipeline/jobs/GenotypedVcfJobWorkflowTest.java

@@ -52,6 +52,7 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static uk.ac.ebi.eva.test.utils.JobTestUtils.assertCompleted;
 import static uk.ac.ebi.eva.utils.FileUtils.getResource;
 
 /**
@@ -101,8 +102,7 @@ public void allStepsShouldBeExecuted() throws Exception {
         JobParameters jobParameters = builder.toJobParameters();
 
         JobExecution execution = jobLauncherTestUtils.launchJob(jobParameters);
-
-        assertEquals(ExitStatus.COMPLETED, execution.getExitStatus());
+        assertCompleted(execution);
 
         Collection<StepExecution> stepExecutions = execution.getStepExecutions();
         Map<String, StepExecution> nameToStepExecution = stepExecutions.stream().collect(
@@ -138,8 +138,7 @@ public void optionalStepsShouldBeSkipped() throws Exception {
         JobParameters jobParameters = builder.annotationSkip(true).statisticsSkip(true).toJobParameters();
 
         JobExecution execution = jobLauncherTestUtils.launchJob(jobParameters);
-
-        assertEquals(ExitStatus.COMPLETED, execution.getExitStatus());
+        assertCompleted(execution);
 
         Set<String> names = execution.getStepExecutions().stream().map(StepExecution::getStepName)
                 .collect(Collectors.toSet());
@@ -153,8 +152,7 @@ public void statsStepsShouldBeSkipped() throws Exception {
         JobParameters jobParameters = builder.statisticsSkip(true).toJobParameters();
 
         JobExecution execution = jobLauncherTestUtils.launchJob(jobParameters);
-
-        assertEquals(ExitStatus.COMPLETED, execution.getExitStatus());
+        assertCompleted(execution);
 
         Collection<StepExecution> stepExecutions = execution.getStepExecutions();
         Map<String, StepExecution> nameToStepExecution = stepExecutions.stream().collect(
@@ -185,8 +183,7 @@ public void annotationStepsShouldBeSkipped() throws Exception {
         JobParameters jobParameters = builder.annotationSkip(true).toJobParameters();
 
         JobExecution execution = jobLauncherTestUtils.launchJob(jobParameters);
-
-        assertEquals(ExitStatus.COMPLETED, execution.getExitStatus());
+        assertCompleted(execution);
 
         Collection<StepExecution> stepExecutions = execution.getStepExecutions();
         Map<String, StepExecution> nameToStepExecution = stepExecutions.stream().collect(

src/test/java/uk/ac/ebi/eva/pipeline/jobs/PopulationStatisticsJobTest.java

@@ -36,13 +36,15 @@
 import uk.ac.ebi.eva.test.configuration.BatchTestConfiguration;
 import uk.ac.ebi.eva.test.rules.PipelineTemporaryFolderRule;
 import uk.ac.ebi.eva.test.rules.TemporaryMongoRule;
+import uk.ac.ebi.eva.test.utils.JobTestUtils;
 import uk.ac.ebi.eva.utils.EvaJobParameterBuilder;
 import uk.ac.ebi.eva.utils.URLHelper;
 
 import java.io.File;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static uk.ac.ebi.eva.test.utils.JobTestUtils.assertCompleted;
 import static uk.ac.ebi.eva.test.utils.TestFileUtils.getResourceUrl;
 import static uk.ac.ebi.eva.utils.FileUtils.getResource;
 
@@ -88,8 +90,7 @@ public void fullPopulationStatisticsJob() throws Exception {
                 .toJobParameters();
 
         JobExecution jobExecution = jobLauncherTestUtils.launchJob(jobParameters);
-        assertEquals(ExitStatus.COMPLETED, jobExecution.getExitStatus());
-        assertEquals(BatchStatus.COMPLETED, jobExecution.getStatus());
+        assertCompleted(jobExecution);
 
         //and the file containing statistics should exist
         File statsFile = new File(URLHelper.getVariantsStatsUri(statsDir, studyId, fileId));

src/test/java/uk/ac/ebi/eva/pipeline/jobs/steps/AnnotationLoaderStepTest.java

@@ -42,13 +42,15 @@
 import uk.ac.ebi.eva.test.data.VepOutputContent;
 import uk.ac.ebi.eva.test.rules.PipelineTemporaryFolderRule;
 import uk.ac.ebi.eva.test.rules.TemporaryMongoRule;
+import uk.ac.ebi.eva.test.utils.JobTestUtils;
 import uk.ac.ebi.eva.utils.EvaJobParameterBuilder;
 import uk.ac.ebi.eva.utils.URLHelper;
 
 import java.nio.file.Paths;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static uk.ac.ebi.eva.test.utils.JobTestUtils.assertCompleted;
 import static uk.ac.ebi.eva.test.utils.TestFileUtils.getResourceUrl;
 
 
@@ -94,8 +96,7 @@ public void shouldLoadAllAnnotations() throws Exception {
 
         JobExecution jobExecution = jobLauncherTestUtils.launchStep(BeanNames.LOAD_VEP_ANNOTATION_STEP, jobParameters);
 
-        assertEquals(ExitStatus.COMPLETED, jobExecution.getExitStatus());
-        assertEquals(BatchStatus.COMPLETED, jobExecution.getStatus());
+        assertCompleted(jobExecution);
 
         //check that documents have the annotation
         DBCursor cursor = mongoRule.getCollection(dbName, collectionVariantsName).find();