From 4eb28cd1b7fbcb860326920b44277d02b328f6b3 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Mon, 15 May 2023 15:07:38 -0500 Subject: [PATCH] Fix for table being added to query twice, if it's added for security, and then for being in a where clause. --- .../actions/tables/query/JoinsContext.java | 27 ++++++++++----- .../rdbms/actions/RDBMSQueryAction.java | 9 +++-- .../rdbms/actions/RDBMSQueryActionTest.java | 33 +++++++++++++++++++ 3 files changed, 59 insertions(+), 10 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/JoinsContext.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/JoinsContext.java index 2a52e634..6994dddd 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/JoinsContext.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/JoinsContext.java @@ -111,13 +111,13 @@ public class JoinsContext if(join.getLeftTable().equals(tmpTable.getName())) { QueryJoin queryJoin = new ImplicitQueryJoinForSecurityLock().withJoinMetaData(join).withType(QueryJoin.Type.INNER); - this.queryJoins.add(queryJoin); // todo something else with aliases? probably. + this.addQueryJoin(queryJoin); tmpTable = instance.getTable(join.getRightTable()); } else if(join.getRightTable().equals(tmpTable.getName())) { QueryJoin queryJoin = new ImplicitQueryJoinForSecurityLock().withJoinMetaData(join.flip()).withType(QueryJoin.Type.INNER); - this.queryJoins.add(queryJoin); // todo something else with aliases? probably. + this.addQueryJoin(queryJoin); // tmpTable = instance.getTable(join.getLeftTable()); } else @@ -145,6 +145,20 @@ public class JoinsContext + /******************************************************************************* + ** Add a query join to the list of query joins, and "process it" + ** + ** use this method to add to the list, instead of ever adding directly, as it's + ** important do to that process step (and we've had bugs when it wasn't done). + *******************************************************************************/ + private void addQueryJoin(QueryJoin queryJoin) throws QException + { + this.queryJoins.add(queryJoin); + processQueryJoin(queryJoin); + } + + + /******************************************************************************* ** If there are any joins in the context that don't have a join meta data, see ** if we can find the JoinMetaData to use for them by looking at the main table's @@ -236,8 +250,7 @@ public class JoinsContext QueryJoin queryJoinToAdd = makeQueryJoinFromJoinAndTableNames(nextTable, tmpTable, joinToAdd); queryJoinToAdd.setType(queryJoin.getType()); addedAnyQueryJoins = true; - this.queryJoins.add(queryJoinToAdd); // todo something else with aliases? probably. - processQueryJoin(queryJoinToAdd); + this.addQueryJoin(queryJoin); } } @@ -410,8 +423,7 @@ public class JoinsContext QueryJoin queryJoin = makeQueryJoinFromJoinAndTableNames(mainTableName, filterTable, join); if(queryJoin != null) { - this.queryJoins.add(queryJoin); // todo something else with aliases? probably. - processQueryJoin(queryJoin); + this.addQueryJoin(queryJoin); found = true; break; } @@ -420,8 +432,7 @@ public class JoinsContext if(!found) { QueryJoin queryJoin = new QueryJoin().withJoinTable(filterTable).withType(QueryJoin.Type.INNER); - this.queryJoins.add(queryJoin); // todo something else with aliases? probably. - processQueryJoin(queryJoin); + this.addQueryJoin(queryJoin); } } } diff --git a/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryAction.java b/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryAction.java index 4522282f..c82a18d4 100644 --- a/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryAction.java +++ b/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryAction.java @@ -131,10 +131,10 @@ public class RDBMSQueryAction extends AbstractRDBMSAction implements QueryInterf } } + Long mark = System.currentTimeMillis(); + try { - Long mark = System.currentTimeMillis(); - ////////////////////////////////////////////// // execute the query - iterate over results // ////////////////////////////////////////////// @@ -173,6 +173,11 @@ public class RDBMSQueryAction extends AbstractRDBMSAction implements QueryInterf return queryOutput; } + catch(Exception e) + { + logSQL(sql, params, mark); + throw (e); + } finally { if(needToCloseConnection) diff --git a/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryActionTest.java b/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryActionTest.java index 692ea261..3d49f858 100644 --- a/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryActionTest.java +++ b/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryActionTest.java @@ -1416,6 +1416,39 @@ public class RDBMSQueryActionTest extends RDBMSActionTest + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testRecordSecurityFromJoinTableAlsoImplicitlyInQuery() throws QException + { + QueryInput queryInput = new QueryInput(); + queryInput.setTableName(TestUtils.TABLE_NAME_ORDER_LINE); + + /////////////////////////////////////////////////////////////////////////////////////////// + // orders 1, 2, and 3 are from store 1, so their lines (5 in total) should be found. // + // note, order 2 has the line with mis-matched store id - but, that shouldn't apply here // + /////////////////////////////////////////////////////////////////////////////////////////// + queryInput.setFilter(new QQueryFilter(new QFilterCriteria("order.id", QCriteriaOperator.IN, List.of(1, 2, 3, 4)))); + QContext.setQSession(new QSession().withSecurityKeyValue(TestUtils.TABLE_NAME_STORE, 1)); + assertThat(new QueryAction().execute(queryInput).getRecords()).hasSize(5); + + /////////////////////////////////////////////////////////////////// + // order 4 should be the only one found this time (with 2 lines) // + /////////////////////////////////////////////////////////////////// + queryInput.setFilter(new QQueryFilter(new QFilterCriteria("order.id", QCriteriaOperator.IN, List.of(1, 2, 3, 4)))); + QContext.setQSession(new QSession().withSecurityKeyValue(TestUtils.TABLE_NAME_STORE, 2)); + assertThat(new QueryAction().execute(queryInput).getRecords()).hasSize(2); + + //////////////////////////////////////////////////////////////// + // make sure we're also good if we explicitly join this table // + //////////////////////////////////////////////////////////////// + queryInput.withQueryJoin(new QueryJoin().withJoinTable(TestUtils.TABLE_NAME_ORDER).withSelect(true)); + assertThat(new QueryAction().execute(queryInput).getRecords()).hasSize(2); + } + + + /******************************************************************************* ** *******************************************************************************/