BI-1183 - Import Validations: Ontology-specific observation criteria by nickpalladino · Pull Request #210 · Breeding-Insight/bi-api

nickpalladino · 2022-09-12T14:55:20Z

Description

Story: BI-1183

Added import template dynamic column processing for phenotypes
Added validations of phenotype column data

Dependencies

bi-web BI-1183

Testing

Example test file included. Test validations to ensure they are working according to card acceptance criteria.
demo.xlsx

Checklist:

I have performed a self-review of my own code
I have tested my code and ensured it meets the acceptance criteria of the story
I have tested that my code works with both the brapi-java-server and BreedBase
I have create/modified unit tests to cover this change
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to documentation
I have run TAF: <please include a link to TAF run>

[BI-1198] All Experiments Table

Converted semaphore to use Redisson's semaphore (distributed semaphore)

…ce/impl design pattern Introduced flag to suppress refreshing cache on startup if desired (useful for integration tests)

HMS17 · 2022-10-05T15:24:18Z

src/main/java/org/breedinginsight/brapps/importer/services/processors/GermplasmProcessor.java

    @Override
    public Map<String, ImportPreviewStatistics> process(List<BrAPIImport> importRows,
-                        Map<Integer, PendingImport> mappedBrAPIImport, Program program, User user, boolean commit) throws ValidatorException {
+                                                        Map<Integer, PendingImport> mappedBrAPIImport, Table table,


Minor thing, but is there a reason you went with Table table here and in Processor.java and then Table data in other files?

Nope, I will change them all to data to be consistent. Pushed changes.

timparsons · 2022-10-07T02:31:54Z

src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java

        ValidationErrors validationErrors = new ValidationErrors();

+        // Get dynamic phenotype columns for processing
+        List<Column<?>> phenotypeCols = fileMappingUtil.getDynamicColumns(data, "ExperimentsTemplateMap");


Is there a constant that's defined anywhere for the ExperimentsTemplateMap? If not, can that be added?

No, I'll add one. Pushed changes.

timparsons · 2022-10-07T02:45:15Z

src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java

+                                                                                        Function3<String, Integer, Integer, ApiResponse<Pair<Optional<T>, Optional<BrAPIAcceptedSearchResponse>>>> searchGetMethod3,
+                                                                                        Function4<BrAPIWSMIMEDataTypes, String, Integer, Integer, ApiResponse<Pair<Optional<T>, Optional<BrAPIAcceptedSearchResponse>>>> searchGetMethod4,


For naming of these to parameters, what about:

searchGetMethod3 goes back to searchGetMethod

searchGetMethod4 changes to searchGetMethodWithMimeType

Sounds good, also added method to reuse, pushed changes.

timparsons

I had tried importing 4 files:

BI_1183_valid_columns.csv

Result: FAIL - Error during validation:

java.util.NoSuchElementException: No value present
	at java.base/java.util.Optional.get(Optional.java:141)
	at org.breedinginsight.brapps.importer.services.processors.ExperimentProcessor.validateObservationValue(ExperimentProcessor.java:757)
	at org.breedinginsight.brapps.importer.services.processors.ExperimentProcessor.process(ExperimentProcessor.java:209)
	at org.breedinginsight.brapps.importer.services.processors.ProcessorManager.process(ProcessorManager.java:65)
	at org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentImportService.process(ExperimentImportService.java:71)
	at org.breedinginsight.brapps.importer.services.FileImportService.lambda$processFile$4(FileImportService.java:406)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1771)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.exec(CompletableFuture.java:1763)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1016)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1665)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1598)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)

BI_1183_valid_with_data.csv

Result: PASS - was presented with the preview table

BI_1183_invalid_columns.csv

Result: PASS - Error showing unrecognized columns

BI_1183_invalid_data.csv

Result: FAIL - Error during validation:

java.util.NoSuchElementException: No value present
	at java.base/java.util.Optional.get(Optional.java:141)
	at org.breedinginsight.brapps.importer.services.processors.ExperimentProcessor.validateObservationValue(ExperimentProcessor.java:757)
	at org.breedinginsight.brapps.importer.services.processors.ExperimentProcessor.process(ExperimentProcessor.java:209)
	at org.breedinginsight.brapps.importer.services.processors.ProcessorManager.process(ProcessorManager.java:65)
	at org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentImportService.process(ExperimentImportService.java:71)
	at org.breedinginsight.brapps.importer.services.FileImportService.lambda$processFile$4(FileImportService.java:406)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1771)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.exec(CompletableFuture.java:1763)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1016)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1665)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1598)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)

