From f6723bb67a44f01f2c2930767ee61b9fa417f629 Mon Sep 17 00:00:00 2001 From: Matchu Date: Sun, 8 Nov 2020 00:20:21 -0800 Subject: [PATCH] fix edge case bugs in itemByName and itemsByName not-found case was erroring instead of returning a simple null, and string trimming wasn't working right, oops! --- src/server/loaders.js | 54 +++--- src/server/query-tests/Item.test.js | 9 +- .../__snapshots__/Item.test.js.snap | 156 ++++-------------- 3 files changed, 65 insertions(+), 154 deletions(-) diff --git a/src/server/loaders.js b/src/server/loaders.js index 210d97a..e3cc6ab 100644 --- a/src/server/loaders.js +++ b/src/server/loaders.js @@ -148,35 +148,39 @@ const buildItemTranslationLoader = (db) => }); const buildItemByNameLoader = (db, loaders) => - new DataLoader(async (names) => { - const qs = names.map((_) => "?").join(", "); - const [rows, _] = await db.execute( - { - // NOTE: In our MySQL schema, this is a case-insensitive exact search. - sql: `SELECT items.*, item_translations.* FROM item_translations + new DataLoader( + async (names) => { + const qs = names.map((_) => "?").join(", "); + const normalizedNames = names.map((name) => name.trim().toLowerCase()); + const [rows, _] = await db.execute( + { + // NOTE: In our MySQL schema, this is a case-insensitive exact search. + sql: `SELECT items.*, item_translations.* FROM item_translations INNER JOIN items ON items.id = item_translations.item_id WHERE name IN (${qs}) AND locale = "en"`, - nestTables: true, - }, - names - ); + nestTables: true, + }, + normalizedNames + ); - const entities = rows.map((row) => { - const item = normalizeRow(row.items); - const itemTranslation = normalizeRow(row.item_translations); - loaders.itemLoader.prime(item.id, item); - loaders.itemTranslationLoader.prime(item.id, itemTranslation); - return { item, itemTranslation }; - }); + const entitiesByName = new Map(); + for (const row of rows) { + const item = normalizeRow(row.items); + const itemTranslation = normalizeRow(row.item_translations); + loaders.itemLoader.prime(item.id, item); + loaders.itemTranslationLoader.prime(item.id, itemTranslation); - return names.map((name) => - entities.find( - (e) => - e.itemTranslation.name.trim().toLowerCase() === - name.trim().toLowerCase() - ) - ); - }); + const normalizedName = itemTranslation.name.trim().toLowerCase(); + entitiesByName.set(normalizedName, { item, itemTranslation }); + } + + return normalizedNames.map( + (name) => + entitiesByName.get(name) || { item: null, itemTranslation: null } + ); + }, + { cacheKeyFn: (name) => name.trim().toLowerCase() } + ); const buildItemSearchLoader = (db, loaders) => new DataLoader(async (queries) => { diff --git a/src/server/query-tests/Item.test.js b/src/server/query-tests/Item.test.js index 9a11d15..4c9eaae 100644 --- a/src/server/query-tests/Item.test.js +++ b/src/server/query-tests/Item.test.js @@ -113,7 +113,14 @@ describe("Item", () => { name thumbnailUrl } - itemsByName(names: ["Zafara Agent Robe", "pile of dung"]) { + notFoundItem: itemByName(name: "?! fake name") { + id + name + thumbnailUrl + } + itemsByName( + names: [" Zafara Agent Robe ", "pile of dung", "?! fake name"] + ) { id name thumbnailUrl diff --git a/src/server/query-tests/__snapshots__/Item.test.js.snap b/src/server/query-tests/__snapshots__/Item.test.js.snap index 1dbdce7..51fb1fa 100644 --- a/src/server/query-tests/__snapshots__/Item.test.js.snap +++ b/src/server/query-tests/__snapshots__/Item.test.js.snap @@ -742,7 +742,9 @@ Object { "name": "Pile of Dung", "thumbnailUrl": "http://images.neopets.com/items/med_booby_5.gif", }, + null, ], + "notFoundItem": null, } `; @@ -753,16 +755,18 @@ Array [ "nestTables": true, "sql": "SELECT items.*, item_translations.* FROM item_translations INNER JOIN items ON items.id = item_translations.item_id - WHERE name IN (?, ?, ?) AND locale = \\"en\\"", + WHERE name IN (?, ?, ?, ?) AND locale = \\"en\\"", "values": Array [ - "Moon and Stars Background", - "Zafara Agent Robe", + "moon and stars background", + "?! fake name", + "zafara agent robe", "pile of dung", ], }, Array [ - "Moon and Stars Background", - "Zafara Agent Robe", + "moon and stars background", + "?! fake name", + "zafara agent robe", "pile of dung", ], ], @@ -790,10 +794,6 @@ Object { "id": "28", "name": "Krawk", }, - Object { - "id": "46", - "name": "Techo", - }, ], }, Object { @@ -812,10 +812,6 @@ Object { "id": "38", "name": "Peophin", }, - Object { - "id": "46", - "name": "Techo", - }, ], }, Object { @@ -838,10 +834,6 @@ Object { "id": "38", "name": "Peophin", }, - Object { - "id": "46", - "name": "Techo", - }, ], }, Object { @@ -948,10 +940,6 @@ Object { "id": "36", "name": "Nimmo", }, - Object { - "id": "37", - "name": "Ogrin", - }, Object { "id": "39", "name": "Poogle", @@ -972,10 +960,6 @@ Object { "id": "45", "name": "Skeith", }, - Object { - "id": "46", - "name": "Techo", - }, Object { "id": "47", "name": "Tonu", @@ -1050,10 +1034,6 @@ Object { "id": "36", "name": "Nimmo", }, - Object { - "id": "37", - "name": "Ogrin", - }, Object { "id": "39", "name": "Poogle", @@ -1062,10 +1042,6 @@ Object { "id": "41", "name": "Quiggle", }, - Object { - "id": "42", - "name": "Ruki", - }, Object { "id": "43", "name": "Scorchio", @@ -1074,10 +1050,6 @@ Object { "id": "45", "name": "Skeith", }, - Object { - "id": "46", - "name": "Techo", - }, Object { "id": "47", "name": "Tonu", @@ -1160,10 +1132,6 @@ Object { "id": "36", "name": "Nimmo", }, - Object { - "id": "37", - "name": "Ogrin", - }, Object { "id": "38", "name": "Peophin", @@ -1184,10 +1152,6 @@ Object { "id": "45", "name": "Skeith", }, - Object { - "id": "46", - "name": "Techo", - }, Object { "id": "47", "name": "Tonu", @@ -1212,10 +1176,6 @@ Object { "id": "81137", "name": "Maraquan Ocean Blue Contacts", "speciesThatNeedModels": Array [ - Object { - "id": "8", - "name": "Chomby", - }, Object { "id": "13", "name": "Flotsam", @@ -1270,10 +1230,6 @@ Object { "id": "81287", "name": "Maraquan White Lace Gown", "speciesThatNeedModels": Array [ - Object { - "id": "8", - "name": "Chomby", - }, Object { "id": "13", "name": "Flotsam", @@ -1324,10 +1280,6 @@ Object { "id": "7", "name": "Chia", }, - Object { - "id": "8", - "name": "Chomby", - }, Object { "id": "13", "name": "Flotsam", @@ -1398,10 +1350,6 @@ Object { "id": "7", "name": "Chia", }, - Object { - "id": "8", - "name": "Chomby", - }, Object { "id": "13", "name": "Flotsam", @@ -1900,10 +1848,6 @@ Object { "id": "53", "name": "Yurble", }, - Object { - "id": "54", - "name": "Zafara", - }, Object { "id": "55", "name": "Vandagyre", @@ -1932,10 +1876,6 @@ Object { "id": "21", "name": "Jubjub", }, - Object { - "id": "24", - "name": "Kiko", - }, Object { "id": "28", "name": "Krawk", @@ -1990,10 +1930,6 @@ Object { "id": "21", "name": "Jubjub", }, - Object { - "id": "24", - "name": "Kiko", - }, Object { "id": "28", "name": "Krawk", @@ -2048,10 +1984,6 @@ Object { "id": "21", "name": "Jubjub", }, - Object { - "id": "24", - "name": "Kiko", - }, Object { "id": "28", "name": "Krawk", @@ -3126,6 +3058,10 @@ Object { "id": "4", "name": "Bori", }, + Object { + "id": "5", + "name": "Bruce", + }, Object { "id": "6", "name": "Buzz", @@ -4714,6 +4650,20 @@ Object { }, ], }, + Object { + "id": "67237", + "name": "Winter Lights Effect", + "speciesThatNeedModels": Array [ + Object { + "id": "28", + "name": "Krawk", + }, + Object { + "id": "38", + "name": "Peophin", + }, + ], + }, Object { "id": "69311", "name": "MME17-S4a: Snow-Covered Balustrade Foreground", @@ -6998,10 +6948,6 @@ Object { "id": "4", "name": "Bori", }, - Object { - "id": "7", - "name": "Chia", - }, Object { "id": "9", "name": "Cybunny", @@ -7014,14 +6960,6 @@ Object { "id": "17", "name": "Grundo", }, - Object { - "id": "20", - "name": "Jetsam", - }, - Object { - "id": "24", - "name": "Kiko", - }, Object { "id": "33", "name": "Meerca", @@ -7032,28 +6970,10 @@ Object { }, ], }, - Object { - "id": "81736", - "name": "Purple & Black Ponytails Wig", - "speciesThatNeedModels": Array [ - Object { - "id": "7", - "name": "Chia", - }, - Object { - "id": "20", - "name": "Jetsam", - }, - ], - }, Object { "id": "81783", "name": "Dyeworks Blue: Decorated Witch Hat and Wig", "speciesThatNeedModels": Array [ - Object { - "id": "7", - "name": "Chia", - }, Object { "id": "26", "name": "Korbat", @@ -7068,18 +6988,6 @@ Object { "id": "5", "name": "Bruce", }, - Object { - "id": "7", - "name": "Chia", - }, - Object { - "id": "20", - "name": "Jetsam", - }, - Object { - "id": "24", - "name": "Kiko", - }, ], }, Object { @@ -7094,10 +7002,6 @@ Object { "id": "11", "name": "Elephante", }, - Object { - "id": "28", - "name": "Krawk", - }, Object { "id": "46", "name": "Techo", @@ -7106,10 +7010,6 @@ Object { "id": "48", "name": "Tuskaninny", }, - Object { - "id": "49", - "name": "Uni", - }, Object { "id": "52", "name": "Xweetok", @@ -7146,6 +7046,7 @@ Array [ "62939", "64195", "64387", + "67237", "69311", "70843", "71937", @@ -7177,7 +7078,6 @@ Array [ "81697", "81708", "81709", - "81736", "81783", "81785", "81792",