QQQ-21 feedback from code review

pom.xml

@@ -53,7 +53,7 @@
       <dependency>
          <groupId>com.kingsrook.qqq</groupId>
          <artifactId>qqq-backend-core</artifactId>
-         <version>0.1.0-20220711.141150-7</version>
+         <version>0.1.0-20220712.140206-8</version>
       </dependency>
       <dependency>
          <groupId>com.kingsrook.qqq</groupId>

src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementation.java

@@ -98,13 +98,14 @@ public class QJavalinImplementation
 
    private static final int SESSION_COOKIE_AGE = 60 * 60 * 24;
 
-   protected static QInstance qInstance;
+   static QInstance qInstance;
 
    private static int DEFAULT_PORT = 8001;
 
    private static Javalin service;
 
 
+
    /*******************************************************************************
     **
     *******************************************************************************/
@@ -153,6 +154,7 @@ public class QJavalinImplementation
    }
 
 
+
    /*******************************************************************************
     **
     *******************************************************************************/
@@ -363,6 +365,9 @@ public class QJavalinImplementation
          setupSession(context, queryRequest);
          queryRequest.setTableName(tableName);
 
+         // todo - validate that the primary key is of the proper type (e.g,. not a string for an id field)
+         //  and throw a 400-series error (tell the user bad-request), rather than, we're doing a 500 (server error)
+
          ///////////////////////////////////////////////////////
          // setup a filter for the primaryKey = the path-pram //
          ///////////////////////////////////////////////////////

src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinProcessHandler.java

@@ -53,8 +53,6 @@ import com.kingsrook.qqq.backend.core.model.metadata.QFieldMetaData;
 import com.kingsrook.qqq.backend.core.model.metadata.QInstance;
 import com.kingsrook.qqq.backend.core.model.metadata.QTableMetaData;
 import com.kingsrook.qqq.backend.core.model.metadata.processes.QProcessMetaData;
-import com.kingsrook.qqq.backend.core.state.StateType;
-import com.kingsrook.qqq.backend.core.state.UUIDAndTypeStateKey;
 import com.kingsrook.qqq.backend.core.utils.CollectionUtils;
 import com.kingsrook.qqq.backend.core.utils.ExceptionUtils;
 import com.kingsrook.qqq.backend.core.utils.JsonUtils;
@@ -145,29 +143,27 @@ public class QJavalinProcessHandler
          runProcessRequest.setStartAfterStep(startAfterStep);
          populateRunProcessRequestWithValuesFromContext(context, runProcessRequest);
 
-         try
+         ////////////////////////////////////////
+         // run the process as an async action //
+         ////////////////////////////////////////
+         Integer timeout = getTimeoutMillis(context);
+         RunProcessResult runProcessResult = new AsyncJobManager().startJob(timeout, TimeUnit.MILLISECONDS, (callback) ->
          {
-            ////////////////////////////////////////
+            runProcessRequest.setAsyncJobCallback(callback);
-            // run the process as an async action //
+            return (new RunProcessAction().execute(runProcessRequest));
-            ////////////////////////////////////////
+         });
-            Integer timeout = getTimeoutMillis(context);
-            RunProcessResult runProcessResult = new AsyncJobManager().startJob(timeout, TimeUnit.MILLISECONDS, (callback) ->
-            {
-               runProcessRequest.setAsyncJobCallback(callback);
-               return (new RunProcessAction().execute(runProcessRequest));
-            });
 
-            LOG.info("Process result error? " + runProcessResult.getException());
+         LOG.info("Process result error? " + runProcessResult.getException());
-            for(QFieldMetaData outputField : QJavalinImplementation.qInstance.getProcess(runProcessRequest.getProcessName()).getOutputFields())
+         for(QFieldMetaData outputField : QJavalinImplementation.qInstance.getProcess(runProcessRequest.getProcessName()).getOutputFields())
-            {
-               LOG.info("Process result output value: " + outputField.getName() + ": " + runProcessResult.getValues().get(outputField.getName()));
-            }
-            serializeRunProcessResultForCaller(resultForCaller, runProcessResult);
-         }
-         catch(JobGoingAsyncException jgae)
          {
-            resultForCaller.put("jobUUID", jgae.getJobUUID());
+            LOG.info("Process result output value: " + outputField.getName() + ": " + runProcessResult.getValues().get(outputField.getName()));
          }
