From 52787a3a8c3edead141d0d0b9a8e48381699eb83 Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Mon, 20 Jul 2020 17:06:54 +0200 Subject: [PATCH 1/2] fix(catalog): add deduplication for logs join --- ...7114117_location_update_log_latest_view.js | 1 + .../src/database/CommonDatabase.test.ts | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/plugins/catalog-backend/migrations/20200527114117_location_update_log_latest_view.js b/plugins/catalog-backend/migrations/20200527114117_location_update_log_latest_view.js index 083d0a38aa..1d530c6bae 100644 --- a/plugins/catalog-backend/migrations/20200527114117_location_update_log_latest_view.js +++ b/plugins/catalog-backend/migrations/20200527114117_location_update_log_latest_view.js @@ -33,6 +33,7 @@ exports.up = async function up(knex) { ) t2 ON t1.location_id = t2.location_id AND t1.created_at = t2.MAXDATE + GROUP BY t1.location_id ORDER BY created_at DESC; `); }; diff --git a/plugins/catalog-backend/src/database/CommonDatabase.test.ts b/plugins/catalog-backend/src/database/CommonDatabase.test.ts index 3d1455a886..21dde4ca74 100644 --- a/plugins/catalog-backend/src/database/CommonDatabase.test.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.test.ts @@ -88,6 +88,31 @@ describe('CommonDatabase', () => { expect(locations).toEqual([output]); const location = await db.location(locations[0].id); expect(location).toEqual(output); + + // If we add 2 new update log events, + // this should not result in location duplication + // due to incorrect join in DB + await db.addLocationUpdateLogEvent( + 'dd12620d-0436-422f-93bd-929aa0788123', + DatabaseLocationUpdateLogStatus.SUCCESS, + ); + + // Have a second in-between + // To avoid having same timestamp on event + await new Promise(res => setTimeout(res, 1000)); + await db.addLocationUpdateLogEvent( + 'dd12620d-0436-422f-93bd-929aa0788123', + DatabaseLocationUpdateLogStatus.FAIL, + ); + + expect(await db.locations()).toEqual([ + { + ...output, + status: DatabaseLocationUpdateLogStatus.FAIL, + timestamp: expect.any(String), + }, + ]); + await db.transaction(tx => db.removeLocation(tx, locations[0].id)); await expect(db.locations()).resolves.toEqual([]); From 0c5325f63082df9feb7a7906173d0b7ab664b69d Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Wed, 22 Jul 2020 01:56:29 +0200 Subject: [PATCH 2/2] fix(catalog-backend): new migration file --- ...7114117_location_update_log_latest_view.js | 1 - ..._location_update_log_latest_deduplicate.js | 35 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 plugins/catalog-backend/migrations/20200721115244_location_update_log_latest_deduplicate.js diff --git a/plugins/catalog-backend/migrations/20200527114117_location_update_log_latest_view.js b/plugins/catalog-backend/migrations/20200527114117_location_update_log_latest_view.js index 1d530c6bae..083d0a38aa 100644 --- a/plugins/catalog-backend/migrations/20200527114117_location_update_log_latest_view.js +++ b/plugins/catalog-backend/migrations/20200527114117_location_update_log_latest_view.js @@ -33,7 +33,6 @@ exports.up = async function up(knex) { ) t2 ON t1.location_id = t2.location_id AND t1.created_at = t2.MAXDATE - GROUP BY t1.location_id ORDER BY created_at DESC; `); }; diff --git a/plugins/catalog-backend/migrations/20200721115244_location_update_log_latest_deduplicate.js b/plugins/catalog-backend/migrations/20200721115244_location_update_log_latest_deduplicate.js new file mode 100644 index 0000000000..df43a22c56 --- /dev/null +++ b/plugins/catalog-backend/migrations/20200721115244_location_update_log_latest_deduplicate.js @@ -0,0 +1,35 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +exports.up = function up(knex) { + return knex.schema.raw(`DROP VIEW location_update_log_latest;`).raw(` + CREATE VIEW location_update_log_latest AS + SELECT t1.* FROM location_update_log t1 + JOIN + ( + SELECT location_id, MAX(created_at) AS MAXDATE + FROM location_update_log + GROUP BY location_id + ) t2 + ON t1.location_id = t2.location_id + AND t1.created_at = t2.MAXDATE + GROUP BY t1.location_id + ORDER BY created_at DESC; +`); +}; + +exports.down = function down(knex) { + knex.schema.raw(`DROP VIEW location_update_log_latest;`); +};