CE-1654 - Update joins built for security-purposes, that if they're for an all-access key, to be outer (LEFT); update tests to reflect this

qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/JoinsContext.java

@@ -82,7 +82,7 @@ public class JoinsContext
    /////////////////////////////////////////////////////////////////////////////
    // we will get a TON of more output if this gets turned up, so be cautious //
    /////////////////////////////////////////////////////////////////////////////
-   private Level logLevel = Level.OFF;
+   private Level logLevel          = Level.OFF;
    private Level logLevelForFilter = Level.OFF;
 
 
@@ -404,6 +404,12 @@ public class JoinsContext
                chainIsInner = false;
             }
 
+            if(hasAllAccessKey(recordSecurityLock))
+            {
+               queryJoin.withType(QueryJoin.Type.LEFT);
+               chainIsInner = false;
+            }
+
             addQueryJoin(queryJoin, "forRecordSecurityLock (non-flipped)", "- ");
             addedQueryJoins.add(queryJoin);
             tmpTable = instance.getTable(join.getRightTable());
@@ -423,6 +429,12 @@ public class JoinsContext
                chainIsInner = false;
             }
 
+            if(hasAllAccessKey(recordSecurityLock))
+            {
+               queryJoin.withType(QueryJoin.Type.LEFT);
+               chainIsInner = false;
+            }
+
             addQueryJoin(queryJoin, "forRecordSecurityLock (flipped)", "- ");
             addedQueryJoins.add(queryJoin);
             tmpTable = instance.getTable(join.getLeftTable());
@@ -456,44 +468,53 @@ public class JoinsContext
 
 
 