+
+         serializeRunProcessResultForCaller(resultForCaller, runProcessResult);
+      }
+      catch(JobGoingAsyncException jgae)
+      {
+         resultForCaller.put("jobUUID", jgae.getJobUUID());
       }
       catch(Exception e)
       {
@@ -199,7 +195,7 @@ public class QJavalinProcessHandler
          serializeRunProcessExceptionForCaller(resultForCaller, runProcessResult.getException().get());
       }
       resultForCaller.put("values", runProcessResult.getValues());
-      runProcessResult.getProcessState().getNextStepName().ifPresent(lastStep -> resultForCaller.put("nextStep", lastStep));
+      runProcessResult.getProcessState().getNextStepName().ifPresent(nextStep -> resultForCaller.put("nextStep", nextStep));
    }
 
 
@@ -297,13 +293,13 @@ public class QJavalinProcessHandler
          {
             case "recordIds":
                @SuppressWarnings("ConstantConditions")
-               Serializable[] idStrings = context.queryParam(recordsParam).split(",");
+               Serializable[] idStrings = paramValue.split(",");
                return (new QQueryFilter().withCriteria(new QFilterCriteria()
                   .withFieldName(primaryKeyField)
                   .withOperator(QCriteriaOperator.IN)
                   .withValues(Arrays.stream(idStrings).toList())));
             case "filterJSON":
-               return (JsonUtils.toObject(context.queryParam(recordsParam), QQueryFilter.class));
+               return (JsonUtils.toObject(paramValue, QQueryFilter.class));
             case "filterId":
                // return (JsonUtils.toObject(context.queryParam(recordsParam), QQueryFilter.class));
                throw (new NotImplementedException("Saved filters are not yet implemented."));
@@ -335,19 +331,20 @@ public class QJavalinProcessHandler
     *******************************************************************************/
    public static void processStatus(Context context)
    {
+      Map<String, Object> resultForCaller = new HashMap<>();
+
       String processUUID = context.pathParam("processUUID");
       String jobUUID     = context.pathParam("jobUUID");
 
-      LOG.info("Request for status of job " + jobUUID);
+      LOG.info("Request for status of process " + processUUID + ", job " + jobUUID);
       Optional<AsyncJobStatus> optionalJobStatus = new AsyncJobManager().getJobStatus(jobUUID);
       if(optionalJobStatus.isEmpty())
       {
-         QJavalinImplementation.handleException(context, new RuntimeException("Could not find status of process step job"));
+         serializeRunProcessExceptionForCaller(resultForCaller, new RuntimeException("Could not find status of process step job"));
       }
       else
       {
-         Map<String, Object> resultForCaller = new HashMap<>();
+         AsyncJobStatus jobStatus = optionalJobStatus.get();
-         AsyncJobStatus      jobStatus       = optionalJobStatus.get();
 
          resultForCaller.put("jobStatus", jobStatus);
          LOG.info("Job status is " + jobStatus.getState() + " for " + jobUUID);
@@ -358,7 +355,7 @@ public class QJavalinProcessHandler
             // if the job is complete, get the process result from state provider, and return it //
             // this output should look like it did if the job finished synchronously!!           //
             ///////////////////////////////////////////////////////////////////////////////////////
-            Optional<ProcessState> processState = RunProcessAction.getStateProvider().get(ProcessState.class, new UUIDAndTypeStateKey(UUID.fromString(processUUID), StateType.PROCESS_STATUS));
+            Optional<ProcessState> processState = RunProcessAction.getState(processUUID);
             if(processState.isPresent())
             {
                RunProcessResult runProcessResult = new RunProcessResult(processState.get());
@@ -366,7 +363,7 @@ public class QJavalinProcessHandler
             }
             else
             {
-               QJavalinImplementation.handleException(context, new RuntimeException("Could not find process results"));
+               serializeRunProcessExceptionForCaller(resultForCaller, new RuntimeException("Could not find results for process " + processUUID));
             }
          }
          else if(jobStatus.getState().equals(AsyncJobState.ERROR))
@@ -379,9 +376,9 @@ public class QJavalinProcessHandler
                serializeRunProcessExceptionForCaller(resultForCaller, jobStatus.getCaughtException());
             }
          }
