From ea2117151ad6290189efd371aff2a6a03fecd091 Mon Sep 17 00:00:00 2001 From: Stephan Kulla Date: Sat, 15 Jun 2024 12:24:41 +0200 Subject: [PATCH] refactor(deleted-entities): Migrate to SQL query --- __tests__/__utils__/handlers.ts | 31 ------- __tests__/schema/entity/deleted-entities.ts | 82 +++---------------- packages/server/src/model/database-layer.ts | 16 ---- packages/server/src/model/serlo.ts | 11 --- .../schema/uuid/abstract-entity/resolvers.ts | 42 +++++++--- 5 files changed, 43 insertions(+), 139 deletions(-) diff --git a/__tests__/__utils__/handlers.ts b/__tests__/__utils__/handlers.ts index 0c80c6ceb..4d74ee90e 100644 --- a/__tests__/__utils__/handlers.ts +++ b/__tests__/__utils__/handlers.ts @@ -21,37 +21,6 @@ const ForDefinitions = { given('UuidQuery').withPayload({ id: uuid.id }).returns(uuid) } }, - DeletedEntitiesQuery(entities: Model<'AbstractEntity'>[]) { - given('UuidQuery').for( - entities.map((entity) => { - return { ...entity, trashed: true } - }), - ) - given('DeletedEntitiesQuery').isDefinedBy(async ({ request }) => { - const body = await request.json() - const { first, after, instance } = body.payload - - const entitiesByInstance = instance - ? entities.filter((entity) => entity.instance === instance) - : entities - - const entitiesByAfter = after - ? entitiesByInstance.filter( - (entity) => new Date(entity.date) > new Date(after), - ) - : entitiesByInstance - - const entitiesByFirst = entitiesByAfter.slice(0, first) - - const deletedEntities = entitiesByFirst.map((entity) => { - return { - id: entity.id, - dateOfDeletion: entity.date, - } - }) - return HttpResponse.json({ deletedEntities }) - }) - }, } type ForDefinitions = typeof ForDefinitions type ForArg = M extends keyof ForDefinitions diff --git a/__tests__/schema/entity/deleted-entities.ts b/__tests__/schema/entity/deleted-entities.ts index 540680d5f..8889943fd 100644 --- a/__tests__/schema/entity/deleted-entities.ts +++ b/__tests__/schema/entity/deleted-entities.ts @@ -1,10 +1,6 @@ import gql from 'graphql-tag' -import { - article as baseArticle, - coursePage as baseCoursePage, -} from '../../../__fixtures__' -import { given, Client } from '../../__utils__' +import { Client } from '../../__utils__' import { Instance } from '~/types' const query = new Client().prepareQuery({ @@ -24,11 +20,9 @@ const query = new Client().prepareQuery({ `, }) -const coursePage = { ...baseCoursePage, instance: Instance.En } -const article = { ...baseArticle, date: '2015-03-01T20:45:56Z' } - -beforeEach(() => { - given('DeletedEntitiesQuery').for(article, coursePage) +beforeEach(async () => { + // The database dump does not contain any deleted entities, thus we create some here + await databaseForTests.mutate('update uuid set trashed = 1 where id = 1615') }) test('returns deleted entities', async () => { @@ -36,27 +30,7 @@ test('returns deleted entities', async () => { entity: { deletedEntities: { nodes: [ - { - dateOfDeletion: article.date, - entity: { id: article.id }, - }, - { - dateOfDeletion: coursePage.date, - entity: { id: coursePage.id }, - }, - ], - }, - }, - }) - - await query.withVariables({ first: 1 }).shouldReturnData({ - entity: { - deletedEntities: { - nodes: [ - { - dateOfDeletion: article.date, - entity: { id: article.id }, - }, + { entity: { id: 1615 }, dateOfDeletion: '2014-04-21T14:40:28.000Z' }, ], }, }, @@ -68,37 +42,20 @@ test('paginates with `after` parameter { entityId, dateOfDeletion}, ', async () .withVariables({ after: Buffer.from( JSON.stringify({ - id: coursePage.id, - dateOfDeletion: coursePage.date, + id: 1615, + dateOfDeletion: '2014-04-21T14:40:28.000Z', }), ).toString('base64'), }) .shouldReturnData({ - entity: { - deletedEntities: { - nodes: [ - { - dateOfDeletion: article.date, - entity: { id: article.id }, - }, - ], - }, - }, + entity: { deletedEntities: { nodes: [] } }, }) }) test('filters by instance', async () => { - await query - .withVariables({ - instance: Instance.En, - }) - .shouldReturnData({ - entity: { - deletedEntities: { - nodes: [{ entity: { id: coursePage.id } }], - }, - }, - }) + await query.withVariables({ instance: Instance.En }).shouldReturnData({ + entity: { deletedEntities: { nodes: [] } }, + }) }) test('fails when `first` is too high', async () => { @@ -111,23 +68,8 @@ test('fails when `after` is malformed', async () => { await query .withVariables({ after: Buffer.from( - JSON.stringify({ - id: article.id, - dateOfDeletion: 'foo', - }), + JSON.stringify({ id: 1865, dateOfDeletion: 'foo' }), ).toString('base64'), }) .shouldFailWithError('BAD_USER_INPUT') }) - -test('fails when database layer returns a 400er response', async () => { - given('DeletedEntitiesQuery').returnsBadRequest() - - await query.shouldFailWithError('BAD_USER_INPUT') -}) - -test('fails when database layer has an internal error', async () => { - given('DeletedEntitiesQuery').hasInternalServerError() - - await query.shouldFailWithError('INTERNAL_SERVER_ERROR') -}) diff --git a/packages/server/src/model/database-layer.ts b/packages/server/src/model/database-layer.ts index cd30daaf4..b74fb36c3 100644 --- a/packages/server/src/model/database-layer.ts +++ b/packages/server/src/model/database-layer.ts @@ -20,22 +20,6 @@ export const spec = { }), canBeNull: false, }, - DeletedEntitiesQuery: { - payload: t.type({ - first: t.number, - after: t.union([t.string, t.undefined]), - instance: t.union([InstanceDecoder, t.undefined]), - }), - response: t.type({ - deletedEntities: t.array( - t.type({ - id: t.number, - dateOfDeletion: t.string, - }), - ), - }), - canBeNull: false, - }, EntitySortMutation: { payload: t.type({ childrenIds: t.array(t.number), entityId: t.number }), response: t.type({ success: t.boolean }), diff --git a/packages/server/src/model/serlo.ts b/packages/server/src/model/serlo.ts index 7576ba158..439fec3e5 100644 --- a/packages/server/src/model/serlo.ts +++ b/packages/server/src/model/serlo.ts @@ -117,16 +117,6 @@ export function createSerloModel({ }, }) - const getDeletedEntities = createRequest({ - type: 'DeletedEntitiesQuery', - decoder: DatabaseLayer.getDecoderFor('DeletedEntitiesQuery'), - async getCurrentValue( - payload: DatabaseLayer.Payload<'DeletedEntitiesQuery'>, - ) { - return DatabaseLayer.makeRequest('DeletedEntitiesQuery', payload) - }, - }) - const sortEntity = createMutation({ type: 'EntitySortMutation', decoder: DatabaseLayer.getDecoderFor('EntitySortMutation'), @@ -164,7 +154,6 @@ export function createSerloModel({ executePrompt, getActiveReviewerIds, getActivityByType, - getDeletedEntities, getPotentialSpamUsers, getUsersByRole, getPages, diff --git a/packages/server/src/schema/uuid/abstract-entity/resolvers.ts b/packages/server/src/schema/uuid/abstract-entity/resolvers.ts index a300ace84..9a28ce028 100644 --- a/packages/server/src/schema/uuid/abstract-entity/resolvers.ts +++ b/packages/server/src/schema/uuid/abstract-entity/resolvers.ts @@ -63,19 +63,39 @@ export const resolvers: Resolvers = { EntityQuery: { async deletedEntities(_parent, payload, context) { const LIMIT = 1000 - const { first = 100, after, instance } = payload + const { first = 100, after, instance = null } = payload if (first > LIMIT) throw new UserInputError(`'first' may not be higher than ${LIMIT}`) - const deletedAfter = after ? decodeDateOfDeletion(after) : undefined - - const { deletedEntities } = - await context.dataSources.model.serlo.getDeletedEntities({ - first: first + 1, - after: deletedAfter, - instance, - }) + const deletedAfter = after + ? decodeDateOfDeletion(after) + : new Date('9999-12-31') + + const deletedEntities = await context.database.fetchAll<{ + id: number + dateOfDeletion: Date + }>( + ` + SELECT + uuid.id as id, + MAX(event.date) AS dateOfDeletion + FROM event + JOIN uuid ON uuid.id = event.uuid_id + JOIN entity ON entity.id = uuid.id + JOIN instance ON instance.id = entity.instance_id + WHERE uuid.id = event.uuid_id + AND event.date < ? + AND (? is null OR instance.subdomain = ?) + AND event.event_type_id = 10 + AND uuid.trashed = 1 + AND entity.type_id NOT IN (35, 39, 40, 41, 42, 43, 44) + GROUP BY uuid.id + ORDER BY dateOfDeletion DESC + LIMIT ?; + `, + [deletedAfter, instance, instance, first.toString()], + ) const nodes = await Promise.all( deletedEntities.map(async (node) => { @@ -85,7 +105,7 @@ export const resolvers: Resolvers = { { id: node.id }, context, ), - dateOfDeletion: node.dateOfDeletion, + dateOfDeletion: node.dateOfDeletion.toISOString(), } }), ) @@ -538,7 +558,7 @@ function decodeDateOfDeletion(after: string) { 'The encoded dateOfDeletion in `after` should be a string in date format', ) - return new Date(dateOfDeletion).toISOString() + return new Date(dateOfDeletion) } async function isAutoreviewEntity(