Replace TestService.get calls when not required

Signed-off-by: Andrea Lamparelli <[email protected]>

Author: lampajr

horreum-backend/src/main/java/io/hyperfoil/tools/horreum/svc/TestServiceImpl.java

@@ -7,6 +7,7 @@
 import com.fasterxml.jackson.databind.node.JsonNodeType;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import io.hyperfoil.tools.horreum.api.SortDirection;
+import io.hyperfoil.tools.horreum.api.alerting.Change;
 import io.hyperfoil.tools.horreum.api.data.Access;
 import io.hyperfoil.tools.horreum.api.data.ExportedLabelValues;
 import io.hyperfoil.tools.horreum.api.data.Fingerprints;
@@ -41,6 +42,7 @@
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.inject.Inject;
 import jakarta.persistence.EntityManager;
+import jakarta.persistence.NoResultException;
 import jakarta.persistence.PersistenceException;
 import jakarta.persistence.Tuple;
 import jakarta.transaction.TransactionManager;
@@ -91,6 +93,7 @@ public class TestServiceImpl implements TestService {
    protected static final String LABEL_ORDER_STOP= "combined.stop";
    protected static final String LABEL_ORDER_JSONPATH = "jsonb_path_query(combined.values,CAST( :orderBy as jsonpath))";
 
+   private static final String COUNT_TEST_BY_ID_QUERY = "SELECT count(id) FROM test WHERE id = ?1";
    protected static final String LABEL_VALUES_QUERY = """
          WITH
          combined as (
@@ -183,6 +186,19 @@ public Test getByNameOrId(String input){
       return TestMapper.from(test);
    }
 
+   /**
+    * Checks whether the provided id belongs to an existing test and if the user can access it
+    * the security check is performed by triggering the RLS at database level
+    * @param id test ID
+    */
+   @WithRoles
+   @Transactional
+   protected boolean checkTestExists(int id) {
+      return 0 != em.createQuery(COUNT_TEST_BY_ID_QUERY, Long.class)
+            .setParameter(1, id)
+            .getSingleResult();
+   }
+
    @WithRoles(extras = Roles.HORREUM_SYSTEM)
    public TestDAO ensureTestExists(String testNameOrId, String token){
       TestDAO test;// = TestMapper.to(getByNameOrId(input)); //why does getByNameOrId not work to create the DAO?
@@ -572,8 +588,7 @@ public void updateFolder(int id, String folder) {
    @SuppressWarnings("unchecked")
    @Override
    public List<Fingerprints> listFingerprints(int testId) {
-      Test test = get(testId,null);
-      if(test == null){
+      if(!checkTestExists(testId)){
          throw ServiceException.serverError("Cannot find test "+testId);
       }
       return Fingerprints.parse( em.createNativeQuery("""
@@ -582,7 +597,7 @@ public List<Fingerprints> listFingerprints(int testId) {
             JOIN dataset ON dataset.id = dataset_id
             WHERE dataset.testid = ?1
             """)
-            .setParameter(1, test.id)
+            .setParameter(1, testId)
             .unwrap(NativeQuery.class).addScalar("fingerprint", JsonBinaryType.INSTANCE)
             .getResultList());
    }
@@ -721,8 +736,7 @@ protected static FilterDef getFilterDef(String filter, Instant before, Instant a
    @WithRoles
    @Override
    public List<ExportedLabelValues> labelValues(int testId, String filter, String before, String after, boolean filtering, boolean metrics, String sort, String direction, int limit, int page, List<String> include, List<String> exclude, boolean multiFilter) {
-      Test test = get(testId,null);
-      if(test == null){
+      if(!checkTestExists(testId)){
          throw ServiceException.serverError("Cannot find test "+testId);
       }
       Object filterObject = Util.getFilterObject(filter);
@@ -785,7 +799,7 @@ public List<ExportedLabelValues> labelValues(int testId, String filter, String b
                   .replace("ORDER_PLACEHOLDER",orderSql);
 
            NativeQuery query =  ((NativeQuery) em.createNativeQuery(sql))
-             .setParameter("testId", test.id)
+             .setParameter("testId", testId)
              .setParameter("filteringLabels", filtering)
              .setParameter("metricLabels", metrics)
              ;
@@ -924,15 +938,14 @@ public void recalculateDatasets(int testId) {
    @Override
    @WithRoles
    public RecalculationStatus getRecalculationStatus(int testId) {
-      Test test = get(testId,null);
-      if(test == null){
+      if(!checkTestExists(testId)){
          throw ServiceException.serverError("Cannot find test "+testId);
       }
-      RecalculationStatus status = recalculations.get(test.id);
+      RecalculationStatus status = recalculations.get(testId);
       if (status == null) {
-         status = new RecalculationStatus(RunDAO.count("testid = ?1 AND trashed = false", test.id));
+         status = new RecalculationStatus(RunDAO.count("testid = ?1 AND trashed = false", testId));
          status.finished = status.totalRuns;
-         status.datasets = DatasetDAO.count("testid", test.id);
+         status.datasets = DatasetDAO.count("testid", testId);
       }
       return status;
    }

horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/TestServiceNoRestTest.java

@@ -32,9 +32,11 @@
 import static io.hyperfoil.tools.horreum.svc.BaseServiceNoRestTest.FOO_TEAM;
 import static io.hyperfoil.tools.horreum.svc.BaseServiceNoRestTest.FOO_TESTER;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 @QuarkusTest
 @QuarkusTestResource(PostgresResource.class)
@@ -160,6 +162,21 @@ void testCreateTestWithExistingName() {
       assertEquals(Response.Status.CONFLICT.getStatusCode(), thrown.getResponse().getStatus());
    }
 
+   @org.junit.jupiter.api.Test
+   void testCheckTestExists() {
+      Test t1 = createSampleTest("test", null, null, null);
+      Test t2 = createSampleTest("1234", null, null, null);
+
+      Test created1 = testService.add(t1);
+      assertNotNull(created1.id);
+      Test created2 = testService.add(t2);
+      assertNotNull(created2.id);
+
+      assertTrue(((TestServiceImpl) testService).checkTestExists(created1.id));
+      assertTrue(((TestServiceImpl) testService).checkTestExists(created2.id));
+      assertFalse(((TestServiceImpl) testService).checkTestExists(9999));
+   }
+
    @org.junit.jupiter.api.Test
    void testUpdateTest() {
       Test t = createSampleTest("test", null, null, null);