+   /***************************************************************************
+    **
+    ***************************************************************************/
+   private boolean hasAllAccessKey(RecordSecurityLock recordSecurityLock)
+   {
+      //////////////////////////////////////////////////////////////////////////////////////////////////////////////
+      // check if the key type has an all-access key, and if so, if it's set to true for the current user/session //
+      //////////////////////////////////////////////////////////////////////////////////////////////////////////////
+      QSecurityKeyType securityKeyType = instance.getSecurityKeyType(recordSecurityLock.getSecurityKeyType());
+      if(StringUtils.hasContent(securityKeyType.getAllAccessKeyName()))
+      {
+         QSession session = QContext.getQSession();
+         if(session.hasSecurityKeyValue(securityKeyType.getAllAccessKeyName(), true, QFieldType.BOOLEAN))
+         {
+            return (true);
+         }
+      }
+
+      return (false);
+   }
+
+
+
    /*******************************************************************************
     **
     *******************************************************************************/
    private void addSubFilterForRecordSecurityLock(RecordSecurityLock recordSecurityLock, QTableMetaData table, String tableNameOrAlias, boolean isOuter, QueryJoin sourceQueryJoin)
    {
-      QSession session = QContext.getQSession();
+      boolean haveAllAccessKey = hasAllAccessKey(recordSecurityLock);
-
+      if(haveAllAccessKey)
-      //////////////////////////////////////////////////////////////////////////////////////////////////////////////
-      // check if the key type has an all-access key, and if so, if it's set to true for the current user/session //
-      //////////////////////////////////////////////////////////////////////////////////////////////////////////////
-      QSecurityKeyType securityKeyType  = instance.getSecurityKeyType(recordSecurityLock.getSecurityKeyType());
-      boolean          haveAllAccessKey = false;
-      if(StringUtils.hasContent(securityKeyType.getAllAccessKeyName()))
       {
-         //////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+         if(sourceQueryJoin != null)
-         // if we have all-access on this key, then we don't need a criterion for it (as long as we're in an AND filter) //
-         //////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-         if(session.hasSecurityKeyValue(securityKeyType.getAllAccessKeyName(), true, QFieldType.BOOLEAN))
          {
-            haveAllAccessKey = true;
+            ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+            // in case the queryJoin object is re-used between queries, and its security criteria need to be different (!!), reset it //
+            // this can be exposed in tests - maybe not entirely expected in real-world, but seems safe enough                        //
+            ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+            sourceQueryJoin.withSecurityCriteria(new ArrayList<>());
+         }
 
-            if(sourceQueryJoin != null)
+         ////////////////////////////////////////////////////////////////////////////////////////
-            {
+         // if we're in an AND filter, then we don't need a criteria for this lock, so return. //
-               ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+         ////////////////////////////////////////////////////////////////////////////////////////
-               // in case the queryJoin object is re-used between queries, and its security criteria need to be different (!!), reset it //
+         boolean inAnAndFilter = securityFilterCursor.getBooleanOperator() == QQueryFilter.BooleanOperator.AND;
-               // this can be exposed in tests - maybe not entirely expected in real-world, but seems safe enough                        //
+         if(inAnAndFilter)
-               ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+         {
-               sourceQueryJoin.withSecurityCriteria(new ArrayList<>());
+            return;
-            }
-
-            ////////////////////////////////////////////////////////////////////////////////////////
-            // if we're in an AND filter, then we don't need a criteria for this lock, so return. //
-            ////////////////////////////////////////////////////////////////////////////////////////
-            boolean inAnAndFilter = securityFilterCursor.getBooleanOperator() == QQueryFilter.BooleanOperator.AND;
-            if(inAnAndFilter)
-            {
-               return;
-            }
          }
       }
 
@@ -545,7 +566,7 @@ public class JoinsContext
       }
       else
       {
-         List<Serializable> securityKeyValues = session.getSecurityKeyValues(recordSecurityLock.getSecurityKeyType(), type);
+         List<Serializable> securityKeyValues = QContext.getQSession().getSecurityKeyValues(recordSecurityLock.getSecurityKeyType(), type);
          if(CollectionUtils.nullSafeIsEmpty(securityKeyValues))
          {
             ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -1193,15 +1214,15 @@ public class JoinsContext
 
       if(isStart)
       {
-         System.out.println("\n" + StringUtils.safeTruncate("--- Start [main table: " + this.mainTableName + "] " + "-".repeat(full), full));
+         System.out.println("\n" + StringUtils.safeTruncate("--- Start [main table: " + this.mainTableName + "] " + "-" .repeat(full), full));
       }
 
       StringBuilder rs           = new StringBuilder();
       String        formatString = "| %-" + md + "s | %-" + md + "s %-" + md + "s | %-" + lg + "s | %-" + sm + "s |\n";
       rs.append(String.format(formatString, "Base Table", "Join Table", "(Alias)", "Join Meta Data", "Type"));
-      String dashesLg = "-".repeat(lg);
+      String dashesLg = "-" .repeat(lg);
-      String dashesMd = "-".repeat(md);
+      String dashesMd = "-" .repeat(md);
-      String dashesSm = "-".repeat(sm);
+      String dashesSm = "-" .repeat(sm);
       rs.append(String.format(formatString, dashesMd, dashesMd, dashesMd, dashesLg, dashesSm));
       if(CollectionUtils.nullSafeHasContents(queryJoins))
       {
@@ -1227,11 +1248,11 @@ public class JoinsContext
 
       if(isEnd)
       {
-         System.out.println(StringUtils.safeTruncate("--- End " + "-".repeat(full), full) + "\n");
+         System.out.println(StringUtils.safeTruncate("--- End " + "-" .repeat(full), full) + "\n");
       }
       else
       {
-         System.out.println(StringUtils.safeTruncate("-".repeat(full), full));
+         System.out.println(StringUtils.safeTruncate("-" .repeat(full), full));
       }
    }
 }

qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSCountActionTest.java

@@ -212,7 +212,7 @@ public class RDBMSCountActionTest extends RDBMSActionTest
       CountInput countInput = new CountInput();
       countInput.setTableName(TestUtils.TABLE_NAME_WAREHOUSE);
 
-      assertThat(new CountAction().execute(countInput).getCount()).isEqualTo(1);
+      assertThat(new CountAction().execute(countInput).getCount()).isEqualTo(4);
    }
 
 }

qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryActionJoinsTest.java

@@ -771,6 +771,61 @@ public class RDBMSQueryActionJoinsTest extends RDBMSActionTest
 
 
 
