Fixing a case in query joins, where a joinMetaData was given, but it needed flipped.

This commit is contained in:
2023-08-08 13:18:13 -05:00
parent d811ed725d
commit c548952281
4 changed files with 255 additions and 57 deletions

View File

@ -30,8 +30,10 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import com.kingsrook.qqq.backend.core.context.QContext;
import com.kingsrook.qqq.backend.core.exceptions.QException;
import com.kingsrook.qqq.backend.core.logging.LogPair;
import com.kingsrook.qqq.backend.core.logging.QLogger;
import com.kingsrook.qqq.backend.core.model.metadata.QInstance;
import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldMetaData;
@ -41,6 +43,7 @@ import com.kingsrook.qqq.backend.core.model.metadata.tables.ExposedJoin;
import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData;
import com.kingsrook.qqq.backend.core.utils.CollectionUtils;
import com.kingsrook.qqq.backend.core.utils.collections.MutableList;
import org.apache.logging.log4j.Level;
import static com.kingsrook.qqq.backend.core.logging.LogUtils.logPair;
@ -60,6 +63,7 @@ public class JoinsContext
// note - will have entries for all tables, not just aliases. //
////////////////////////////////////////////////////////////////
private final Map<String, String> aliasToTableNameMap = new HashMap<>();
private Level logLevel = Level.OFF;
@ -69,12 +73,14 @@ public class JoinsContext
*******************************************************************************/
public JoinsContext(QInstance instance, String tableName, List<QueryJoin> queryJoins, QQueryFilter filter) throws QException
{
log("--- START ----------------------------------------------------------------------", logPair("mainTable", tableName));
this.instance = instance;
this.mainTableName = tableName;
this.queryJoins = new MutableList<>(queryJoins);
for(QueryJoin queryJoin : this.queryJoins)
{
log("Processing input query join", logPair("joinTable", queryJoin.getJoinTable()), logPair("alias", queryJoin.getAlias()), logPair("baseTableOrAlias", queryJoin.getBaseTableOrAlias()), logPair("joinMetaDataName", () -> queryJoin.getJoinMetaData().getName()));
processQueryJoin(queryJoin);
}
@ -83,48 +89,7 @@ public class JoinsContext
///////////////////////////////////////////////////////////////
for(RecordSecurityLock recordSecurityLock : CollectionUtils.nonNullList(instance.getTable(tableName).getRecordSecurityLocks()))
{
///////////////////////////////////////////////////////////////////////////////////////////////////
// ok - so - the join name chain is going to be like this: //
// for a table: orderLineItemExtrinsic (that's 2 away from order, where the security field is): //
// - securityFieldName = order.clientId //
// - joinNameChain = orderJoinOrderLineItem, orderLineItemJoinOrderLineItemExtrinsic //
// so - to navigate from the table to the security field, we need to reverse the joinNameChain, //
// and step (via tmpTable variable) back to the securityField //
///////////////////////////////////////////////////////////////////////////////////////////////////
ArrayList<String> joinNameChain = new ArrayList<>(CollectionUtils.nonNullList(recordSecurityLock.getJoinNameChain()));
Collections.reverse(joinNameChain);
QTableMetaData tmpTable = instance.getTable(mainTableName);
for(String joinName : joinNameChain)
{
if(this.queryJoins.stream().anyMatch(queryJoin ->
{
QJoinMetaData joinMetaData = Objects.requireNonNullElseGet(queryJoin.getJoinMetaData(), () -> findJoinMetaData(instance, tableName, queryJoin.getJoinTable()));
return (joinMetaData != null && Objects.equals(joinMetaData.getName(), joinName));
}))
{
continue;
}
QJoinMetaData join = instance.getJoin(joinName);
if(join.getLeftTable().equals(tmpTable.getName()))
{
QueryJoin queryJoin = new ImplicitQueryJoinForSecurityLock().withJoinMetaData(join).withType(QueryJoin.Type.INNER);
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.addQueryJoin(queryJoin); //
tmpTable = instance.getTable(join.getLeftTable());
}
else
{
throw (new QException("Error adding security lock joins to query - table name [" + tmpTable.getName() + "] not found in join [" + joinName + "]"));
}
}
ensureRecordSecurityLockIsRepresented(instance, tableName, recordSecurityLock);
}
ensureFilterIsRepresented(filter);
@ -141,6 +106,86 @@ public class JoinsContext
}
}
*/
log("Constructed JoinsContext", logPair("mainTableName", this.mainTableName), logPair("queryJoins", this.queryJoins.stream().map(qj -> qj.getJoinTable()).collect(Collectors.joining(","))));
log("--- END ------------------------------------------------------------------------");
}
/*******************************************************************************
**
*******************************************************************************/
private void ensureRecordSecurityLockIsRepresented(QInstance instance, String tableName, RecordSecurityLock recordSecurityLock) throws QException
{
///////////////////////////////////////////////////////////////////////////////////////////////////
// ok - so - the join name chain is going to be like this: //
// for a table: orderLineItemExtrinsic (that's 2 away from order, where the security field is): //
// - securityFieldName = order.clientId //
// - joinNameChain = orderJoinOrderLineItem, orderLineItemJoinOrderLineItemExtrinsic //
// so - to navigate from the table to the security field, we need to reverse the joinNameChain, //
// and step (via tmpTable variable) back to the securityField //
///////////////////////////////////////////////////////////////////////////////////////////////////
ArrayList<String> joinNameChain = new ArrayList<>(CollectionUtils.nonNullList(recordSecurityLock.getJoinNameChain()));
Collections.reverse(joinNameChain);
log("Evaluating recordSecurityLock", logPair("recordSecurityLock", recordSecurityLock.getFieldName()), logPair("joinNameChain", joinNameChain));
QTableMetaData tmpTable = instance.getTable(mainTableName);
for(String joinName : joinNameChain)
{
///////////////////////////////////////////////////////////////////////////////////////////////////////
// check the joins currently in the query - if any are for this table, then we don't need to add one //
///////////////////////////////////////////////////////////////////////////////////////////////////////
List<QueryJoin> matchingJoins = this.queryJoins.stream().filter(queryJoin ->
{
QJoinMetaData joinMetaData = null;
if(queryJoin.getJoinMetaData() != null)
{
joinMetaData = queryJoin.getJoinMetaData();
}
else
{
joinMetaData = findJoinMetaData(instance, tableName, queryJoin.getJoinTable());
}
return (joinMetaData != null && Objects.equals(joinMetaData.getName(), joinName));
}).toList();
if(CollectionUtils.nullSafeHasContents(matchingJoins))
{
/////////////////////////////////////////////////////////////////////////////////////////////////////////////////
// note - if a user added a join as an outer type, we need to change it to be inner, for the security purpose. //
// todo - is this always right? what about nulls-allowed? //
/////////////////////////////////////////////////////////////////////////////////////////////////////////////////
log("- skipping join already in the query", logPair("joinName", joinName));
if(matchingJoins.get(0).getType().equals(QueryJoin.Type.LEFT) || matchingJoins.get(0).getType().equals(QueryJoin.Type.RIGHT))
{
log("- - although... it was here as an outer - so switching it to INNER", logPair("joinName", joinName));
matchingJoins.get(0).setType(QueryJoin.Type.INNER);
}
continue;
}
QJoinMetaData join = instance.getJoin(joinName);
if(join.getLeftTable().equals(tmpTable.getName()))
{
QueryJoin queryJoin = new ImplicitQueryJoinForSecurityLock().withJoinMetaData(join).withType(QueryJoin.Type.INNER);
this.addQueryJoin(queryJoin, "forRecordSecurityLock (non-flipped)");
tmpTable = instance.getTable(join.getRightTable());
}
else if(join.getRightTable().equals(tmpTable.getName()))
{
QueryJoin queryJoin = new ImplicitQueryJoinForSecurityLock().withJoinMetaData(join.flip()).withType(QueryJoin.Type.INNER);
this.addQueryJoin(queryJoin, "forRecordSecurityLock (flipped)");
tmpTable = instance.getTable(join.getLeftTable());
}
else
{
throw (new QException("Error adding security lock joins to query - table name [" + tmpTable.getName() + "] not found in join [" + joinName + "]"));
}
}
}
@ -151,8 +196,15 @@ public class JoinsContext
** 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
private void addQueryJoin(QueryJoin queryJoin, String reason) throws QException
{
log("Adding query join to context",
logPair("reason", reason),
logPair("joinTable", queryJoin.getJoinTable()),
logPair("joinMetaData.name", () -> queryJoin.getJoinMetaData().getName()),
logPair("joinMetaData.leftTable", () -> queryJoin.getJoinMetaData().getLeftTable()),
logPair("joinMetaData.rightTable", () -> queryJoin.getJoinMetaData().getRightTable())
);
this.queryJoins.add(queryJoin);
processQueryJoin(queryJoin);
}
@ -177,10 +229,46 @@ public class JoinsContext
addedJoin = false;
for(QueryJoin queryJoin : queryJoins)
{
/////////////////////////////////////////////////////////////////////
// if the join has joinMetaData, then we don't need to process it. //
/////////////////////////////////////////////////////////////////////
if(queryJoin.getJoinMetaData() == null)
///////////////////////////////////////////////////////////////////////////////////////////////
// if the join has joinMetaData, then we don't need to process it... unless it needs flipped //
///////////////////////////////////////////////////////////////////////////////////////////////
QJoinMetaData joinMetaData = queryJoin.getJoinMetaData();
if(joinMetaData != null)
{
boolean isJoinLeftTableInQuery = false;
String joinMetaDataLeftTable = joinMetaData.getLeftTable();
if(joinMetaDataLeftTable.equals(mainTableName))
{
isJoinLeftTableInQuery = true;
}
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
// check the other joins in this query - if any of them have this join's left-table as their baseTable, then set the flag to true //
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
for(QueryJoin otherJoin : queryJoins)
{
if(otherJoin == queryJoin)
{
continue;
}
if(Objects.equals(otherJoin.getBaseTableOrAlias(), joinMetaDataLeftTable))
{
isJoinLeftTableInQuery = true;
break;
}
}
/////////////////////////////////////////////////////////////////////////////////
// if the join's left-table isn't in the query, then we need to flip the join. //
/////////////////////////////////////////////////////////////////////////////////
if(!isJoinLeftTableInQuery)
{
log("Flipping queryJoin because its leftTable wasn't found in the query", logPair("joinMetaDataName", joinMetaData.getName()), logPair("leftTable", joinMetaDataLeftTable));
queryJoin.setJoinMetaData(joinMetaData.flip());
}
}
else
{
//////////////////////////////////////////////////////////////////////
// try to find a direct join between the main table and this table. //
@ -190,6 +278,7 @@ public class JoinsContext
QJoinMetaData found = findJoinMetaData(instance, baseTableName, queryJoin.getJoinTable());
if(found != null)
{
log("Found joinMetaData - setting it in queryJoin", logPair("joinMetaDataName", found.getName()), logPair("baseTableName", baseTableName), logPair("joinTable", queryJoin.getJoinTable()));
queryJoin.setJoinMetaData(found);
}
else
@ -197,15 +286,13 @@ public class JoinsContext
/////////////////////////////////////////////////////////////////////////////////////////////////////////////////
// else, the join must be indirect - so look for an exposedJoin that will have a joinPath that will connect us //
/////////////////////////////////////////////////////////////////////////////////////////////////////////////////
LOG.debug("Looking for an exposed join...", logPair("mainTable", mainTableName), logPair("joinTable", queryJoin.getJoinTable()));
QTableMetaData mainTable = instance.getTable(mainTableName);
boolean addedAnyQueryJoins = false;
for(ExposedJoin exposedJoin : CollectionUtils.nonNullList(mainTable.getExposedJoins()))
{
if(queryJoin.getJoinTable().equals(exposedJoin.getJoinTable()))
{
LOG.debug("Found an exposed join", logPair("mainTable", mainTableName), logPair("joinTable", queryJoin.getJoinTable()), logPair("joinPath", exposedJoin.getJoinPath()));
log("Found an exposed join", logPair("mainTable", mainTableName), logPair("joinTable", queryJoin.getJoinTable()), logPair("joinPath", exposedJoin.getJoinPath()));
/////////////////////////////////////////////////////////////////////////////////////
// loop backward through the join path (from the joinTable back to the main table) //
@ -250,7 +337,7 @@ public class JoinsContext
QueryJoin queryJoinToAdd = makeQueryJoinFromJoinAndTableNames(nextTable, tmpTable, joinToAdd);
queryJoinToAdd.setType(queryJoin.getType());
addedAnyQueryJoins = true;
this.addQueryJoin(queryJoinToAdd);
this.addQueryJoin(queryJoinToAdd, "forExposedJoin");
}
}
@ -377,9 +464,9 @@ public class JoinsContext
**
** e.g., Given:
** FROM `order` INNER JOIN line_item li
** hasAliasOrTable("order") => true
** hasAliasOrTable("li") => false
** hasAliasOrTable("line_item") => true
** hasTable("order") => true
** hasTable("li") => false
** hasTable("line_item") => true
*******************************************************************************/
public boolean hasTable(String table)
{
@ -415,15 +502,17 @@ public class JoinsContext
for(String filterTable : filterTables)
{
log("Evaluating filterTable", logPair("filterTable", filterTable));
if(!aliasToTableNameMap.containsKey(filterTable) && !Objects.equals(mainTableName, filterTable))
{
log("- table not in query - adding it", logPair("filterTable", filterTable));
boolean found = false;
for(QJoinMetaData join : CollectionUtils.nonNullMap(QContext.getQInstance().getJoins()).values())
{
QueryJoin queryJoin = makeQueryJoinFromJoinAndTableNames(mainTableName, filterTable, join);
if(queryJoin != null)
{
this.addQueryJoin(queryJoin);
this.addQueryJoin(queryJoin, "forFilter (join found in instance)");
found = true;
break;
}
@ -432,7 +521,7 @@ public class JoinsContext
if(!found)
{
QueryJoin queryJoin = new QueryJoin().withJoinTable(filterTable).withType(QueryJoin.Type.INNER);
this.addQueryJoin(queryJoin);
this.addQueryJoin(queryJoin, "forFilter (join not found in instance)");
}
}
}
@ -569,4 +658,14 @@ public class JoinsContext
{
}
/*******************************************************************************
**
*******************************************************************************/
private void log(String message, LogPair... logPairs)
{
LOG.log(logLevel, message, null, logPairs);
}
}

View File

@ -66,6 +66,7 @@ public class TestUtils
public static final String TABLE_NAME_PERSONAL_ID_CARD = "personalIdCard";
public static final String TABLE_NAME_STORE = "store";
public static final String TABLE_NAME_ORDER = "order";
public static final String TABLE_NAME_ORDER_INSTRUCTIONS = "orderInstructions";
public static final String TABLE_NAME_ITEM = "item";
public static final String TABLE_NAME_ORDER_LINE = "orderLine";
public static final String TABLE_NAME_LINE_ITEM_EXTRINSIC = "orderLineExtrinsic";
@ -245,6 +246,16 @@ public class TestUtils
.withField(new QFieldMetaData("storeId", QFieldType.INTEGER).withBackendName("store_id").withPossibleValueSourceName(TABLE_NAME_STORE))
.withField(new QFieldMetaData("billToPersonId", QFieldType.INTEGER).withBackendName("bill_to_person_id").withPossibleValueSourceName(TABLE_NAME_PERSON))
.withField(new QFieldMetaData("shipToPersonId", QFieldType.INTEGER).withBackendName("ship_to_person_id").withPossibleValueSourceName(TABLE_NAME_PERSON))
.withField(new QFieldMetaData("currentOrderInstructionsId", QFieldType.INTEGER).withBackendName("current_order_instructions_id").withPossibleValueSourceName(TABLE_NAME_PERSON))
);
qInstance.addTable(defineBaseTable(TABLE_NAME_ORDER_INSTRUCTIONS, "order_instructions")
.withRecordSecurityLock(new RecordSecurityLock()
.withSecurityKeyType(TABLE_NAME_STORE)
.withFieldName("order.storeId")
.withJoinNameChain(List.of("orderInstructionsJoinOrder")))
.withField(new QFieldMetaData("orderId", QFieldType.INTEGER).withBackendName("order_id"))
.withField(new QFieldMetaData("instructions", QFieldType.STRING))
);
qInstance.addTable(defineBaseTable(TABLE_NAME_ITEM, "item")
@ -357,6 +368,22 @@ public class TestUtils
.withJoinOn(new JoinOn("id", "orderLineId"))
);
qInstance.addJoin(new QJoinMetaData()
.withName("orderJoinCurrentOrderInstructions")
.withLeftTable(TABLE_NAME_ORDER)
.withRightTable(TABLE_NAME_ORDER_INSTRUCTIONS)
.withType(JoinType.ONE_TO_ONE)
.withJoinOn(new JoinOn("currentOrderInstructionsId", "id"))
);
qInstance.addJoin(new QJoinMetaData()
.withName("orderInstructionsJoinOrder")
.withLeftTable(TABLE_NAME_ORDER_INSTRUCTIONS)
.withRightTable(TABLE_NAME_ORDER)
.withType(JoinType.MANY_TO_ONE)
.withJoinOn(new JoinOn("orderId", "id"))
);
qInstance.addPossibleValueSource(new QPossibleValueSource()
.withName("store")
.withType(QPossibleValueSourceType.TABLE)

View File

@ -32,10 +32,12 @@ import java.util.Objects;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Predicate;
import com.kingsrook.qqq.backend.core.actions.QBackendTransaction;
import com.kingsrook.qqq.backend.core.actions.tables.CountAction;
import com.kingsrook.qqq.backend.core.actions.tables.InsertAction;
import com.kingsrook.qqq.backend.core.actions.tables.QueryAction;
import com.kingsrook.qqq.backend.core.context.QContext;
import com.kingsrook.qqq.backend.core.exceptions.QException;
import com.kingsrook.qqq.backend.core.model.actions.tables.count.CountInput;
import com.kingsrook.qqq.backend.core.model.actions.tables.insert.InsertInput;
import com.kingsrook.qqq.backend.core.model.actions.tables.query.QCriteriaOperator;
import com.kingsrook.qqq.backend.core.model.actions.tables.query.QFilterCriteria;
@ -1695,4 +1697,51 @@ public class RDBMSQueryActionTest extends RDBMSActionTest
}
/*******************************************************************************
**
*******************************************************************************/
@Test
void testMultipleReversedDirectionJoinsBetweenSameTables() throws QException
{
QContext.setQSession(new QSession().withSecurityKeyValue(TestUtils.SECURITY_KEY_STORE_ALL_ACCESS, true));
{
/////////////////////////////////////////////////////////
// assert a failure if the join to use isn't specified //
/////////////////////////////////////////////////////////
QueryInput queryInput = new QueryInput();
queryInput.setTableName(TestUtils.TABLE_NAME_ORDER);
queryInput.withQueryJoin(new QueryJoin(TestUtils.TABLE_NAME_ORDER_INSTRUCTIONS));
assertThatThrownBy(() -> new QueryAction().execute(queryInput)).rootCause().hasMessageContaining("More than 1 join was found");
}
Integer noOfOrders = new CountAction().execute(new CountInput(TestUtils.TABLE_NAME_ORDER)).getCount();
Integer noOfOrderInstructions = new CountAction().execute(new CountInput(TestUtils.TABLE_NAME_ORDER_INSTRUCTIONS)).getCount();
{
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
// make sure we can join on order.current_order_instruction_id = order_instruction.id -- and that we get back 1 row per order //
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
QueryInput queryInput = new QueryInput();
queryInput.setTableName(TestUtils.TABLE_NAME_ORDER);
queryInput.withQueryJoin(new QueryJoin(TestUtils.TABLE_NAME_ORDER_INSTRUCTIONS).withJoinMetaData(QContext.getQInstance().getJoin("orderJoinCurrentOrderInstructions")));
QueryOutput queryOutput = new QueryAction().execute(queryInput);
assertEquals(noOfOrders, queryOutput.getRecords().size());
}
{
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
// make sure we can join on order.id = order_instruction.order_id -- and that we get back 1 row per order instruction //
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
QueryInput queryInput = new QueryInput();
queryInput.setTableName(TestUtils.TABLE_NAME_ORDER);
queryInput.withQueryJoin(new QueryJoin(TestUtils.TABLE_NAME_ORDER_INSTRUCTIONS).withJoinMetaData(QContext.getQInstance().getJoin("orderInstructionsJoinOrder")));
QueryOutput queryOutput = new QueryAction().execute(queryInput);
assertEquals(noOfOrderInstructions, queryOutput.getRecords().size());
}
}
}

View File

@ -84,6 +84,7 @@ DROP TABLE IF EXISTS line_item_extrinsic;
DROP TABLE IF EXISTS order_line;
DROP TABLE IF EXISTS item;
DROP TABLE IF EXISTS `order`;
DROP TABLE IF EXISTS order_instructions;
DROP TABLE IF EXISTS warehouse_store_int;
DROP TABLE IF EXISTS store;
DROP TABLE IF EXISTS warehouse;
@ -123,7 +124,8 @@ CREATE TABLE `order`
id INT AUTO_INCREMENT PRIMARY KEY,
store_id INT REFERENCES store,
bill_to_person_id INT,
ship_to_person_id INT
ship_to_person_id INT,
current_order_instructions_id INT -- f-key to order_instructions, which also has an f-key back here!
);
-- variable orders
@ -136,6 +138,27 @@ INSERT INTO `order` (id, store_id, bill_to_person_id, ship_to_person_id) VALUES
INSERT INTO `order` (id, store_id, bill_to_person_id, ship_to_person_id) VALUES (7, 3, null, 5);
INSERT INTO `order` (id, store_id, bill_to_person_id, ship_to_person_id) VALUES (8, 3, null, 5);
CREATE TABLE order_instructions
(
id INT AUTO_INCREMENT PRIMARY KEY,
order_id INT,
instructions VARCHAR(250)
);
-- give orders 1 & 2 multiple versions of the instruction record
INSERT INTO order_instructions (id, order_id, instructions) VALUES (1, 1, 'order 1 v1');
INSERT INTO order_instructions (id, order_id, instructions) VALUES (2, 1, 'order 1 v2');
UPDATE `order` SET current_order_instructions_id = 2 WHERE id=1;
INSERT INTO order_instructions (id, order_id, instructions) VALUES (3, 2, 'order 2 v1');
INSERT INTO order_instructions (id, order_id, instructions) VALUES (4, 2, 'order 2 v2');
INSERT INTO order_instructions (id, order_id, instructions) VALUES (5, 2, 'order 2 v3');
UPDATE `order` SET current_order_instructions_id = 5 WHERE id=2;
-- give all other orders just 1 instruction
INSERT INTO order_instructions (order_id, instructions) SELECT id, concat('order ', id, ' v1') FROM `order` WHERE current_order_instructions_id IS NULL;
UPDATE `order` SET current_order_instructions_id = (SELECT MIN(id) FROM order_instructions WHERE order_id = `order`.id) WHERE current_order_instructions_id is null;
CREATE TABLE order_line
(
id INT AUTO_INCREMENT PRIMARY KEY,