-
-         context.result(JsonUtils.toJson(resultForCaller));
       }
+
+      context.result(JsonUtils.toJson(resultForCaller));
    }
 
 
@@ -397,7 +394,9 @@ public class QJavalinProcessHandler
          Integer skip        = Objects.requireNonNullElse(QJavalinImplementation.integerQueryParam(context, "skip"), 0);
          Integer limit       = Objects.requireNonNullElse(QJavalinImplementation.integerQueryParam(context, "limit"), 20);
 
-         Optional<ProcessState> optionalProcessState = RunProcessAction.getStateProvider().get(ProcessState.class, new UUIDAndTypeStateKey(UUID.fromString(processUUID), StateType.PROCESS_STATUS));
+         // todo - potential optimization - if a future state provider could take advantage of it,
+         //  we might pass the skip & limit in to a method that fetch just those 'n' rows from state, rather than the whole thing?
+         Optional<ProcessState> optionalProcessState = RunProcessAction.getState(processUUID);
          if(optionalProcessState.isEmpty())
          {
             throw (new Exception("Could not find process results."));
@@ -411,7 +410,7 @@ public class QJavalinProcessHandler
          }
 
          Map<String, Object> resultForCaller = new HashMap<>();
-         List<QRecord> recordPage = CollectionUtils.safelyGetPage(records, skip, limit);
+         List<QRecord>       recordPage      = CollectionUtils.safelyGetPage(records, skip, limit);
          resultForCaller.put("records", recordPage);
          context.result(JsonUtils.toJson(resultForCaller));
       }

src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinProcessHandlerTest.java

@@ -47,7 +47,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
  *******************************************************************************/
 class QJavalinProcessHandlerTest extends QJavalinTestBase
 {
-   private static final int MORE_THAN_TIMEOUT = 1000;
+   private static final int MORE_THAN_TIMEOUT = 500;
    private static final int LESS_THAN_TIMEOUT = 50;
 
 
@@ -96,8 +96,9 @@ class QJavalinProcessHandlerTest extends QJavalinTestBase
       assertEquals(200, response.getStatus());
       JSONObject jsonObject = JsonUtils.toJSONObject(response.getBody());
       assertNotNull(jsonObject);
+      String processUUID = jsonObject.getString("processUUID");
 
-      // todo - once we know how to get records from a process, add that call
+      getProcessRecords(processUUID, 2);
    }
 
 
@@ -120,8 +121,45 @@ class QJavalinProcessHandlerTest extends QJavalinTestBase
       assertEquals(200, response.getStatus());
       JSONObject jsonObject = JsonUtils.toJSONObject(response.getBody());
       assertNotNull(jsonObject);
+      String processUUID = jsonObject.getString("processUUID");
 
