CE-1955 Revert splitting out records with mapping errors, to help do less spoon-feeding; also, avoid double-running customizer

qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/InsertAction.java

@@ -122,7 +122,7 @@ public class InsertAction extends AbstractQActionFunction<InsertInput, InsertOut
       /////////////////////////////
       // run standard validators //
       /////////////////////////////
-      performValidations(insertInput, false);
+      performValidations(insertInput, false, false);
 
       //////////////////////////////////////////////////////
       // use the backend module to actually do the insert //
@@ -225,7 +225,7 @@ public class InsertAction extends AbstractQActionFunction<InsertInput, InsertOut
    /*******************************************************************************
     **
     *******************************************************************************/
-   public void performValidations(InsertInput insertInput, boolean isPreview) throws QException
+   public void performValidations(InsertInput insertInput, boolean isPreview, boolean didAlreadyRunCustomizer) throws QException
    {
       if(CollectionUtils.nullSafeIsEmpty(insertInput.getRecords()))
       {
@@ -237,12 +237,10 @@ public class InsertAction extends AbstractQActionFunction<InsertInput, InsertOut
       ///////////////////////////////////////////////////////////////////
       // load the pre-insert customizer and set it up, if there is one //
       // then we'll run it based on its WhenToRun value                //
+      // note - if we already ran it, then don't re-run it!            //
       ///////////////////////////////////////////////////////////////////
-      Optional<TableCustomizerInterface> preInsertCustomizer = QCodeLoader.getTableCustomizer(table, TableCustomizers.PRE_INSERT_RECORD.getRole());
+      Optional<TableCustomizerInterface> preInsertCustomizer = didAlreadyRunCustomizer ? Optional.empty() : QCodeLoader.getTableCustomizer(table, TableCustomizers.PRE_INSERT_RECORD.getRole());
-      if(preInsertCustomizer.isPresent())
+      runPreInsertCustomizerIfItIsTime(insertInput, isPreview, preInsertCustomizer, AbstractPreInsertCustomizer.WhenToRun.BEFORE_ALL_VALIDATIONS);
-      {
-         runPreInsertCustomizerIfItIsTime(insertInput, isPreview, preInsertCustomizer, AbstractPreInsertCustomizer.WhenToRun.BEFORE_ALL_VALIDATIONS);
-      }
 
       setDefaultValuesInRecords(table, insertInput.getRecords());
 

qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/bulk/insert/BulkInsertTransformStep.java

@@ -187,65 +187,8 @@ public class BulkInsertTransformStep extends AbstractTransformStep
    @Override
    public void runOnePage(RunBackendStepInput runBackendStepInput, RunBackendStepOutput runBackendStepOutput) throws QException
    {
-      int            recordsInThisPage = runBackendStepInput.getRecords().size();
+      QTableMetaData table   = QContext.getQInstance().getTable(runBackendStepInput.getTableName());
-      QTableMetaData table             = QContext.getQInstance().getTable(runBackendStepInput.getTableName());
+      List<QRecord>  records = runBackendStepInput.getRecords();
-
-      //////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-      // split the records into 2 lists:  those w/ errors (e.g., from the bulk-load mapping), and those that are okay //
-      //////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-      List<QRecord> recordsWithoutAnyErrors = new ArrayList<>();
-      List<QRecord> recordsWithSomeErrors   = new ArrayList<>();
-      for(QRecord record : runBackendStepInput.getRecords())
-      {
-         List<QErrorMessage> errorsFromAssociations = getErrorsFromAssociations(record);
-         if(CollectionUtils.nullSafeHasContents(errorsFromAssociations))
-         {
-            List<QErrorMessage> recordErrors = Objects.requireNonNullElseGet(record.getErrors(), () -> new ArrayList<>());
-            recordErrors.addAll(errorsFromAssociations);
-            record.setErrors(recordErrors);
-         }
-
-         if(CollectionUtils.nullSafeHasContents(record.getErrors()))
-         {
-            recordsWithSomeErrors.add(record);
-         }
-         else
-         {
-            recordsWithoutAnyErrors.add(record);
-         }
-      }
-
-      //////////////////////////////////////////////////////////////////
-      // propagate errors that came into this step out to the summary //
-      //////////////////////////////////////////////////////////////////
-      if(!recordsWithSomeErrors.isEmpty())
-      {
-         for(QRecord record : recordsWithSomeErrors)
-         {
-            for(QErrorMessage error : record.getErrors())
-            {
-               if(error instanceof AbstractBulkLoadRollableValueError rollableValueError)
-               {
-                  processSummaryWarningsAndErrorsRollup.addError(rollableValueError.getMessageToUseAsProcessSummaryRollupKey(), null);
-                  addToErrorToExampleRowValueMap(rollableValueError, record);
-               }
-               else
-               {
-                  processSummaryWarningsAndErrorsRollup.addError(error.getMessage(), null);
-               }
-            }
-         }
-      }
-
-      if(recordsWithoutAnyErrors.isEmpty())
-      {
-         /////////////////////////////////////////////////////////////////////////////////
-         // skip the rest of this method if there aren't any records w/o errors in them //
-         // but, advance our counter before we return.                                  //
-         /////////////////////////////////////////////////////////////////////////////////
-         this.rowsProcessed += recordsInThisPage;
-         return;
-      }
 
       /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
       // set up an insert-input, which will be used as input to the pre-customizer as well as for additional validations //
@@ -253,7 +196,7 @@ public class BulkInsertTransformStep extends AbstractTransformStep
       InsertInput insertInput = new InsertInput();
       insertInput.setInputSource(QInputSource.USER);
       insertInput.setTableName(runBackendStepInput.getTableName());
-      insertInput.setRecords(recordsWithoutAnyErrors);
+      insertInput.setRecords(records);
       insertInput.setSkipUniqueKeyCheck(true);
 
       //////////////////////////////////////////////////////////////////////
@@ -262,34 +205,41 @@ public class BulkInsertTransformStep extends AbstractTransformStep
       // we do this, in case it needs to, for example, adjust values that //
       // are part of a unique key                                         //
       //////////////////////////////////////////////////////////////////////
-      Optional<TableCustomizerInterface> preInsertCustomizer = QCodeLoader.getTableCustomizer(table, TableCustomizers.PRE_INSERT_RECORD.getRole());
+      boolean                            didAlreadyRunCustomizer = false;
+      Optional<TableCustomizerInterface> preInsertCustomizer     = QCodeLoader.getTableCustomizer(table, TableCustomizers.PRE_INSERT_RECORD.getRole());
       if(preInsertCustomizer.isPresent())
       {
          AbstractPreInsertCustomizer.WhenToRun whenToRun = preInsertCustomizer.get().whenToRunPreInsert(insertInput, true);
          if(WhenToRun.BEFORE_ALL_VALIDATIONS.equals(whenToRun) || WhenToRun.BEFORE_UNIQUE_KEY_CHECKS.equals(whenToRun))
          {
-            List<QRecord> recordsAfterCustomizer = preInsertCustomizer.get().preInsert(insertInput, recordsWithoutAnyErrors, true);
+            List<QRecord> recordsAfterCustomizer = preInsertCustomizer.get().preInsert(insertInput, records, true);
             runBackendStepInput.setRecords(recordsAfterCustomizer);
 
-            ///////////////////////////////////////////////////////////////////////////////////////
+            /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-            // todo - do we care if the customizer runs both now, and in the validation below?   //
+            // so we used to have a comment here asking "do we care if the customizer runs both now, and in the validation below?" //
-            // right now we'll let it run both times, but maybe that should be protected against //
+            // when implementing Bulk Load V2, we were seeing that some customizers were adding errors to records, both now, and   //
-            ///////////////////////////////////////////////////////////////////////////////////////
+            // when they ran below.  so, at that time, we added this boolean, to track and avoid the double-run...                 //
+            // we could also imagine this being a setting on the pre-insert customizer, similar to its whenToRun attribute...      //
+            /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+            didAlreadyRunCustomizer = true;
          }
       }
 
+      ///////////////////////////////////////////////////////////////////////////////
+      // If the table has unique keys - then capture all values on these records   //
+      // for each key and set up a processSummaryLine for each of the table's UK's //
+      ///////////////////////////////////////////////////////////////////////////////
       Map<UniqueKey, Set<List<Serializable>>> existingKeys = new HashMap<>();
       List<UniqueKey>                         uniqueKeys   = CollectionUtils.nonNullList(table.getUniqueKeys());
       for(UniqueKey uniqueKey : uniqueKeys)
       {
-         existingKeys.put(uniqueKey, UniqueKeyHelper.getExistingKeys(null, table, recordsWithoutAnyErrors, uniqueKey).keySet());
+         existingKeys.put(uniqueKey, UniqueKeyHelper.getExistingKeys(null, table, records, uniqueKey).keySet());
          ukErrorSummaries.computeIfAbsent(uniqueKey, x -> new ProcessSummaryLineWithUKSampleValues(Status.ERROR));
       }
 
       /////////////////////////////////////////////////////////////////////////////////////////////////////////////////
       // on the validate step, we haven't read the full file, so we don't know how many rows there are - thus        //
       // record count is null, and the ValidateStep won't be setting status counters - so - do it here in that case. //
-      // todo - move this up (before the early return?)                                                              //
       /////////////////////////////////////////////////////////////////////////////////////////////////////////////////
       if(runBackendStepInput.getStepName().equals(StreamedETLWithFrontendProcess.STEP_NAME_VALIDATE))
       {
@@ -311,14 +261,14 @@ public class BulkInsertTransformStep extends AbstractTransformStep
       // Note, we want to do our own UK checking here, even though InsertAction also tries to do it, because InsertAction //
       // will only be getting the records in pages, but in here, we'll track UK's across pages!!                          //
       //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-      List<QRecord> recordsWithoutUkErrors = getRecordsWithoutUniqueKeyErrors(recordsWithoutAnyErrors, existingKeys, uniqueKeys, table);
+      List<QRecord> recordsWithoutUkErrors = getRecordsWithoutUniqueKeyErrors(records, existingKeys, uniqueKeys, table);
 
       /////////////////////////////////////////////////////////////////////////////////
       // run all validation from the insert action - in Preview mode (boolean param) //
       /////////////////////////////////////////////////////////////////////////////////
       insertInput.setRecords(recordsWithoutUkErrors);
       InsertAction insertAction = new InsertAction();
-      insertAction.performValidations(insertInput, true);
+      insertAction.performValidations(insertInput, true, didAlreadyRunCustomizer);
       List<QRecord> validationResultRecords = insertInput.getRecords();
 
       /////////////////////////////////////////////////////////////////
@@ -327,12 +277,28 @@ public class BulkInsertTransformStep extends AbstractTransformStep
       List<QRecord> outputRecords = new ArrayList<>();
       for(QRecord record : validationResultRecords)
       {
+         List<QErrorMessage> errorsFromAssociations = getErrorsFromAssociations(record);
+         if(CollectionUtils.nullSafeHasContents(errorsFromAssociations))
+         {
+            List<QErrorMessage> recordErrors = Objects.requireNonNullElseGet(record.getErrors(), () -> new ArrayList<>());
+            recordErrors.addAll(errorsFromAssociations);
+            record.setErrors(recordErrors);
+         }
+
          if(CollectionUtils.nullSafeHasContents(record.getErrors()))
          {
             for(QErrorMessage error : record.getErrors())
             {
-               processSummaryWarningsAndErrorsRollup.addError(error.getMessage(), null);
+               if(error instanceof AbstractBulkLoadRollableValueError rollableValueError)
-               addToErrorToExampleRowMap(error.getMessage(), record);
+               {
+                  processSummaryWarningsAndErrorsRollup.addError(rollableValueError.getMessageToUseAsProcessSummaryRollupKey(), null);
+                  addToErrorToExampleRowValueMap(rollableValueError, record);
+               }
+               else
+               {
+                  processSummaryWarningsAndErrorsRollup.addError(error.getMessage(), null);
+                  addToErrorToExampleRowMap(error.getMessage(), record);
+               }
             }
          }
          else if(CollectionUtils.nullSafeHasContents(record.getWarnings()))
@@ -356,7 +322,7 @@ public class BulkInsertTransformStep extends AbstractTransformStep
       }
 
       runBackendStepOutput.setRecords(outputRecords);
-      this.rowsProcessed += recordsInThisPage;
+      this.rowsProcessed += records.size();
    }
 
 
@@ -574,7 +540,7 @@ public class BulkInsertTransformStep extends AbstractTransformStep
             ProcessSummaryLine line          = entry.getValue();
             List<RowValue>     rowValues     = errorToExampleRowValueMap.get(message);
             String             exampleOrFull = rowValues.size() < line.getCount() ? "Example " : "";
-            line.setMessageSuffix(line.getMessageSuffix() + ".  " + exampleOrFull + "Values:");
+            line.setMessageSuffix(line.getMessageSuffix() + periodIfNeeded(line.getMessageSuffix()) + "  " + exampleOrFull + "Values:");
             line.setBulletsOfText(new ArrayList<>(rowValues.stream().map(String::valueOf).toList()));
          }
          else if(errorToExampleRowsMap.containsKey(message))
@@ -582,7 +548,7 @@ public class BulkInsertTransformStep extends AbstractTransformStep
             ProcessSummaryLine line            = entry.getValue();
             List<String>       rowDescriptions = errorToExampleRowsMap.get(message);
             String             exampleOrFull   = rowDescriptions.size() < line.getCount() ? "Example " : "";
-            line.setMessageSuffix(line.getMessageSuffix() + ".  " + exampleOrFull + "Records:");
+            line.setMessageSuffix(line.getMessageSuffix() + periodIfNeeded(line.getMessageSuffix()) + "  " + exampleOrFull + "Records:");
             line.setBulletsOfText(new ArrayList<>(rowDescriptions.stream().map(String::valueOf).toList()));
          }
       }