Traits used: bi_ontology_template_v11_2021-12-01-test-ontology-with-method-desc.xls

nickpalladino · 2022-10-14T15:10:31Z

I had tried importing 4 files:

* [BI_1183_valid_columns.csv](https://github.com/Breeding-Insight/bi-api/files/9748497/BI_1183_valid_columns.csv)

Result: FAIL - Error during validation:

java.util.NoSuchElementException: No value present
	at java.base/java.util.Optional.get(Optional.java:141)
	at org.breedinginsight.brapps.importer.services.processors.ExperimentProcessor.validateObservationValue(ExperimentProcessor.java:757)
	at org.breedinginsight.brapps.importer.services.processors.ExperimentProcessor.process(ExperimentProcessor.java:209)
	at org.breedinginsight.brapps.importer.services.processors.ProcessorManager.process(ProcessorManager.java:65)
	at org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentImportService.process(ExperimentImportService.java:71)
	at org.breedinginsight.brapps.importer.services.FileImportService.lambda$processFile$4(FileImportService.java:406)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1771)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.exec(CompletableFuture.java:1763)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1016)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1665)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1598)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)

* [BI_1183_valid_with_data.csv](https://github.com/Breeding-Insight/bi-api/files/9748498/BI_1183_valid_with_data.csv)

Result: PASS - was presented with the preview table

* [BI_1183_invalid_columns.csv](https://github.com/Breeding-Insight/bi-api/files/9748499/BI_1183_invalid_columns.csv)

Result: PASS - Error showing unrecognized columns

* [BI_1183_invalid_data.csv](https://github.com/Breeding-Insight/bi-api/files/9748500/BI_1183_invalid_data.csv)

Result: FAIL - Error during validation:

java.util.NoSuchElementException: No value present
	at java.base/java.util.Optional.get(Optional.java:141)
	at org.breedinginsight.brapps.importer.services.processors.ExperimentProcessor.validateObservationValue(ExperimentProcessor.java:757)
	at org.breedinginsight.brapps.importer.services.processors.ExperimentProcessor.process(ExperimentProcessor.java:209)
	at org.breedinginsight.brapps.importer.services.processors.ProcessorManager.process(ProcessorManager.java:65)
	at org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentImportService.process(ExperimentImportService.java:71)
	at org.breedinginsight.brapps.importer.services.FileImportService.lambda$processFile$4(FileImportService.java:406)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1771)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.exec(CompletableFuture.java:1763)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1016)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1665)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1598)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)

Traits used: bi_ontology_template_v11_2021-12-01-test-ontology-with-method-desc.xls

Pushed fix for failing cases

… the cell

timparsons · 2022-10-15T02:55:46Z

src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java

