Skip to content

Ena 5863 updating validation scope #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Rajkumar-D wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Ena 5863 updating validation scope #148

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e6b05e3
Updating the validationScope to support different layer of processing
Rajkumar-D Oct 19, 2023
6a4becd
Updating the validationScope to support different layer of processing
Rajkumar-D Oct 19, 2023
17b6b7b
Removing ValidationScopes that is not currently used
Rajkumar-D Oct 23, 2023
f80e8e9
Removing ValidationScopes that is not currently used
Rajkumar-D Oct 23, 2023
b2b1c90
ENA-5863: Changes for validation scope
Rajkumar-D Nov 7, 2023
f96a364
changes for adding ValidationScope
Rajkumar-D Jan 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ plugins {
archivesBaseName = 'sequencetools'
group = 'uk.ac.ebi.ena.sequence'

version = '2.18.11'
version = '2.18.11.SNAPSHOT'

sourceCompatibility = 1.8
targetCompatibility = 1.8
Expand Down
48 changes: 24 additions & 24 deletions src/main/java/uk/ac/ebi/embl/api/validation/ValidationScope.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,48 +10,45 @@
*/
package uk.ac.ebi.embl.api.validation;

import java.util.Arrays;
import java.util.List;

public enum ValidationScope {
/** Putff (ENA) */
EMBL(Group.SEQUENCE),
EMBL(Group.PUTFF),
/** Putff (NCBI) */
NCBI(Group.SEQUENCE),
NCBI(Group.PUTFF, Group.NCBI),
/** Putff (NCBI master) */
NCBI_MASTER(Group.SEQUENCE),
NCBI_MASTER(Group.PUTFF, Group.NCBI),
/** Pipeline (Webin-CLI sequence scope) */
EMBL_TEMPLATE(Group.SEQUENCE),
EMBL_TEMPLATE(Group.PIPELINE),
/** Putff (patent protein) */
EPO_PEPTIDE(Group.SEQUENCE),
EPO_PEPTIDE(Group.EPO),
/** Putff (patent) */
EPO(Group.SEQUENCE),
/** TODO: remove if not used */
INSDC(Group.SEQUENCE),
/** TODO: remove if not used */
EGA(Group.SEQUENCE),
/** TODO: remove if not used */
ARRAYEXPRESS(Group.SEQUENCE),
EPO(Group.EPO),
/** Pipeline (Webin-CLI genome scope) */
ASSEMBLY_MASTER(Group.ASSEMBLY),
ASSEMBLY_MASTER(Group.ASSEMBLY, Group.PIPELINE),
/** Pipeline (Webin-CLI genome scope) */
ASSEMBLY_CONTIG(Group.ASSEMBLY),
ASSEMBLY_CONTIG(Group.ASSEMBLY, Group.PIPELINE),
/** Pipeline (Webin-CLI genome scope) */
ASSEMBLY_SCAFFOLD(Group.ASSEMBLY),
ASSEMBLY_SCAFFOLD(Group.ASSEMBLY, Group.PIPELINE),
/** Pipeline (Webin-CLI genome scope) */
ASSEMBLY_CHROMOSOME(Group.ASSEMBLY),
ASSEMBLY_CHROMOSOME(Group.ASSEMBLY, Group.PIPELINE),
/** Pipeline (Webin-CLI transcriptome scope) */
ASSEMBLY_TRANSCRIPTOME(Group.ASSEMBLY);
ASSEMBLY_TRANSCRIPTOME(Group.PIPELINE);
Copy link
Contributor

@raskoleinonen raskoleinonen Oct 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have only four groups (not sure what to call them):

  • Group.PIPELINE/WEBIN /** Webin submissions processed by pipelines. */
  • Group.EPO /** EPO submissions processed by putff. */
  • Group.NCBI /** NCBI submissions processed by putff. */
  • GROUP.PUTFF/ENA /** ENA submissions processed by putff. */

We might wish to rename:

  • EMBL_TEMPLATE -> TEMPLATE
  • ASSEMBLY_TRANSCRIPTOME -> TRANSCRIPTOME


private final Group group;
private final List<Group> groups;

ValidationScope(Group group) {
this.group = group;
ValidationScope(Group... groups) {
this.groups = Arrays.asList(groups);
}

public boolean isInGroup(Group group) {
return this.group == group;
return this.groups.contains(group);
}

public Group group() {
return this.group;
public List<Group> groups() {
return this.groups;
}

public static ValidationScope get(String scope) {
Expand All @@ -77,8 +74,11 @@ public int getAssemblyLevel() {
}

public enum Group {
PIPELINE,
Copy link
Contributor

@raskoleinonen raskoleinonen Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest to add a few comments ->

/** Webin submissions processed by pipelines. */
PIPELINE

/** Non-Webin submissions processed by putff. */
PUTFF

PUTFF,
ASSEMBLY,
SEQUENCE
NCBI,
EPO
}

public static ValidationScope getScope(FileType fileType) {
Expand Down
1 change: 0 additions & 1 deletion src/main/java/uk/ac/ebi/embl/api/validation/check/entry/DescriptionCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

@ExcludeScope(
validationScope = {
ValidationScope.ARRAYEXPRESS,
ValidationScope.ASSEMBLY_CHROMOSOME,
ValidationScope.ASSEMBLY_CONTIG,
ValidationScope.ASSEMBLY_MASTER,
Expand Down
3 changes: 0 additions & 3 deletions src/main/java/uk/ac/ebi/embl/api/validation/check/entry/ReferenceCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@
ValidationScope.EMBL_TEMPLATE,
ValidationScope.EPO_PEPTIDE,
ValidationScope.NCBI,
ValidationScope.ARRAYEXPRESS,
ValidationScope.EGA,
ValidationScope.INSDC,
ValidationScope.NCBI_MASTER
})
public class ReferenceCheck extends EntryValidationCheck {
Expand Down
3 changes: 0 additions & 3 deletions src/main/java/uk/ac/ebi/embl/api/validation/check/feature/NCBIQualifierCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,16 @@
+ "Feature qualifier \\\"{0}\\\" value \\\"{1}\\\" does not comply to the qualifier specifications. Refer to the feature documentation or ask a curator for guidance.\"")
@ExcludeScope(
validationScope = {
ValidationScope.ARRAYEXPRESS,
ValidationScope.ASSEMBLY_CHROMOSOME,
ValidationScope.ASSEMBLY_CONTIG,
ValidationScope.ASSEMBLY_MASTER,
ValidationScope.ASSEMBLY_SCAFFOLD,
ValidationScope.ASSEMBLY_TRANSCRIPTOME,
ValidationScope.EGA,
ValidationScope.EMBL,
ValidationScope.EMBL,
ValidationScope.EMBL_TEMPLATE,
ValidationScope.EPO,
ValidationScope.EPO_PEPTIDE,
ValidationScope.INSDC,
ValidationScope.NCBI_MASTER
})
public class NCBIQualifierCheck extends FeatureValidationCheck {
Expand Down
3 changes: 0 additions & 3 deletions src/main/java/uk/ac/ebi/embl/api/validation/fixer/entry/JournalFix.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,16 @@
@Description("Journal has been modified from:{0} to:{1}")
@ExcludeScope(
validationScope = {
ValidationScope.ARRAYEXPRESS,
ValidationScope.ASSEMBLY_CHROMOSOME,
ValidationScope.ASSEMBLY_CONTIG,
ValidationScope.ASSEMBLY_MASTER,
ValidationScope.ASSEMBLY_SCAFFOLD,
ValidationScope.ASSEMBLY_TRANSCRIPTOME,
ValidationScope.EGA,
ValidationScope.EMBL,
ValidationScope.EMBL,
ValidationScope.EMBL_TEMPLATE,
ValidationScope.EPO,
ValidationScope.EPO_PEPTIDE,
ValidationScope.INSDC,
ValidationScope.NCBI_MASTER
})
public class JournalFix extends EntryValidationCheck {
Expand Down
3 changes: 0 additions & 3 deletions src/main/java/uk/ac/ebi/embl/api/validation/fixer/sourcefeature/CountryQualifierFix.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,17 @@
@Description("\"{0}\" qualifier value \"{1}\" is invalid, a note has been added.")
@ExcludeScope(
validationScope = {
ValidationScope.ARRAYEXPRESS,
ValidationScope.ASSEMBLY_CHROMOSOME,
ValidationScope.ASSEMBLY_CONTIG,
ValidationScope.ASSEMBLY_MASTER,
ValidationScope.NCBI_MASTER,
ValidationScope.ASSEMBLY_SCAFFOLD,
ValidationScope.ASSEMBLY_TRANSCRIPTOME,
ValidationScope.EGA,
ValidationScope.EMBL,
ValidationScope.EMBL,
ValidationScope.EMBL_TEMPLATE,
ValidationScope.EPO,
ValidationScope.EPO_PEPTIDE,
ValidationScope.INSDC
})
public class CountryQualifierFix extends FeatureValidationCheck {
private static final String COUNTRY_QUALIFIER_VALUE_FIX_ID_1 = "CountryQualifierFix_1";
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/uk/ac/ebi/embl/api/validation/plan/ValidationPlan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ protected boolean isInValidationScopeGroup(ValidationScope.Group[] validationSco
if (groupScope == null) {
continue;
}
if (groupScope.equals(validationScope.group())) return true;
if (validationScope.groups().contains(groupScope)) return true;
}
return false;
}
Expand Down
10 changes: 10 additions & 0 deletions src/test/java/uk/ac/ebi/embl/api/validation/ValidationMessageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
package uk.ac.ebi.embl.api.validation;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

import java.io.IOException;
import java.io.StringWriter;
Expand Down Expand Up @@ -122,4 +123,13 @@ public void testWriteTextMessageChangedFormatter() throws IOException {
ValidationMessage.setDefaultMessageFormatter(mf);
}
}

@Test
public void testValidationScopeWithGroup() {
assertTrue(ValidationScope.NCBI.isInGroup(ValidationScope.Group.NCBI));
Copy link
Contributor

@raskoleinonen raskoleinonen Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the validation scope and group tests into a new class:

ValidationScopeTest

and test that all ValidationScope enums against isInGroup:

@Test
public void testValidationScopeIsInGroup() { 
assertTrue(ValidationScope.NCBI.isInGroup(ValidationScope.Group.PUTFF));
assertFalse(ValidationScope.NCBI.isInGroup(ValidationScope.Group.PIPELINE));
etc.
...
}

assertTrue(ValidationScope.NCBI.isInGroup(ValidationScope.Group.PUTFF));

assertTrue(ValidationScope.ASSEMBLY_MASTER.isInGroup(ValidationScope.Group.ASSEMBLY));
assertTrue(ValidationScope.ASSEMBLY_MASTER.isInGroup(ValidationScope.Group.PIPELINE));
}
}
39 changes: 31 additions & 8 deletions src/test/java/uk/ac/ebi/embl/api/validation/plan/ValidationPlanTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,39 @@ public void testIsInValidationScope_Right() {
}

@Test
public void testIsInValidationScope_Many() {
assertTrue(
plan.isInValidationScope(
new ValidationScope[] {ValidationScope.EMBL, ValidationScope.INSDC}));
public void testIsInValidationScopePutffGroup() {
Copy link
Contributor

@raskoleinonen raskoleinonen Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Same comment as the previous one) -> Please move the validation scope and group tests into a new class:

ValidationScopeTest

and test that all ValidationScope enums against isInGroup:

@Test
public void testValidationScopeIsInGroup() { 
assertTrue(ValidationScope.NCBI.isInGroup(ValidationScope.Group.PUTFF));
assertFalse(ValidationScope.NCBI.isInGroup(ValidationScope.Group.PIPELINE));
etc.
...
}

assertTrue(plan.validationScope.isInGroup(ValidationScope.Group.PUTFF));
assertFalse(plan.validationScope.isInGroup(ValidationScope.Group.PIPELINE));
}

@Test
public void testIsInValidationScope_ManyWrong() {
assertFalse(
plan.isInValidationScope(
new ValidationScope[] {ValidationScope.EPO, ValidationScope.INSDC}));
public void testIsInValidationScopePipelineGroup() {
Copy link
Contributor

@raskoleinonen raskoleinonen Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replicate what is in the ValidationScope::testValidationScopeIsInGroup() in ValidationPlanTest. The only difference is that ValidationPlanTest creates the ValidationPlan.

ValidationPlan plan =
new ValidationPlan(ValidationScope.ASSEMBLY_MASTER, false) {
@Override
public ValidationResult execute(Object target) {
return null;
}
};

assertTrue(plan.validationScope.isInGroup(ValidationScope.Group.PIPELINE));
assertTrue(plan.validationScope.isInGroup(ValidationScope.Group.ASSEMBLY));
assertFalse(plan.validationScope.isInGroup(ValidationScope.Group.PUTFF));
}

@Test
public void testIsInValidationScopeNcbiGroup() {
Copy link
Contributor

@raskoleinonen raskoleinonen Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replicate what is in the ValidationScope::testValidationScopeIsInGroup() in ValidationPlanTest. The only difference is that ValidationPlanTest creates the ValidationPlan.

ValidationPlan plan =
new ValidationPlan(ValidationScope.NCBI, false) {
@Override
public ValidationResult execute(Object target) {
return null;
}
};

assertTrue(plan.validationScope.isInGroup(ValidationScope.Group.NCBI));
assertTrue(plan.validationScope.isInGroup(ValidationScope.Group.PUTFF));
assertFalse(plan.validationScope.isInGroup(ValidationScope.Group.ASSEMBLY));

}
}
6 changes: 4 additions & 2 deletions src/test/java/uk/ac/ebi/embl/template/TemplateEntryProcessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ public void testEntryProcess_ERT000002() throws Exception {

// Test with invalid taxid
expectedException.expect(TaxonomyException.class);
expectedException.expectMessage("Error while calling the url: https://www.ebi.ac.uk/ena/taxonomy/rest/tax-id/960600000000");
expectedException.expectMessage(
"Error while calling the url: https://www.ebi.ac.uk/ena/taxonomy/rest/tax-id/960600000000");
templateVariables = getTemplateVariables_ERT000002("960600000000");
executeEntryProcessInvalidTaxId(
"960600000000", templateInfo_ERT000002, molType_ERT000002, templateVariables);
Expand Down Expand Up @@ -168,7 +169,8 @@ public void testEntryProcess_ERT000056() throws Exception {

// Test with invalid taxid
expectedException.expect(TaxonomyException.class);
expectedException.expectMessage("Error while calling the url: https://www.ebi.ac.uk/ena/taxonomy/rest/tax-id/960600000000");
expectedException.expectMessage(
"Error while calling the url: https://www.ebi.ac.uk/ena/taxonomy/rest/tax-id/960600000000");
templateVariables = getTemplateVariables_ERT000056("960600000000");
executeEntryProcessInvalidTaxId(
"960600000000", templateInfo_ERT000056, molType_ERT000056, templateVariables);
Expand Down