@@ -594,6 +560,21 @@ public class BulkInsertTransformStep extends AbstractTransformStep
 
 
 
+   /***************************************************************************
+    *
+    ***************************************************************************/
+   private String periodIfNeeded(String input)
+   {
+      if(input != null && input.matches(".*\\. *$"))
+      {
+         return ("");
+      }
+
+      return (".");
+   }
+
+
+
    /***************************************************************************
     **
     ***************************************************************************/

qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/bulk/insert/mapping/BulkLoadValueTypeError.java

@@ -44,7 +44,7 @@ public class BulkLoadValueTypeError extends AbstractBulkLoadRollableValueError
     *******************************************************************************/
    public BulkLoadValueTypeError(String fieldName, Serializable value, QFieldType type, String fieldLabel)
    {
-      super("Value [" + value + "] for field [" + fieldLabel + "] could not be converted to type [" + type + "]");
+      super("Cannot convert value [" + value + "] for field [" + fieldLabel + "] to type [" + type.getMixedCaseLabel() + "]");
       this.value = value;
       this.type = type;
       this.fieldLabel = fieldLabel;

qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/processes/implementations/bulk/insert/BulkInsertTransformStepTest.java

@@ -41,6 +41,7 @@ import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldMetaData;
 import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldType;
 import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData;
 import com.kingsrook.qqq.backend.core.model.metadata.tables.UniqueKey;
+import com.kingsrook.qqq.backend.core.model.statusmessages.BadInputStatusMessage;
 import com.kingsrook.qqq.backend.core.modules.backend.implementations.memory.MemoryRecordStore;
 import com.kingsrook.qqq.backend.core.processes.implementations.bulk.insert.mapping.BulkLoadRecordUtils;
 import com.kingsrook.qqq.backend.core.processes.implementations.bulk.insert.mapping.BulkLoadValueTypeError;
@@ -367,4 +368,61 @@ class BulkInsertTransformStepTest extends BaseTest
          .hasCount(1);
    }
 