+   /*******************************************************************************
+    ** Error seen in CTLive - query for a record in a sub-table, but whose security
+    ** key comes from a main table, but the main-table record doesn't exist.
+    **
+    ** In this QInstance, our warehouse table's security key comes from
+    ** storeWarehouseInt.storeId - so if we insert a warehouse, but no stores, we
+    ** might not be able to find it (if this bug exists!)
+    *******************************************************************************/
+   @Test
+   void testRequestedJoinWithTableWhoseSecurityFieldIsInMainTableAndNoRowIsInMainTable() throws Exception
+   {
+      runTestSql("INSERT INTO warehouse (name) VALUES ('Springfield')", null);
+
+      QueryInput queryInput = new QueryInput();
+      queryInput.setTableName(TestUtils.TABLE_NAME_WAREHOUSE);
+      queryInput.setFilter(new QQueryFilter(new QFilterCriteria("name", QCriteriaOperator.EQUALS, "Springfield")));
+
+      /////////////////////////////////////////
+      // with all access key, should find it //
+      /////////////////////////////////////////
+      QContext.setQSession(new QSession().withSecurityKeyValue(TestUtils.SECURITY_KEY_STORE_ALL_ACCESS, true));
+      assertThat(new QueryAction().execute(queryInput).getRecords()).hasSize(1);
+
+      ////////////////////////////////////////////
+      // with a regular key, should not find it //
+      ////////////////////////////////////////////
+      QContext.setQSession(new QSession().withSecurityKeyValue(TestUtils.TABLE_NAME_STORE, 1));
+      assertThat(new QueryAction().execute(queryInput).getRecords()).hasSize(0);
+
+      /////////////////////////////////////////
+      // now assign the warehouse to a store //
+      /////////////////////////////////////////
+      runTestSql("INSERT INTO warehouse_store_int (store_id, warehouse_id) SELECT 1, id FROM warehouse WHERE name='Springfield'", null);
+
+      /////////////////////////////////////////
+      // with all access key, should find it //
+      /////////////////////////////////////////
+      QContext.setQSession(new QSession().withSecurityKeyValue(TestUtils.SECURITY_KEY_STORE_ALL_ACCESS, true));
+      assertThat(new QueryAction().execute(queryInput).getRecords()).hasSize(1);
+
+      ///////////////////////////////////////////////////////
+      // with a regular key, should find it if key matches //
+      ///////////////////////////////////////////////////////
+      QContext.setQSession(new QSession().withSecurityKeyValue(TestUtils.TABLE_NAME_STORE, 1));
+      assertThat(new QueryAction().execute(queryInput).getRecords()).hasSize(1);
+
+      //////////////////////////////////////////////////////////////////
+      // with a regular key, should not find it if key does not match //
+      //////////////////////////////////////////////////////////////////
+      QContext.setQSession(new QSession().withSecurityKeyValue(TestUtils.TABLE_NAME_STORE, 2));
+      assertThat(new QueryAction().execute(queryInput).getRecords()).hasSize(0);
+
+   }
+
+
    /*******************************************************************************
     **
     *******************************************************************************/
@@ -888,7 +943,10 @@ public class RDBMSQueryActionJoinsTest extends RDBMSActionTest
 
 
    /*******************************************************************************
-    **
+    ** Note, this test was originally written asserting size=1... but reading
+    ** the data, for an all-access key, that seems wrong - as the user should see
+    ** all the records in this table, not just ones associated with a store...
+    ** so, switching to 4 (same issue in CountActionTest too).
     *******************************************************************************/
    @Test
    void testRecordSecurityWithLockFromJoinTableWhereTheKeyIsOnTheManySide() throws QException
@@ -897,8 +955,9 @@ public class RDBMSQueryActionJoinsTest extends RDBMSActionTest
       QueryInput queryInput = new QueryInput();
       queryInput.setTableName(TestUtils.TABLE_NAME_WAREHOUSE);
 
-      assertThat(new QueryAction().execute(queryInput).getRecords())
+      List<QRecord> records = new QueryAction().execute(queryInput).getRecords();
-         .hasSize(1);
+      assertThat(records)
+         .hasSize(4);
    }