+        try {
+            traits = ontologyService.getTraitsByProgramId(program.getId(), true);
+        } catch (DoesNotExistException e) {
+            log.error(e.getMessage());


Can you add the exception e as a second parameter to this log statement (that way the stack trace will be printed too)

Yep, pushed changes.

timparsons · 2022-10-15T02:58:12Z

src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java

+                break;
+            case DATE:
+                if (!validDateValue(value)) {
+                    addRowError(columnHeader, "Incorrect date format (YYYY-MM-DD) detected", validationErrors, row);


This error message is a little confusing. What about "Incorrect date format detected. Expected YYYY-MM-DD"?

Sure, pushed changes.

timparsons · 2022-10-15T03:02:33Z

src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java

+    }
+
+    private boolean validNumericRange(BigDecimal value, Scale validValues) {
+        if (value.compareTo(BigDecimal.valueOf(validValues.getValidValueMin())) >= 0 &&


This code will throw a NullPointerException if validValues.getValidValueMin or validValues.getValidValueMax are null

Thanks for spotting that, pushed changes.

rob-ouser-bi and others added 27 commits August 3, 2022 16:49

[autocommit] bumping build number

c38b112

Merge pull request #201 from Breeding-Insight/feature/BI-1198-2

806ccd7

[BI-1198] All Experiments Table

[autocommit] bumping build number

0da2077

Update version.properties

04156d0

[autocommit] bumping build number

e205dd7

[BI-1555] - Converted ProgramCache to use Redis instead of Guava cache.

80246f8

[BI-1555] - Additional changes for cache to work properly

5762506

[BI-1555] - removing embedded micronaut dependencies

3800527

[BI-1555] - Converting Redis library to Redisson.

8a5de9b

Converted semaphore to use Redisson's semaphore (distributed semaphore)

[BI-1555] - Configuring redis container for integration tests

f000245

[BI-1555] - Updating ProgramCacheUnitTest

4331f42

[BI-1555] - Stopping redis container on integration tests

6176d84

[BI-1555] - Adding docker network config to tests

4c34b71

[BI-1555] - adding robustness to the caching processor queue

b1c7b7f

[BI-1555] - removing redundant injected constructor from ProgramService

74f7880

[BI-1555] - Refactoring TraitDAO and ProgramDAO to follow the interfa…

8afd892

…ce/impl design pattern Introduced flag to suppress refreshing cache on startup if desired (useful for integration tests)

[BI-1555] - Fixing unit tests

99281b6

[BI-1555] Cleanup

fb96326

[BI-1555] - Fixing unit tests

34f0b08

[BI-1555] - bumping heap size for unit tests

2a61f14

[BI-1555] - bumping surefire plugin version

3693c4a

Update pom.xml

bb5883d

[BI-1555] - removing unnecessary surefire dependencies

0113e01

[BI-1555] - Refactoring how containers are created and managed

cf95ad1

[BI-1555] - fixing compilation error

f726923

[BI-1555] - Fixing failed tests

ac6437e

Added migration to include fileFieldName for ObsUnitID

c3a0051

github-actions bot added the feature label Sep 12, 2022

nickpalladino added 2 commits September 13, 2022 09:26

Removed unused acting user variable

1ab4ddd

Added parsed data to import mapping

dce8e9d

nickpalladino requested review from a team, HMS17 and timparsons and removed request for a team October 4, 2022 17:53

HMS17 approved these changes Oct 5, 2022

View reviewed changes

Updated Table data parameter names to be consistent

7cbca14

nickpalladino assigned timparsons Oct 6, 2022

timparsons requested changes Oct 7, 2022

View reviewed changes

nickpalladino added 2 commits October 7, 2022 10:15

Added constant for experiment template

dda592b

Changed search method names and reused search method

55bc2ba

nickpalladino requested a review from timparsons October 7, 2022 18:14

timparsons force-pushed the future/1.0 branch from d60da41 to 7cb9557 Compare October 10, 2022 18:40

timparsons requested changes Oct 10, 2022

View reviewed changes

nickpalladino added 4 commits October 11, 2022 11:32

Merge branch 'future/1.0' into feature/BI-1183

1d5cb81

Renamed for version conflict

41d4d78

Fix program cache issues

a790c69

Fix for empty numeric values

7b27a45

nickpalladino requested a review from timparsons October 14, 2022 15:35

nickpalladino changed the base branch from future/1.0 to develop October 14, 2022 18:45

nickpalladino and others added 2 commits October 14, 2022 14:48

Merge branch 'develop' into feature/BI-1183

1639863

[BI-1183] - Only validating imported observations if there is data in…

45f38f1

… the cell

timparsons requested changes Oct 15, 2022

View reviewed changes

nickpalladino added 3 commits October 20, 2022 16:30

Code review logging change

25b4279

Updated date format error message for clarity

825d7ca

Updated valid numeric range to account for missing min or max

e809b8a

nickpalladino requested a review from timparsons October 21, 2022 21:06

timparsons approved these changes Oct 24, 2022

View reviewed changes

nickpalladino merged commit 15797c9 into develop Oct 25, 2022

nickpalladino deleted the feature/BI-1183 branch October 25, 2022 19:13

		Function3<String, Integer, Integer, ApiResponse<Pair<Optional<T>, Optional<BrAPIAcceptedSearchResponse>>>> searchGetMethod3,
		Function4<BrAPIWSMIMEDataTypes, String, Integer, Integer, ApiResponse<Pair<Optional<T>, Optional<BrAPIAcceptedSearchResponse>>>> searchGetMethod4,

Conversation

nickpalladino commented Sep 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Dependencies

Testing

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timparsons left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickpalladino commented Oct 14, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nickpalladino commented Sep 12, 2022 •

edited

Loading

timparsons left a comment •

edited

Loading