From 4bbb17863391fb9618c438cbdb8a58528cf44de5 Mon Sep 17 00:00:00 2001 From: Marcin Antas Date: Thu, 19 May 2022 14:23:12 +0200 Subject: [PATCH 1/3] nearText move params are not supporting inside objects parameter --- docker-compose.yml | 2 +- graphql/getter.test.js | 155 ++++++++++++++++++++++++++++++++++++++-- graphql/journey.test.js | 19 +++++ graphql/nearText.js | 79 +++++++++++++++++--- schema/journey.test.js | 2 + 5 files changed, 239 insertions(+), 18 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 5215016..368c186 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,7 +1,7 @@ version: '3.4' services: weaviate: - image: semitechnologies/weaviate:1.13.0 + image: semitechnologies/weaviate:1.13.2 restart: on-failure:0 ports: - "8080:8080" diff --git a/graphql/getter.test.js b/graphql/getter.test.js index d69a4a2..88ee1eb 100644 --- a/graphql/getter.test.js +++ b/graphql/getter.test.js @@ -286,6 +286,76 @@ describe("nearText searchers", () => { expect(mockClient.query).toHaveBeenCalledWith(expectedQuery); }); + test("with moveTo with objects parameter", () => { + const mockClient = { + query: jest.fn(), + }; + + const expectedQuery = + `{Get{Person` + + `(nearText:{concepts:["foo","bar"],certainty:0.7,moveTo:{objects:[{id:"uuid"},{beacon:"beacon"}],force:0.7}})` + + `{name}}}`; + + new Getter(mockClient) + .withClassName("Person") + .withFields("name") + .withNearText({ + concepts: ["foo", "bar"], + certainty: 0.7, + moveTo: { force: 0.7, objects: [{ id: "uuid" }, {beacon: "beacon"}] }, + }) + .do(); + + expect(mockClient.query).toHaveBeenCalledWith(expectedQuery); + }); + + test("with moveAwayFrom with objects parameter", () => { + const mockClient = { + query: jest.fn(), + }; + + const expectedQuery = + `{Get{Person` + + `(nearText:{concepts:["foo","bar"],certainty:0.7,moveAwayFrom:{objects:[{id:"uuid"},{beacon:"beacon"}],force:0.7}})` + + `{name}}}`; + + new Getter(mockClient) + .withClassName("Person") + .withFields("name") + .withNearText({ + concepts: ["foo", "bar"], + certainty: 0.7, + moveAwayFrom: { force: 0.7, objects: [{ id: "uuid" }, {beacon: "beacon"}] }, + }) + .do(); + + expect(mockClient.query).toHaveBeenCalledWith(expectedQuery); + }); + + test("with moveTo and moveAway with objects parameter", () => { + const mockClient = { + query: jest.fn(), + }; + + const expectedQuery = + `{Get{Person` + + `(nearText:{concepts:["foo","bar"],certainty:0.7,moveTo:{objects:[{id:"uuid"}],force:0.7},moveAwayFrom:{objects:[{beacon:"beacon"}],force:0.5}})` + + `{name}}}`; + + new Getter(mockClient) + .withClassName("Person") + .withFields("name") + .withNearText({ + concepts: ["foo", "bar"], + certainty: 0.7, + moveTo: { force: 0.7, objects: [{ id: "uuid" }] }, + moveAwayFrom: { force: 0.5, objects: [{ beacon: "beacon" }] }, + }) + .do(); + + expect(mockClient.query).toHaveBeenCalledWith(expectedQuery); + }); + describe("queries with invalid nearText searchers", () => { const mockClient = { query: jest.fn(), @@ -308,31 +378,102 @@ describe("nearText searchers", () => { msg: "nearText filter: certainty must be a number", }, { - title: "moveTo without concepts", + title: "moveTo empty object", nearText: { concepts: ["foo"], moveTo: {} }, - msg: "nearText filter: moveTo.concepts must be an array", + msg: "nearText filter: moveTo.concepts or moveTo.objects must be present", }, { - title: "moveTo without concepts", + title: "moveTo without force with concepts", nearText: { concepts: ["foo"], moveTo: { concepts: ["foo"] } }, - msg: "nearText filter: moveTo must have fields 'concepts' and 'force'", + msg: "nearText filter: moveTo must have fields 'concepts' or 'objects' and 'force'", + }, + { + title: "moveTo without force with objects", + nearText: { concepts: ["foo"], moveTo: { objects: [{beacon: "beacon"}] } }, + msg: "nearText filter: moveTo must have fields 'concepts' or 'objects' and 'force'", }, { title: "moveAwayFrom without concepts", nearText: { concepts: ["foo"], moveAwayFrom: {} }, - msg: "nearText filter: moveAwayFrom.concepts must be an array", + msg: "nearText filter: moveAwayFrom.concepts or moveAwayFrom.objects must be present", }, { - title: "moveAwayFrom without concepts", + title: "moveAwayFrom without force with concepts", nearText: { concepts: ["foo"], moveAwayFrom: { concepts: ["foo"] } }, msg: - "nearText filter: moveAwayFrom must have fields 'concepts' and 'force'", + "nearText filter: moveAwayFrom must have fields 'concepts' or 'objects' and 'force'", + }, + { + title: "moveAwayFrom without force with objects", + nearText: { concepts: ["foo"], moveAwayFrom: { objects: [{id: "uuid"}] } }, + msg: + "nearText filter: moveAwayFrom must have fields 'concepts' or 'objects' and 'force'", }, { title: "autocorrect of wrong type", nearText: { concepts: ["foo"], autocorrect: "foo" }, msg: "nearText filter: autocorrect must be a boolean", }, + { + title: "moveTo with empty objects", + nearText: { concepts: ["foo"], moveTo: { force: 0.8, objects: {} } }, + msg: + "nearText filter: moveTo.concepts or moveTo.objects must be present", + }, + { + title: "moveTo with empty object in objects", + nearText: { concepts: ["foo"], moveTo: { force: 0.8, objects: [{}] } }, + msg: + "nearText filter: moveTo.objects[0].id or moveTo.objects[0].beacon must be present", + }, + { + title: "moveTo with objects[0].id not of string type", + nearText: { concepts: ["foo"], moveTo: { force: 0.8, objects: [{id: 0.8}] } }, + msg: + "nearText filter: moveTo.objects[0].id must be string", + }, + { + title: "moveTo with objects[0].beacon not of string type", + nearText: { concepts: ["foo"], moveTo: { force: 0.8, objects: [{beacon: 0.8}] } }, + msg: + "nearText filter: moveTo.objects[0].beacon must be string", + }, + { + title: "moveTo with objects[0].id not of string type and objects[1].beacon not of string type", + nearText: { concepts: ["foo"], moveTo: { force: 0.8, objects: [{id: 0.8},{beacon: 0.8}] } }, + msg: + "nearText filter: moveTo.objects[0].id must be string, moveTo.objects[1].beacon must be string", + }, + { + title: "moveAwayFrom with empty objects", + nearText: { concepts: ["foo"], moveAwayFrom: { force: 0.8, objects: {} } }, + msg: + "nearText filter: moveAwayFrom.concepts or moveAwayFrom.objects must be present", + }, + { + title: "moveAwayFrom with empty object in objects", + nearText: { concepts: ["foo"], moveAwayFrom: { force: 0.8, objects: [{}] } }, + msg: + "nearText filter: moveAwayFrom.objects[0].id or moveAwayFrom.objects[0].beacon must be present", + }, + { + title: "moveAwayFrom with objects[0].id not of string type", + nearText: { concepts: ["foo"], moveAwayFrom: { force: 0.8, objects: [{id: 0.8}] } }, + msg: + "nearText filter: moveAwayFrom.objects[0].id must be string", + }, + { + title: "moveAwayFrom with objects[0].beacon not of string type", + nearText: { concepts: ["foo"], moveAwayFrom: { force: 0.8, objects: [{beacon: 0.8}] } }, + msg: + "nearText filter: moveAwayFrom.objects[0].beacon must be string", + }, + { + title: "moveAwayFrom with objects[0].id not of string type and objects[1].beacon not of string type", + nearText: { concepts: ["foo"], moveAwayFrom: { force: 0.8, objects: [{id: 0.8},{beacon: 0.8}] } }, + msg: + "nearText filter: moveAwayFrom.objects[0].id must be string, moveAwayFrom.objects[1].beacon must be string", + }, ]; tests.forEach((t) => { diff --git a/graphql/journey.test.js b/graphql/journey.test.js index 6b9bf3f..ba9c480 100644 --- a/graphql/journey.test.js +++ b/graphql/journey.test.js @@ -102,6 +102,23 @@ describe("the graphql journey", () => { .catch((e) => fail("it should not have error'd" + e)); }); + test("graphql get with nearText with moveTo and moveAwayFrom", () => { + return client.graphql + .get() + .withClassName("Article") + .withNearText({ + concepts: ["Article"], certainty: 0.7, + moveTo: { objects:[{ id: "abefd256-8574-442b-9293-9205193737e2" }], force: 0.7 }, + moveAwayFrom: { objects:[{ id: "abefd256-8574-442b-9293-9205193737e1" }], force: 0.5 }, + }) + .withFields("_additional { id }") + .do() + .then((res) => { + expect(res.data.Get.Article.length).toBe(3); + }) + .catch((e) => fail("it should not have error'd" + e)); + }); + test("graphql get expected failure - multiple nearMedia filters", () => { return expect(() => { client.graphql @@ -509,6 +526,7 @@ const setup = async (client) => { }, }, { + id: "abefd256-8574-442b-9293-9205193737e1", class: "Article", properties: { wordCount: 40, @@ -517,6 +535,7 @@ const setup = async (client) => { }, }, { + id: "abefd256-8574-442b-9293-9205193737e2", class: "Article", properties: { wordCount: 600, diff --git a/graphql/nearText.js b/graphql/nearText.js index f58cb2b..c6e834a 100644 --- a/graphql/nearText.js +++ b/graphql/nearText.js @@ -14,7 +14,13 @@ export default class GraphQLNearText { } if (this.moveTo) { - let moveToArgs = [`concepts:${JSON.stringify(this.moveToConcepts)}`]; + let moveToArgs = [] + if (this.moveToConcepts) { + moveToArgs = [...moveToArgs, `concepts:${JSON.stringify(this.moveToConcepts)}`]; + } + if (this.moveToObjects) { + moveToArgs = [...moveToArgs, `objects:${this.moveToObjects}`]; + } if (this.moveToForce) { moveToArgs = [...moveToArgs, `force:${this.moveToForce}`]; } @@ -22,9 +28,13 @@ export default class GraphQLNearText { } if (this.moveAwayFrom) { - let moveAwayFromArgs = [ - `concepts:${JSON.stringify(this.moveAwayFromConcepts)}`, - ]; + let moveAwayFromArgs = []; + if (this.moveAwayFromConcepts) { + moveAwayFromArgs = [...moveAwayFromArgs, `concepts:${JSON.stringify(this.moveAwayFromConcepts)}`]; + } + if (this.moveAwayFromObjects) { + moveAwayFromArgs = [...moveAwayFromArgs, `objects:${this.moveAwayFromObjects}`]; + } if (this.moveAwayFromForce) { moveAwayFromArgs = [ ...moveAwayFromArgs, @@ -50,17 +60,17 @@ export default class GraphQLNearText { } if (this.moveTo) { - if (!this.moveToForce || !this.moveToConcepts) { + if (!this.moveToForce || (!this.moveToConcepts && !this.moveToObjects)) { throw new Error( - "nearText filter: moveTo must have fields 'concepts' and 'force'" + "nearText filter: moveTo must have fields 'concepts' or 'objects' and 'force'" ); } } if (this.moveAwayFrom) { - if (!this.moveAwayFromForce || !this.moveAwayFromConcepts) { + if (!this.moveAwayFromForce || (!this.moveAwayFromConcepts && !this.moveAwayFromObjects)) { throw new Error( - "nearText filter: moveAwayFrom must have fields 'concepts' and 'force'" + "nearText filter: moveAwayFrom must have fields 'concepts' or 'objects' and 'force'" ); } } @@ -111,10 +121,18 @@ export default class GraphQLNearText { throw new Error("nearText filter: moveTo must be object"); } - if (!Array.isArray(target.concepts)) { + if (!Array.isArray(target.concepts) && !Array.isArray(target.objects)) { + throw new Error("nearText filter: moveTo.concepts or moveTo.objects must be present"); + } + + if (target.concepts && !Array.isArray(target.concepts)) { throw new Error("nearText filter: moveTo.concepts must be an array"); } + if (target.objects && !Array.isArray(target.objects)) { + throw new Error("nearText filter: moveTo.objects must be an array"); + } + if (target.force && typeof target.force != "number") { throw new Error("nearText filter: moveTo.force must be a number"); } @@ -122,6 +140,9 @@ export default class GraphQLNearText { this.moveTo = true; this.moveToConcepts = target.concepts; this.moveToForce = target.force; + if (target.objects) { + this.moveToObjects = this.parseMoveObjects("moveTo", target.objects); + } } parseMoveAwayFrom(target) { @@ -129,12 +150,20 @@ export default class GraphQLNearText { throw new Error("nearText filter: moveAwayFrom must be object"); } - if (!Array.isArray(target.concepts)) { + if (!Array.isArray(target.concepts) && !Array.isArray(target.objects)) { + throw new Error("nearText filter: moveAwayFrom.concepts or moveAwayFrom.objects must be present"); + } + + if (target.concepts && !Array.isArray(target.concepts)) { throw new Error( "nearText filter: moveAwayFrom.concepts must be an array" ); } + if (target.objects && !Array.isArray(target.objects)) { + throw new Error("nearText filter: moveAwayFrom.objects must be an array"); + } + if (target.force && typeof target.force != "number") { throw new Error("nearText filter: moveAwayFrom.force must be a number"); } @@ -142,6 +171,9 @@ export default class GraphQLNearText { this.moveAwayFrom = true; this.moveAwayFromConcepts = target.concepts; this.moveAwayFromForce = target.force; + if (target.objects) { + this.moveAwayFromObjects = this.parseMoveObjects("moveAwayFrom", target.objects); + } } parseAutocorrect(autocorrect) { @@ -151,4 +183,31 @@ export default class GraphQLNearText { this.autocorrect = autocorrect; } + + parseMoveObjects(moveXXX, objects) { + let moveObjects = []; + let errors = []; + for (var i in objects) { + if (!objects[i].id && !objects[i].beacon) { + errors.push(`${moveXXX}.objects[${i}].id or ${moveXXX}.objects[${i}].beacon must be present`) + } else if (objects[i].id && typeof objects[i].id !== "string") { + errors.push(`${moveXXX}.objects[${i}].id must be string`) + } else if (objects[i].beacon && typeof objects[i].beacon !== "string") { + errors.push(`${moveXXX}.objects[${i}].beacon must be string`) + } else { + var objs = [] + if (objects[i].id) { + objs.push(`id:"${objects[i].id}"`); + } + if (objects[i].beacon) { + objs.push(`beacon:"${objects[i].beacon}"`); + } + moveObjects.push(`{${objs.join(",")}}`) + } + } + if (errors.length > 0) { + throw new Error(`nearText filter: ${errors.join(", ")}`); + } + return `[${moveObjects.join(",")}]` + } } diff --git a/schema/journey.test.js b/schema/journey.test.js index 9e66fcd..baea274 100644 --- a/schema/journey.test.js +++ b/schema/journey.test.js @@ -150,6 +150,7 @@ describe("schema", () => { vectorizer: "text2vec-contextionary", vectorIndexConfig: { cleanupIntervalSeconds: 300, + distance: "cosine", dynamicEfFactor: 8, dynamicEfMax: 500, dynamicEfMin: 100, @@ -381,6 +382,7 @@ function newClassObject(className) { vectorizer: 'text2vec-contextionary', vectorIndexConfig: { cleanupIntervalSeconds: 300, + distance: "cosine", dynamicEfFactor: 8, dynamicEfMax: 500, dynamicEfMin: 100, From 6b7706ccd2cc0658875ca97c61de18661cb341bf Mon Sep 17 00:00:00 2001 From: Andrzej Liszka Date: Mon, 23 May 2022 12:55:13 +0200 Subject: [PATCH 2/3] WVT-114: concepts and objects validation improvement --- graphql/getter.test.js | 4 ++-- graphql/nearText.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/graphql/getter.test.js b/graphql/getter.test.js index 88ee1eb..4083cfe 100644 --- a/graphql/getter.test.js +++ b/graphql/getter.test.js @@ -418,7 +418,7 @@ describe("nearText searchers", () => { title: "moveTo with empty objects", nearText: { concepts: ["foo"], moveTo: { force: 0.8, objects: {} } }, msg: - "nearText filter: moveTo.concepts or moveTo.objects must be present", + "nearText filter: moveTo.objects must be an array", }, { title: "moveTo with empty object in objects", @@ -448,7 +448,7 @@ describe("nearText searchers", () => { title: "moveAwayFrom with empty objects", nearText: { concepts: ["foo"], moveAwayFrom: { force: 0.8, objects: {} } }, msg: - "nearText filter: moveAwayFrom.concepts or moveAwayFrom.objects must be present", + "nearText filter: moveAwayFrom.objects must be an array", }, { title: "moveAwayFrom with empty object in objects", diff --git a/graphql/nearText.js b/graphql/nearText.js index c6e834a..577ce96 100644 --- a/graphql/nearText.js +++ b/graphql/nearText.js @@ -121,7 +121,7 @@ export default class GraphQLNearText { throw new Error("nearText filter: moveTo must be object"); } - if (!Array.isArray(target.concepts) && !Array.isArray(target.objects)) { + if (!target.concepts && !target.objects) { throw new Error("nearText filter: moveTo.concepts or moveTo.objects must be present"); } @@ -150,7 +150,7 @@ export default class GraphQLNearText { throw new Error("nearText filter: moveAwayFrom must be object"); } - if (!Array.isArray(target.concepts) && !Array.isArray(target.objects)) { + if (!target.concepts && !target.objects) { throw new Error("nearText filter: moveAwayFrom.concepts or moveAwayFrom.objects must be present"); } From 577b12b70f571107d301b27024c2a38751d9fe5f Mon Sep 17 00:00:00 2001 From: Marcin Antas Date: Mon, 23 May 2022 20:28:48 +0200 Subject: [PATCH 3/3] Changed the name of the move param in parseMoveObjects method --- graphql/nearText.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/graphql/nearText.js b/graphql/nearText.js index 577ce96..c6aa3c5 100644 --- a/graphql/nearText.js +++ b/graphql/nearText.js @@ -184,16 +184,16 @@ export default class GraphQLNearText { this.autocorrect = autocorrect; } - parseMoveObjects(moveXXX, objects) { + parseMoveObjects(move, objects) { let moveObjects = []; let errors = []; for (var i in objects) { if (!objects[i].id && !objects[i].beacon) { - errors.push(`${moveXXX}.objects[${i}].id or ${moveXXX}.objects[${i}].beacon must be present`) + errors.push(`${move}.objects[${i}].id or ${move}.objects[${i}].beacon must be present`) } else if (objects[i].id && typeof objects[i].id !== "string") { - errors.push(`${moveXXX}.objects[${i}].id must be string`) + errors.push(`${move}.objects[${i}].id must be string`) } else if (objects[i].beacon && typeof objects[i].beacon !== "string") { - errors.push(`${moveXXX}.objects[${i}].beacon must be string`) + errors.push(`${move}.objects[${i}].beacon must be string`) } else { var objs = [] if (objects[i].id) {