-      // todo - once we know how to get records from a process, add that call
+      getProcessRecords(processUUID, 3);
+   }
+
+
+
+   /*******************************************************************************
+    **
+    *******************************************************************************/
+   private JSONObject getProcessRecords(String processUUID, int expectedNoOfRecords)
+   {
+      return (getProcessRecords(processUUID, expectedNoOfRecords, 0, 20));
+   }
+
+
+
+   /*******************************************************************************
+    **
+    *******************************************************************************/
+   private JSONObject getProcessRecords(String processUUID, int expectedNoOfRecords, int skip, int limit)
+   {
+      HttpResponse<String> response;
+      JSONObject           jsonObject;
+      response = Unirest.get(BASE_URL + "/processes/greet/" + processUUID + "/records?skip=" + skip + "&limit=" + limit).asString();
+      jsonObject = JsonUtils.toJSONObject(response.getBody());
+      assertNotNull(jsonObject);
+
+      if(expectedNoOfRecords == 0)
+      {
+         assertFalse(jsonObject.has("records"));
+      }
+      else
+      {
+         assertTrue(jsonObject.has("records"));
+         JSONArray records = jsonObject.getJSONArray("records");
+         assertEquals(expectedNoOfRecords, records.length());
+      }
+      return (jsonObject);
    }
 
 
@@ -133,7 +171,7 @@ class QJavalinProcessHandlerTest extends QJavalinTestBase
    @Test
    public void test_processGreetInitWithQueryValues()
    {
-      HttpResponse<String> response = Unirest.get(BASE_URL + "/processes/greet/init?recordsParam=recordIds&recordIds=2,3&greetingPrefix=Hey&greetingSuffix=Jude").asString();
+      HttpResponse<String> response = Unirest.post(BASE_URL + "/processes/greet/init?recordsParam=recordIds&recordIds=2,3&greetingPrefix=Hey&greetingSuffix=Jude").asString();
       assertEquals(200, response.getStatus());
       JSONObject jsonObject = JsonUtils.toJSONObject(response.getBody());
       assertNotNull(jsonObject);
@@ -321,7 +359,7 @@ class QJavalinProcessHandlerTest extends QJavalinTestBase
 
 
    /*******************************************************************************
-    ** every time a process step (sync or async) has gone async, expect what the
+    ** every time a process step (or init) has gone async, expect what the
     ** response should look like
     *******************************************************************************/
    private JSONObject assertProcessStepWentAsyncResponse(HttpResponse<String> response)
@@ -412,16 +450,30 @@ class QJavalinProcessHandlerTest extends QJavalinTestBase
       assertNotNull(jsonObject);
       String processUUID = jsonObject.getString("processUUID");
 
-      response = Unirest.get(BASE_URL + "/processes/greet/" + processUUID + "/records").asString();
+      jsonObject = getProcessRecords(processUUID, 2);
-      jsonObject = JsonUtils.toJSONObject(response.getBody());
+      JSONArray  records = jsonObject.getJSONArray("records");
-      assertNotNull(jsonObject);
-      assertTrue(jsonObject.has("records"));
-      JSONArray records = jsonObject.getJSONArray("records");
-      assertEquals(2, records.length());
       JSONObject record0 = records.getJSONObject(0);
       JSONObject values  = record0.getJSONObject("values");
       assertTrue(values.has("id"));
       assertTrue(values.has("firstName"));
    }
 
+
+
+   /*******************************************************************************
+    ** test getting records back from a process with skip & Limit
+    **
+    *******************************************************************************/
+   @Test
+   public void test_processRecordsSkipAndLimit()
+   {
+      HttpResponse<String> response    = Unirest.get(BASE_URL + "/processes/greet/init?recordsParam=recordIds&recordIds=1,2,3,4,5").asString();
+      JSONObject           jsonObject  = JsonUtils.toJSONObject(response.getBody());
+      String               processUUID = jsonObject.getString("processUUID");
+
+      getProcessRecords(processUUID, 5);
+      getProcessRecords(processUUID, 1, 4, 5);
+      getProcessRecords(processUUID, 0, 5, 5);
+   }
+
 }

src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinTestBase.java

@@ -47,7 +47,7 @@ public class QJavalinTestBase
    public static void beforeAll()
    {
       qJavalinImplementation = new QJavalinImplementation(TestUtils.defineInstance());
-      QJavalinProcessHandler.setAsyncStepTimeoutMillis(500);
+      QJavalinProcessHandler.setAsyncStepTimeoutMillis(250);
       qJavalinImplementation.startJavalinServer(PORT);
    }