+
+
+   /*******************************************************************************
+    **
+    *******************************************************************************/
+   @Test
+   void testPropagationOfErrorsFromAssociations() throws QException
+   {
+      ////////////////////////////////////////////////
+      // set line item lineNumber field as required //
+      ////////////////////////////////////////////////
+      QInstance instance = TestUtils.defineInstance();
+      instance.getTable(TestUtils.TABLE_NAME_LINE_ITEM).getField("lineNumber").setIsRequired(true);
+      reInitInstanceInContext(instance);
+
+      QContext.getQSession().withSecurityKeyValue(TestUtils.SECURITY_KEY_TYPE_STORE, 1);
+
+      ///////////////////////////////////////////
+      // setup & run the bulk insert transform //
+      ///////////////////////////////////////////
+      BulkInsertTransformStep bulkInsertTransformStep = new BulkInsertTransformStep();
+      RunBackendStepInput     input                   = new RunBackendStepInput();
+      RunBackendStepOutput    output                  = new RunBackendStepOutput();
+
+      input.setTableName(TestUtils.TABLE_NAME_ORDER);
+      input.setStepName(StreamedETLWithFrontendProcess.STEP_NAME_VALIDATE);
+      input.setRecords(ListBuilder.of(
+         new QRecord().withValue("storeId", 1).withAssociatedRecord("orderLine", new QRecord()),
+         new QRecord().withValue("storeId", 1).withAssociatedRecord("orderLine", new QRecord().withError(new BadInputStatusMessage("some mapping error")))
+      ));
+
+      bulkInsertTransformStep.preRun(input, output);
+      bulkInsertTransformStep.runOnePage(input, output);
+      ArrayList<ProcessSummaryLineInterface> processSummary = bulkInsertTransformStep.getProcessSummary(output, false);
+
+      ProcessSummaryAssert.assertThat(processSummary)
+         .hasLineWithMessageContaining("some mapping error")
+         .hasMessageContaining("Records:")
+         .hasStatus(Status.ERROR)
+         .hasCount(1);
+
+      ProcessSummaryAssert.assertThat(processSummary)
+         .hasLineWithMessageContaining("records were processed from the file")
+         .hasStatus(Status.INFO)
+         .hasCount(2);
+
+      ProcessSummaryAssert.assertThat(processSummary)
+         .hasLineWithMessageContaining("Order record will be inserted")
+         .hasStatus(Status.OK)
+         .hasCount(1);
+
+      ProcessSummaryAssert.assertThat(processSummary)
+         .hasLineWithMessageContaining("Order Line record will be inserted")
+         .hasStatus(Status.OK)
+         .hasCount(1);
+   }
+
 }