Repository: dynamoosejs/dynamoose (js)

Issue:
`.query()` is not working
`Cat.query("breed").contains("Terrier").exec()` will error: 
> Index can't be found for query.

@fishcharlie wrote:
> Dynamoose currently requires an index, but DynamoDB does not require an index when querying

I can confirm it works when I add an index to the schema, even tho I dont query for the index. But if I add an additional condition the same error occurs.

Pull request:
Query improvements
Closes #796 


Patch:
diff --git a/lib/Condition.js b/lib/Condition.js
index f61d2e06..5885cf46 100644
--- a/lib/Condition.js
+++ b/lib/Condition.js
@@ -107,7 +107,6 @@ Condition.prototype.requestObject = function(settings = {"conditionString": "Con
 			const [key, value] = entry;
 			// TODO: we should fix this so that we can do `isDynamoItem(value)`
 			if (!Document.isDynamoObject({"key": value})) {
-				console.log(value, Object.keys(value));
 				obj.ExpressionAttributeValues[key] = Document.toDynamo(value, {"type": "value"});
 			}
 			return obj;
@@ -178,10 +177,20 @@ Condition.prototype.requestObject = function(settings = {"conditionString": "Con
 				return object;
 			}
 
-			object[settings.conditionString] = `${object[settings.conditionString]}${object[settings.conditionString] !== "" ? ` ${arr[i - 1] === OR ? "OR" : "AND"} ` : ""}${expression}`;
+			const conditionStringNewItems = [expression];
+			if (object[settings.conditionString].length > 0) {
+				conditionStringNewItems.unshift(` ${arr[i - 1] === OR ? "OR" : "AND"} `);
+			}
+			conditionStringNewItems.forEach((item) => {
+				if (typeof object[settings.conditionString] === "string") {
+					object[settings.conditionString] = `${object[settings.conditionString]}${item}`;
+				} else {
+					object[settings.conditionString].push(item.trim());
+				}
+			});
 
 			return object;
-		}, {[settings.conditionString]: "", "ExpressionAttributeNames": {}, "ExpressionAttributeValues": {}});
+		}, {[settings.conditionString]: settings.conditionStringType === "array" ? [] : "", "ExpressionAttributeNames": {}, "ExpressionAttributeValues": {}});
 	}
 	return main(this.settings.conditions);
 };
