MET-5444 Code review comments

europeana · Sep 25, 2023 · d969306 · d969306
1 parent 8342c06
commit d969306
Show file tree

Hide file tree

Showing 11 changed files with 52 additions and 40 deletions.
diff --git a/src/main/java/eu/europeana/metis/sandbox/controller/ratelimit/RateLimitInterceptor.java b/src/main/java/eu/europeana/metis/sandbox/controller/ratelimit/RateLimitInterceptor.java
@@ -23,7 +23,7 @@
  */
 public class RateLimitInterceptor implements HandlerInterceptor {
 
-    private static final int VALUE_TO_TRANSFORM_TO_SECONDS = 1_000_000_000;
+    private static final int CONVERSION_FACTOR_FROM_NANOS_TO_SECONDS = 1_000_000_000;
 
     private final Integer capacity;
     private final PostgreSQLadvisoryLockBasedProxyManager postgreSQLManager;
@@ -50,11 +50,11 @@ public boolean preHandle(HttpServletRequest request, @NotNull HttpServletRespons
         response.addHeader("X-Rate-Limit-Limit", String.valueOf(capacity));
         if (probe.isConsumed()) {
             response.addHeader("X-Rate-Limit-Remaining", String.valueOf(probe.getRemainingTokens()));
-            final long waitForRefill = probe.getNanosToWaitForReset() / VALUE_TO_TRANSFORM_TO_SECONDS;
+            final long waitForRefill = probe.getNanosToWaitForReset() / CONVERSION_FACTOR_FROM_NANOS_TO_SECONDS;
             response.addHeader("X-Rate-Limit-Reset", String.valueOf(waitForRefill));
             return true;
         } else {
-            final long waitForRefill = probe.getNanosToWaitForRefill() / VALUE_TO_TRANSFORM_TO_SECONDS;
+            final long waitForRefill = probe.getNanosToWaitForRefill() / CONVERSION_FACTOR_FROM_NANOS_TO_SECONDS;
             response.addHeader("X-Rate-Limit-Reset", String.valueOf(waitForRefill));
             response.sendError(HttpStatus.TOO_MANY_REQUESTS.value(),
                     "You have exhausted your API Request Quota");

diff --git a/...in/java/eu/europeana/metis/sandbox/service/validationworkflow/ExternalValidationStep.java b/...in/java/eu/europeana/metis/sandbox/service/validationworkflow/ExternalValidationStep.java
@@ -36,7 +36,7 @@ public ValidationStepContent performStep(Record recordToValidate) {
         ValidationStepContent validationStepContent;
         try {
             RecordInfo recordInfoValidated = externalValidationService.validate(recordToValidate);
-            validationStepContent = ValidatedRecordExtractor.extractResults(Step.VALIDATE_EXTERNAL, recordInfoValidated);
+            validationStepContent = ValidatedRecordExtractor.extractValidationStepContent(Step.VALIDATE_EXTERNAL, recordInfoValidated);
             recordLogService.logRecordEvent(new RecordProcessEvent(recordInfoValidated, Step.VALIDATE_EXTERNAL, Status.SUCCESS));
         } catch (Exception ex) {
             LOGGER.error("external validation step fail", ex);

diff --git a/.../europeana/metis/sandbox/service/validationworkflow/InternalValidationValidationStep.java b/.../europeana/metis/sandbox/service/validationworkflow/InternalValidationValidationStep.java
@@ -38,7 +38,7 @@ public ValidationStepContent performStep(Record recordToValidate) {
             RecordInfo recordInfoValidated = internalValidationService.validate(recordToValidate);
             recordToValidate = ValidatedRecordExtractor.extractRecord(recordInfoValidated);
             LOGGER.info("internal validation step success {}", recordToValidate.getDatasetName());
-            validationResult = ValidatedRecordExtractor.extractResults(Step.VALIDATE_INTERNAL, recordInfoValidated);
+            validationResult = ValidatedRecordExtractor.extractValidationStepContent(Step.VALIDATE_INTERNAL, recordInfoValidated);
             recordLogService.logRecordEvent(new RecordProcessEvent(recordInfoValidated, Step.VALIDATE_INTERNAL, Status.SUCCESS));
             recordLogService.logRecordEvent(new RecordProcessEvent(recordInfoValidated, Step.CLOSE, Status.SUCCESS));
         } catch (Exception ex) {

diff --git a/...a/eu/europeana/metis/sandbox/service/validationworkflow/TransformationValidationStep.java b/...a/eu/europeana/metis/sandbox/service/validationworkflow/TransformationValidationStep.java
@@ -33,18 +33,18 @@ public TransformationValidationStep(TransformationService transformationService,
 
     @Override
     public ValidationStepContent performStep(Record recordToValidate) {
-        ValidationStepContent validationResult;
+        ValidationStepContent validationStepContent;
         try {
             RecordInfo recordInfoValidated = transformationService.transformToEdmInternal(recordToValidate);
-            validationResult = ValidatedRecordExtractor.extractResults(Step.TRANSFORM, recordInfoValidated);
+            validationStepContent = ValidatedRecordExtractor.extractValidationStepContent(Step.TRANSFORM, recordInfoValidated);
             recordLogService.logRecordEvent(new RecordProcessEvent(recordInfoValidated, Step.TRANSFORM, Status.SUCCESS));
         } catch (Exception ex) {
             LOGGER.error("transformation validation step fail", ex);
-            validationResult = new ValidationStepContent(new ValidationResult(Step.TRANSFORM,
+            validationStepContent = new ValidationStepContent(new ValidationResult(Step.TRANSFORM,
                     new RecordValidationMessage(RecordValidationMessage.Type.ERROR, ex.toString()),
                     ValidationResult.Status.FAILED), recordToValidate);
             recordLogService.logRecordEvent(new RecordProcessEvent(new RecordInfo(recordToValidate), Step.TRANSFORM, Status.FAIL));
         }
-        return validationResult;
+        return validationStepContent;
     }
 }
diff --git a/.../java/eu/europeana/metis/sandbox/service/validationworkflow/ValidatedRecordExtractor.java b/.../java/eu/europeana/metis/sandbox/service/validationworkflow/ValidatedRecordExtractor.java
@@ -15,6 +15,10 @@
 public class ValidatedRecordExtractor {
     private static final Logger LOGGER = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
+    private ValidatedRecordExtractor(){
+        throw new IllegalStateException("Validated record utility class");
+    }
+
     /**
      * Extract record.
      *
@@ -41,7 +45,7 @@ public static Record extractRecord(RecordInfo validatedRecordInfo) {
      * @param recordInfo        the record info
      * @return the list
      */
-    public static ValidationStepContent extractResults(Step step,RecordInfo recordInfo) {
+    public static ValidationStepContent extractValidationStepContent(Step step, RecordInfo recordInfo) {
         ValidationStepContent result;
         if (recordInfo.getErrors().isEmpty()) {
             result = new ValidationStepContent(new ValidationResult(step,

diff --git a/...java/eu/europeana/metis/sandbox/service/validationworkflow/ValidationWorkflowService.java b/...java/eu/europeana/metis/sandbox/service/validationworkflow/ValidationWorkflowService.java
@@ -149,29 +149,29 @@ private void finalizeValidationPatternAnalysis(String datasetId, ExecutionPoint
     private List<ValidationResult> performSteps(Record recordToProcess){
         List<ValidationResult> validationResults = new ArrayList<>();
 
-        ValidationStepContent harvestResult = harvestValidationStep.performStep(recordToProcess);
-        validationResults.add(harvestResult.getValidationStepResult());
+        ValidationStepContent harvestStepContent = harvestValidationStep.performStep(recordToProcess);
+        validationResults.add(harvestStepContent.getValidationStepResult());
 
-        if(harvestResult.getValidationStepResult().getStatus() == ValidationResult.Status.FAILED){
+        if(harvestStepContent.getValidationStepResult().getStatus() == ValidationResult.Status.FAILED){
             return validationResults;
         }
 
-        ValidationStepContent externalValidationResult = externalValidationStep.performStep(harvestResult.getRecordStepResult());
-        validationResults.add(externalValidationResult.getValidationStepResult());
+        ValidationStepContent externalValidationStepContent = externalValidationStep.performStep(harvestStepContent.getRecordStepResult());
+        validationResults.add(externalValidationStepContent.getValidationStepResult());
 
-        if(externalValidationResult.getValidationStepResult().getStatus() == ValidationResult.Status.FAILED){
+        if(externalValidationStepContent.getValidationStepResult().getStatus() == ValidationResult.Status.FAILED){
             return validationResults;
         }
 
-        ValidationStepContent transformationResult = transformationValidationStep.performStep(externalValidationResult.getRecordStepResult());
-        validationResults.add(transformationResult.getValidationStepResult());
+        ValidationStepContent transformationStepContent = transformationValidationStep.performStep(externalValidationStepContent.getRecordStepResult());
+        validationResults.add(transformationStepContent.getValidationStepResult());
 
-        if(transformationResult.getValidationStepResult().getStatus() == ValidationResult.Status.FAILED){
+        if(transformationStepContent.getValidationStepResult().getStatus() == ValidationResult.Status.FAILED){
             return validationResults;
         }
 
-        ValidationStepContent internalValidationResult = internalValidationValidationStep.performStep(transformationResult.getRecordStepResult());
-        validationResults.add(internalValidationResult.getValidationStepResult());
+        ValidationStepContent internalValidationStepContent = internalValidationValidationStep.performStep(transformationStepContent.getRecordStepResult());
+        validationResults.add(internalValidationStepContent.getValidationStepResult());
 
         return validationResults;
     }

diff --git a/...ava/eu/europeana/metis/sandbox/service/validationworkflow/ExternalValidationStepTest.java b/...ava/eu/europeana/metis/sandbox/service/validationworkflow/ExternalValidationStepTest.java
@@ -1,6 +1,5 @@
 package eu.europeana.metis.sandbox.service.validationworkflow;
 
-import eu.europeana.metis.sandbox.common.Step;
 import eu.europeana.metis.sandbox.common.locale.Country;
 import eu.europeana.metis.sandbox.common.locale.Language;
 import eu.europeana.metis.sandbox.domain.Record;
@@ -16,7 +15,9 @@
 import java.nio.charset.StandardCharsets;
 import java.util.Optional;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.times;
@@ -79,10 +80,10 @@ void validate_expectFail() {
         when(externalValidationService.validate(any())).thenThrow(new RuntimeException("External validation failure"));
 
         //when
-        ValidationStepContent validationResults = externalValidationStep.performStep(recordToValidate);
+        ValidationStepContent validationStepContent = externalValidationStep.performStep(recordToValidate);
 
         //then
-        ValidationResult result = validationResults.getValidationStepResult();
+        ValidationResult result = validationStepContent.getValidationStepResult();
         assertNotNull(result);
         assertEquals(ValidationResult.Status.FAILED, result.getStatus());
         Optional<RecordValidationMessage> message = result.getMessages().stream().findFirst();

diff --git a/...java/eu/europeana/metis/sandbox/service/validationworkflow/HarvestValidationStepTest.java b/...java/eu/europeana/metis/sandbox/service/validationworkflow/HarvestValidationStepTest.java
@@ -1,6 +1,5 @@
 package eu.europeana.metis.sandbox.service.validationworkflow;
 
-import eu.europeana.metis.sandbox.common.exception.ServiceException;
 import eu.europeana.metis.sandbox.common.locale.Country;
 import eu.europeana.metis.sandbox.common.locale.Language;
 import eu.europeana.metis.sandbox.domain.Record;
@@ -18,7 +17,10 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 
 
 @ExtendWith(MockitoExtension.class)
@@ -44,10 +46,10 @@ void validate_expectSuccess() {
                 .build();
 
         //when
-        ValidationStepContent validationResults = harvestValidationStep.performStep(record);
+        ValidationStepContent validationStepContent = harvestValidationStep.performStep(record);
 
         //then
-        ValidationResult result = validationResults.getValidationStepResult();
+        ValidationResult result = validationStepContent.getValidationStepResult();
         assertNotNull(result);
         assertEquals(ValidationResult.Status.PASSED, result.getStatus());
         Optional<RecordValidationMessage> message = result.getMessages().stream().findFirst();
@@ -73,10 +75,10 @@ void validate_expectFail() {
                 .build();
 
         //when
-        ValidationStepContent validationResults = harvestValidationStep.performStep(record);
+        ValidationStepContent validationStepContent = harvestValidationStep.performStep(record);
 
         //then
-        ValidationResult result = validationResults.getValidationStepResult();
+        ValidationResult result = validationStepContent.getValidationStepResult();
         assertNotNull(result);
         assertEquals(ValidationResult.Status.FAILED, result.getStatus());
         Optional<RecordValidationMessage> message = result.getMessages().stream().findFirst();

diff --git a/...opeana/metis/sandbox/service/validationworkflow/InternalValidationValidationStepTest.java b/...opeana/metis/sandbox/service/validationworkflow/InternalValidationValidationStepTest.java
@@ -1,6 +1,5 @@
 package eu.europeana.metis.sandbox.service.validationworkflow;
 
-import eu.europeana.metis.sandbox.common.Step;
 import eu.europeana.metis.sandbox.common.locale.Country;
 import eu.europeana.metis.sandbox.common.locale.Language;
 import eu.europeana.metis.sandbox.domain.Record;
@@ -16,7 +15,9 @@
 import java.nio.charset.StandardCharsets;
 import java.util.Optional;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.times;

diff --git a/.../europeana/metis/sandbox/service/validationworkflow/TransformationValidationStepTest.java b/.../europeana/metis/sandbox/service/validationworkflow/TransformationValidationStepTest.java
@@ -1,6 +1,6 @@
 package eu.europeana.metis.sandbox.service.validationworkflow;
 
-import eu.europeana.metis.sandbox.common.Step;
+
 import eu.europeana.metis.sandbox.common.locale.Country;
 import eu.europeana.metis.sandbox.common.locale.Language;
 import eu.europeana.metis.sandbox.domain.Record;
@@ -16,7 +16,9 @@
 import java.nio.charset.StandardCharsets;
 import java.util.Optional;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;

diff --git a/...a/eu/europeana/metis/sandbox/service/validationworkflow/ValidatedRecordExtractorTest.java b/...a/eu/europeana/metis/sandbox/service/validationworkflow/ValidatedRecordExtractorTest.java
@@ -12,7 +12,9 @@
 import java.util.List;
 import java.util.Optional;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 
 class ValidatedRecordExtractorTest {
 
@@ -60,10 +62,10 @@ void extractResults_expectSuccess() {
                 .build();
         RecordInfo recordInfo = new RecordInfo(expectedRecord);
         // when
-        ValidationStepContent validationResults = ValidatedRecordExtractor.extractResults(Step.HARVEST_FILE, recordInfo);
+        ValidationStepContent validationStepContent = ValidatedRecordExtractor.extractValidationStepContent(Step.HARVEST_FILE, recordInfo);
 
         // then
-        ValidationResult result = validationResults.getValidationStepResult();
+        ValidationResult result = validationStepContent.getValidationStepResult();
         assertNotNull(result);
         assertEquals(ValidationResult.Status.PASSED, result.getStatus());
         Optional<RecordValidationMessage> message = result.getMessages().stream().findFirst();
@@ -86,10 +88,10 @@ void extractResults_withErrors_expectSuccess() {
         RecordInfo recordInfo = new RecordInfo(expectedRecord, List.of(new RecordError("Fail message1", "stackTrace1"),
                 new RecordError("Fail message2", "stackTrace2")));
         // when
-        ValidationStepContent validationResults = ValidatedRecordExtractor.extractResults(Step.HARVEST_FILE, recordInfo);
+        ValidationStepContent validationStepContent = ValidatedRecordExtractor.extractValidationStepContent(Step.HARVEST_FILE, recordInfo);
 
         // then
-        ValidationResult result = validationResults.getValidationStepResult();
+        ValidationResult result = validationStepContent.getValidationStepResult();
         assertEquals(ValidationResult.Status.FAILED, result.getStatus());
         Optional<RecordValidationMessage> message = result.getMessages().stream().findFirst();
         assertEquals("Fail message1", message.get().getMessage());