diff --git a/lib/DocumentRetriever.js b/lib/DocumentRetriever.js
index 5a0fb547..5e4cd339 100644
--- a/lib/DocumentRetriever.js
+++ b/lib/DocumentRetriever.js
@@ -45,7 +45,7 @@ function main(documentRetrieverTypeString) {
 		});
 		C.prototype[`get${utils.capitalize_first_letter(documentRetrieverType.type)}Request`] = async function() {
 			const object = {
-				...this.settings.condition.requestObject({"defaultPrefix": "", "conditionString": "FilterExpression"}),
+				...this.settings.condition.requestObject({"defaultPrefix": "", "conditionString": "FilterExpression", "conditionStringType": "array"}),
 				"TableName": model.name
 			};
 
@@ -67,16 +67,20 @@ function main(documentRetrieverTypeString) {
 					return res;
 				}, {});
 				const index = utils.array_flatten(Object.values(indexes)).find((index) => {
-					const {hash, range} = index.KeySchema.reduce((res, item) => {
+					const {hash/*, range*/} = index.KeySchema.reduce((res, item) => {
 						res[item.KeyType.toLowerCase()] = item.AttributeName;
 						return res;
 					}, {});
-					return (comparisonChart[hash] || {}).type === "EQ" && (!range || !comparisonChart[range] || comparisonChart[range].type === "EQ");
+					// TODO: we need to write logic here to prioritize indexes with a range key that is being queried.
+					return (comparisonChart[hash] || {}).type === "EQ"/* && (!range || comparisonChart[range])*/;
 				});
 				if (!index) {
-					throw new Error.InvalidParameter("Index can't be found for query.");
+					if ((comparisonChart[model.schema.getHashKey()] || {}).type !== "EQ") {
+						throw new Error.InvalidParameter("Index can't be found for query.");
+					}
+				} else {
+					object.IndexName = index.IndexName;
 				}
-				object.IndexName = index.IndexName;
 			}
 			function moveParameterNames(val, prefix) {
 				const entry = Object.entries(object.ExpressionAttributeNames).find((entry) => entry[1] === val);
@@ -84,15 +88,27 @@ function main(documentRetrieverTypeString) {
 					return;
 				}
 				const [key, value] = entry;
+				const filterExpressionIndex = object.FilterExpression.findIndex((item) => item.includes(key));
+				const filterExpression = object.FilterExpression[filterExpressionIndex];
+				if (filterExpression.includes("attribute_exists") || filterExpression.includes("contains")) {
+					return;
+				}
 				object.ExpressionAttributeNames[`#${prefix}a`] = value;
 				delete object.ExpressionAttributeNames[key];
 
 				const valueKey = key.replace("#a", ":v");
-				object.ExpressionAttributeValues[`:${prefix}v`] = object.ExpressionAttributeValues[valueKey];
-				delete object.ExpressionAttributeValues[valueKey];
-				object.KeyConditionExpression = `${object.KeyConditionExpression || ""}${object.KeyConditionExpression ? " AND " : ""}#${prefix}a = :${prefix}v`;
+				Object.keys(object.ExpressionAttributeValues).filter((key) => key.startsWith(valueKey)).forEach((key) => {
+					object.ExpressionAttributeValues[key.replace(new RegExp(":v\\d"), `:${prefix}v`)] = object.ExpressionAttributeValues[key];
+					delete object.ExpressionAttributeValues[key];
+				});
+				const newExpression = filterExpression.replace(key, `#${prefix}a`).replace(new RegExp(valueKey, "g"), `:${prefix}v`);
 
-				object.FilterExpression = object.FilterExpression.replace(`${key} = ${valueKey} AND `, "").replace(` AND ${key} = ${valueKey}`, "").replace(`${key} = ${valueKey}`, "");
+				object.KeyConditionExpression = `${object.KeyConditionExpression || ""}${object.KeyConditionExpression ? " AND " : ""}${newExpression}`;
+				utils.object.delete(object.FilterExpression, filterExpressionIndex);
+				const previousElementIndex = filterExpressionIndex === 0 ? 0 : filterExpressionIndex - 1;
+				if (object.FilterExpression[previousElementIndex] === "AND") {
+					utils.object.delete(object.FilterExpression, previousElementIndex);
+				}
 			}
 			if (documentRetrieverType.type === "query") {
 				const index = utils.array_flatten(Object.values(indexes)).find((index) => index.IndexName === object.IndexName);
@@ -106,9 +122,10 @@ function main(documentRetrieverTypeString) {
 					if (range) {
 						moveParameterNames(range, "qr");
 					}
-
-					if (!object.FilterExpression) {
-						delete object.FilterExpression;
+				} else {
+					moveParameterNames(model.schema.getHashKey(), "qh");
+					if (model.schema.getRangeKey()) {
+						moveParameterNames(model.schema.getRangeKey(), "qr");
 					}
 				}
 			}
@@ -122,6 +139,13 @@ function main(documentRetrieverTypeString) {
 				object.TotalSegments = this.settings.parallel;
 			}
 
+			if (object.FilterExpression) {
+				object.FilterExpression = object.FilterExpression.join(" ");
+			}
+			if (object.FilterExpression === "") {
+				delete object.FilterExpression;
+			}
+
 			return object;
 		};
 		C.prototype.exec = function(callback) {
diff --git a/lib/utils/object/delete.js b/lib/utils/object/delete.js
index 81e4c82d..3669a3fc 100644
--- a/lib/utils/object/delete.js
+++ b/lib/utils/object/delete.js
@@ -1,5 +1,5 @@
 module.exports = (object, key) => {
-	const keys = key.split(".");
+	const keys = (typeof key === "number" ? `${key}` : key).split(".");
 
 	if (keys.length === 1) {
 		if (Array.isArray(object)) {


Test patch:
diff --git a/test/Query.js b/test/Query.js
index 89de8bb9..5ca67543 100644
--- a/test/Query.js
+++ b/test/Query.js
@@ -143,6 +143,155 @@ describe("Query", () => {
 					});
 				});
 
+				it("Should send correct request on query.exec if querying main key", async () => {
+					queryPromiseResolver = () => ({"Items": []});
+					Model = new dynamoose.Model("Cat", {"id": String, "name": String, "age": Number});
+					await callType.func(Model.query("id").eq("HelloWorld").exec).bind(Model.query("id").eq("HelloWorld"))();
+					expect(queryParams).to.eql({
+						"TableName": "Cat",
+						"ExpressionAttributeNames": {
+							"#qha": "id"
+						},
+						"ExpressionAttributeValues": {
+							":qhv": {"S": "HelloWorld"}
+						},
+						"KeyConditionExpression": "#qha = :qhv"
+					});
+				});
+
+				it("Should send correct request on query.exec if querying main key with range key", async () => {
+					queryPromiseResolver = () => ({"Items": []});
+					Model = new dynamoose.Model("Cat", {"id": String, "name": {"type": String, "rangeKey": true}, "age": Number});
+					await callType.func(Model.query("id").eq("HelloWorld").where("name").eq("Charlie").exec).bind(Model.query("id").eq("HelloWorld").where("name").eq("Charlie"))();
+					expect(queryParams).to.eql({
+						"TableName": "Cat",
+						"ExpressionAttributeNames": {
+							"#qha": "id",
+							"#qra": "name"
+						},
+						"ExpressionAttributeValues": {
+							":qhv": {"S": "HelloWorld"},
+							":qrv": {"S": "Charlie"}
+						},
+						"KeyConditionExpression": "#qha = :qhv AND #qra = :qrv"
+					});
+				});
+
+				it("Should send correct request on query.exec if querying main key with range key as less than comparison", async () => {
+					queryPromiseResolver = () => ({"Items": []});
+					Model = new dynamoose.Model("Cat", {"id": String, "name": String, "age": {"type": Number, "rangeKey": true}});
+					await callType.func(Model.query("id").eq("HelloWorld").where("age").lt(10).exec).bind(Model.query("id").eq("HelloWorld").where("age").lt(10))();
+					expect(queryParams).to.eql({
+						"TableName": "Cat",
+						"ExpressionAttributeNames": {
+							"#qha": "id",
+							"#qra": "age"
+						},
+						"ExpressionAttributeValues": {
+							":qhv": {"S": "HelloWorld"},
+							":qrv": {"N": "10"}
+						},
+						"KeyConditionExpression": "#qha = :qhv AND #qra < :qrv"
+					});
+				});
+
+				it("Should send correct request on query.exec using range key as less than comparison", async () => {
+					queryPromiseResolver = () => ({"Items": []});
+					Model = new dynamoose.Model("Cat", {"id": String, "name": {"type": String, "index": {"global": true, "rangeKey": "age"}}, "age": Number});
+					await callType.func(Model.query("name").eq("Charlie").where("age").lt(10).exec).bind(Model.query("name").eq("Charlie").where("age").lt(10))();
+					expect(queryParams).to.eql({
+						"TableName": "Cat",
+						"IndexName": "nameGlobalIndex",
+						"ExpressionAttributeNames": {
+							"#qha": "name",
+							"#qra": "age"
+						},
+						"ExpressionAttributeValues": {
+							":qhv": {"S": "Charlie"},
+							":qrv": {"N": "10"}
+						},
+						"KeyConditionExpression": "#qha = :qhv AND #qra < :qrv"
+					});
+				});
+
+				it("Should send correct request on query.exec using range key as between comparison", async () => {
+					queryPromiseResolver = () => ({"Items": []});
+					Model = new dynamoose.Model("Cat", {"id": String, "name": {"type": String, "index": {"global": true, "rangeKey": "age"}}, "age": Number});
+					await callType.func(Model.query("name").eq("Charlie").where("age").between(10, 20).exec).bind(Model.query("name").eq("Charlie").where("age").between(10, 20))();
+					expect(queryParams).to.eql({
+						"TableName": "Cat",
+						"IndexName": "nameGlobalIndex",
+						"ExpressionAttributeNames": {
+							"#qha": "name",
+							"#qra": "age"
+						},
+						"ExpressionAttributeValues": {
+							":qhv": {"S": "Charlie"},
+							":qrv_1": {"N": "10"},
+							":qrv_2": {"N": "20"}
+						},
+						"KeyConditionExpression": "#qha = :qhv AND #qra BETWEEN :qrv_1 AND :qrv_2"
+					});
+				});
+
+				it("Should send correct request on query.exec using range key as exists comparison", async () => {
+					queryPromiseResolver = () => ({"Items": []});
+					Model = new dynamoose.Model("Cat", {"id": String, "name": {"type": String, "index": {"global": true, "rangeKey": "age"}}, "age": Number});
+					await callType.func(Model.query("name").eq("Charlie").where("age").exists().exec).bind(Model.query("name").eq("Charlie").where("age").exists())();
+					expect(queryParams).to.eql({
+						"TableName": "Cat",
+						"IndexName": "nameGlobalIndex",
+						"ExpressionAttributeNames": {
+							"#qha": "name",
+							"#a1": "age"
+						},
+						"ExpressionAttributeValues": {
+							":qhv": {"S": "Charlie"}
+						},
+						"KeyConditionExpression": "#qha = :qhv",
+						"FilterExpression": "attribute_exists (#a1)"
+					});
+				});
+
+				it("Should send correct request on query.exec using range key as contains comparison", async () => {
+					queryPromiseResolver = () => ({"Items": []});
+					Model = new dynamoose.Model("Cat", {"id": String, "name": {"type": String, "index": {"global": true, "rangeKey": "age"}}, "age": Number});
+					await callType.func(Model.query("name").eq("Charlie").where("age").contains(10).exec).bind(Model.query("name").eq("Charlie").where("age").contains(10))();
+					expect(queryParams).to.eql({
+						"TableName": "Cat",
+						"IndexName": "nameGlobalIndex",
+						"ExpressionAttributeNames": {
+							"#qha": "name",
+							"#a1": "age"
+						},
+						"ExpressionAttributeValues": {
+							":qhv": {"S": "Charlie"},
+							":v1": {"N": "10"}
+						},
+						"KeyConditionExpression": "#qha = :qhv",
+						"FilterExpression": "contains (#a1, :v1)"
+					});
+				});
+
+				it("Should send correct request on query.exec using range key as beginsWith comparison", async () => {
+					queryPromiseResolver = () => ({"Items": []});
+					Model = new dynamoose.Model("Cat", {"id": String, "name": {"type": String, "index": {"global": true, "rangeKey": "age"}}, "age": Number});
+					await callType.func(Model.query("name").eq("Charlie").where("age").beginsWith(10).exec).bind(Model.query("name").eq("Charlie").where("age").beginsWith(10))();
+					expect(queryParams).to.eql({
+						"TableName": "Cat",
+						"IndexName": "nameGlobalIndex",
+						"ExpressionAttributeNames": {
+							"#qha": "name",
+							"#qra": "age"
+						},
+						"ExpressionAttributeValues": {
+							":qhv": {"S": "Charlie"},
+							":qrv": {"N": "10"}
+						},
+						"KeyConditionExpression": "#qha = :qhv AND begins_with (#qra, :qrv)"
+					});
+				});
+
 				it("Should send correct request on query.exec for one object passed in", async () => {
 					queryPromiseResolver = () => ({"Items": []});
 					await callType.func(Model.query({"name": "Charlie"}).exec).bind(Model.query({"name": "Charlie"}))();
diff --git a/test/utils/object/delete.js b/test/utils/object/delete.js
index 9ee5e927..ba23d990 100644
--- a/test/utils/object/delete.js
+++ b/test/utils/object/delete.js
@@ -27,6 +27,10 @@ describe("utils.object.delete", () => {
 			"input": [["world", "universe"], "0"],
 			"output": ["universe"]
 		},
+		{
+			"input": [["world", "universe"], 0],
+			"output": ["universe"]
+		},
 		{
 			"input": [{"hello": ["world", "universe"]}, "hello.0"],
 			"output": {"hello": ["universe"]}