From 174fc5b53ce7e58a192f4ec63f57d5eb4756d24e Mon Sep 17 00:00:00 2001 From: Lorenz Sieben Date: Fri, 6 Dec 2024 16:50:11 +0100 Subject: [PATCH 1/4] Change schema to cache computed properties in the database --- dbschema/default.esdl | 128 +++++++++++++++++------ dbschema/migrations/00005-m1smzp7.edgeql | 111 ++++++++++++++++++++ 2 files changed, 205 insertions(+), 34 deletions(-) create mode 100644 dbschema/migrations/00005-m1smzp7.edgeql diff --git a/dbschema/default.esdl b/dbschema/default.esdl index 9cad41a..9a7a42c 100644 --- a/dbschema/default.esdl +++ b/dbschema/default.esdl @@ -3,6 +3,30 @@ module default { scalar type AnalysisType extending enum; scalar type ControllerResponse extending enum; scalar type ComplaintType extending enum; + scalar type ProceedingState extending + enum; + scalar type ComplaintState extending + enum; + abstract type CreatedOn { required property createdOn: datetime { @@ -26,7 +50,9 @@ module default { } type Analysis { - required proceeding: Proceeding { on target delete delete source; }; + single proceeding := + .'formal' and not exists(.userNetworkActivity) else - 'askLoggedIntoAppStore' if .complaintType ?= 'formal' and not exists(.loggedIntoAppStore) else - 'askDeviceHasRegisteredSimCard' if .complaintType ?= 'formal' and not exists(.deviceHasRegisteredSimCard) else - 'askDeveloperAddress' if not exists(.developerAddress) else - 'readyToSend' + ComplaintState.notYet if .state != ProceedingState.awaitingComplaint else + ComplaintState.askIsUserOfApp if not exists(.complainantIsUserOfApp) else + ComplaintState.askAuthority if not exists(.complaintAuthority) else + ComplaintState.askComplaintType if not exists(.complaintType) else + ComplaintState.askUserNetworkActivity if .complaintType ?= ComplaintType.formal and not exists(.userNetworkActivity) else + ComplaintState.askLoggedIntoAppStore if .complaintType ?= ComplaintType.formal and not exists(.loggedIntoAppStore) else + ComplaintState.askDeviceHasRegisteredSimCard if .complaintType ?= ComplaintType.formal and not exists(.deviceHasRegisteredSimCard) else + ComplaintState.askDeveloperAddress if not exists(.developerAddress) else + ComplaintState.readyToSend ); - single initialAnalysis := (select Analysis filter .proceeding = Proceeding and .type = 'initial' limit 1); - single secondAnalysis := (select Analysis filter .proceeding = Proceeding and .type = 'second' limit 1); + stateUpdatedOn := (.; + ALTER TYPE default::Proceeding { + CREATE REQUIRED PROPERTY state: default::ProceedingState { + SET default := (default::ProceedingState.needsInitialAnalysis); + CREATE REWRITE + INSERT + USING ((default::ProceedingState.erased IF EXISTS (.erased) ELSE (default::ProceedingState.expired IF EXISTS (.expired) ELSE (default::ProceedingState.needsInitialAnalysis IF (NOT (EXISTS (.initialAnalysis)) AND EXISTS (.requestedAnalysis)) ELSE (default::ProceedingState.initialAnalysisFailed IF (NOT (EXISTS (.initialAnalysis)) AND NOT (EXISTS (.requestedAnalysis))) ELSE (default::ProceedingState.initialAnalysisFoundNothing IF NOT (.initialAnalysis.foundTrackers) ELSE (default::ProceedingState.awaitingControllerNotice IF (NOT (EXISTS (.noticeSent)) AND .initialAnalysis.foundTrackers) ELSE (default::ProceedingState.awaitingControllerResponse IF NOT (EXISTS (.controllerResponse)) ELSE (default::ProceedingState.needsSecondAnalysis IF (NOT (EXISTS (.secondAnalysis)) AND EXISTS (.requestedAnalysis)) ELSE (default::ProceedingState.secondAnalysisFailed IF (NOT (EXISTS (.secondAnalysis)) AND NOT (EXISTS (.requestedAnalysis))) ELSE (default::ProceedingState.secondAnalysisFoundNothing IF NOT (.secondAnalysis.foundTrackers) ELSE (default::ProceedingState.awaitingComplaint IF (NOT (EXISTS (.complaintSent)) AND .secondAnalysis.foundTrackers) ELSE default::ProceedingState.complaintSent)))))))))))); + CREATE REWRITE + UPDATE + USING ((default::ProceedingState.erased IF EXISTS (.erased) ELSE (default::ProceedingState.expired IF EXISTS (.expired) ELSE (default::ProceedingState.needsInitialAnalysis IF (NOT (EXISTS (.initialAnalysis)) AND EXISTS (.requestedAnalysis)) ELSE (default::ProceedingState.initialAnalysisFailed IF (NOT (EXISTS (.initialAnalysis)) AND NOT (EXISTS (.requestedAnalysis))) ELSE (default::ProceedingState.initialAnalysisFoundNothing IF NOT (.initialAnalysis.foundTrackers) ELSE (default::ProceedingState.awaitingControllerNotice IF (NOT (EXISTS (.noticeSent)) AND .initialAnalysis.foundTrackers) ELSE (default::ProceedingState.awaitingControllerResponse IF NOT (EXISTS (.controllerResponse)) ELSE (default::ProceedingState.needsSecondAnalysis IF (NOT (EXISTS (.secondAnalysis)) AND EXISTS (.requestedAnalysis)) ELSE (default::ProceedingState.secondAnalysisFailed IF (NOT (EXISTS (.secondAnalysis)) AND NOT (EXISTS (.requestedAnalysis))) ELSE (default::ProceedingState.secondAnalysisFoundNothing IF NOT (.secondAnalysis.foundTrackers) ELSE (default::ProceedingState.awaitingComplaint IF (NOT (EXISTS (.complaintSent)) AND .secondAnalysis.foundTrackers) ELSE default::ProceedingState.complaintSent)))))))))))); + }; + }; + CREATE SCALAR TYPE default::ComplaintState EXTENDING enum; + ALTER TYPE default::Proceeding { + CREATE REQUIRED PROPERTY complaintState := ((default::ComplaintState.notYet IF (.state != default::ProceedingState.awaitingComplaint) ELSE (default::ComplaintState.askIsUserOfApp IF NOT (EXISTS (.complainantIsUserOfApp)) ELSE (default::ComplaintState.askAuthority IF NOT (EXISTS (.complaintAuthority)) ELSE (default::ComplaintState.askComplaintType IF NOT (EXISTS (.complaintType)) ELSE (default::ComplaintState.askUserNetworkActivity IF ((.complaintType ?= default::ComplaintType.formal) AND NOT (EXISTS (.userNetworkActivity))) ELSE (default::ComplaintState.askLoggedIntoAppStore IF ((.complaintType ?= default::ComplaintType.formal) AND NOT (EXISTS (.loggedIntoAppStore))) ELSE (default::ComplaintState.askDeviceHasRegisteredSimCard IF ((.complaintType ?= default::ComplaintType.formal) AND NOT (EXISTS (.deviceHasRegisteredSimCard))) ELSE (default::ComplaintState.askDeveloperAddress IF NOT (EXISTS (.developerAddress)) ELSE default::ComplaintState.readyToSend))))))))); + }; + ALTER TYPE default::Proceeding { + CREATE TRIGGER logStateUpdate + AFTER UPDATE + FOR EACH + WHEN ((__old__.state != __new__.state)) + DO (UPDATE + default::ProceedingUpdateLog + FILTER + (.proceeding = __new__) + SET { + stateUpdatedOn := std::datetime_of_statement() + }); + }; + UPDATE default::Proceeding SET { + secondAnalysis := (SELECT . Date: Fri, 6 Dec 2024 20:05:12 +0100 Subject: [PATCH 2/4] Update JS API to the changes --- src/pages/a/[platform]/[appId]/index.astro | 7 ++++-- src/pages/private-api/analysis-result.ts | 27 ++++++++++++---------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/pages/a/[platform]/[appId]/index.astro b/src/pages/a/[platform]/[appId]/index.astro index 8dc8e7a..5e8f8a6 100644 --- a/src/pages/a/[platform]/[appId]/index.astro +++ b/src/pages/a/[platform]/[appId]/index.astro @@ -22,11 +22,14 @@ const analyses = await e .run(client); if (analyses.length === 0) return new Response('We have not analyzed this app yet.', { status: 404 }); +const mostRecentAnalysis = analyses[0]!; +if (!mostRecentAnalysis.proceeding) + throw new Error('The analysis has no backlink to a proceeding. This should never happen.'); // An app can theoretically change its name with each new version, we thus use the one we saw in the most recent proceeding. -const name = analyses[0]!.proceeding.appName; +const name = mostRecentAnalysis.proceeding.appName; // On iOS, we need the adamId for getAppMeta(). -const adamId = analyses[0]!.app.adamId; +const adamId = mostRecentAnalysis.app!.adamId; --- diff --git a/src/pages/private-api/analysis-result.ts b/src/pages/private-api/analysis-result.ts index 19b6163..1a9fd8e 100644 --- a/src/pages/private-api/analysis-result.ts +++ b/src/pages/private-api/analysis-result.ts @@ -68,21 +68,24 @@ export const POST: APIRoute = async ({ request }) => { if (app.id !== proceeding.app.appId) throw new Error('The HAR file contains traffic for a different app.'); await e - .insert(e.Analysis, { - proceeding: e - // eslint-disable-next-line camelcase - .select(e.Proceeding, () => ({ filter_single: { id: proceeding.id } })), - type: requestedAnalysis.type, + .update(e.Proceeding, () => ({ + // eslint-disable-next-line camelcase + filter_single: { id: proceeding.id }, + set: { + [requestedAnalysis.type === 'initial' ? 'initialAnalysis' : 'secondAnalysis']: e.insert(e.Analysis, { + type: requestedAnalysis.type, - startDate: new Date(res.har.log._tweasel.startDate), - endDate: new Date(res.har.log._tweasel.endDate), + startDate: new Date(res.har.log._tweasel.startDate), + endDate: new Date(res.har.log._tweasel.endDate), - appVersion: app.version, - appVersionCode: app.versionCode, + appVersion: app.version, + appVersionCode: app.versionCode, - har: JSON.stringify(res.har), - trackHarResult: res.trackHarResult, - }) + har: JSON.stringify(res.har), + trackHarResult: res.trackHarResult, + }), + }, + })) .run(client); await deleteRequestedAnalysis(); From 334b93d0ca352a2969e11faf56101337c4221b5a Mon Sep 17 00:00:00 2001 From: Lorenz Sieben Date: Wed, 11 Dec 2024 17:00:38 +0100 Subject: [PATCH 3/4] Fix integration tests --- cypress/e2e/use-cases/android.cy.ts | 8 ++++---- src/pages/p/[platform]/[appId]/index.ts | 10 ++++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/cypress/e2e/use-cases/android.cy.ts b/cypress/e2e/use-cases/android.cy.ts index 29dd7c0..13d8d3e 100644 --- a/cypress/e2e/use-cases/android.cy.ts +++ b/cypress/e2e/use-cases/android.cy.ts @@ -65,7 +65,7 @@ describe('Use case: Analysing Android apps', () => { cy.contains('To Whom It May Concern:'); cy.get('#upload').selectFile('cypress/fixtures/qd/notice.eml'); - cy.get('button').contains('Upload').click(); + cy.get('button').contains('Upload').scrollIntoView().click({ force: true }); cy.contains('Waiting for the developer'); cy.get('#upload'); @@ -101,19 +101,19 @@ describe('Use case: Analysing Android apps', () => { cy.contains('Irish Data Protection Commission').click(); cy.contains('How to contact the DPA about the app?'); - if (useCase === 'informal-complaint') cy.contains('Informally ask the DPA to look in the app').click(); + if (useCase === 'informal-complaint') cy.contains('Informally ask the DPA to look into the app').click(); else cy.contains('Check whether I am personally affected').click(); if (useCase === 'formal-complaint') { cy.contains('Prove that you are affected by the tracking'); cy.get('#upload').selectFile('cypress/fixtures/qd/trackercontrol.csv'); - cy.get('button').contains('Upload').click(); + cy.get('button').contains('Upload').scrollIntoView().click({ force: true }); cy.contains('How did you install the app?'); cy.contains('through the Google Play Store').click(); - cy.contains('Do you have a SIM card in your phone?'); + cy.contains('Do you have a SIM card in your device?'); cy.contains('I have a SIM card').click(); } diff --git a/src/pages/p/[platform]/[appId]/index.ts b/src/pages/p/[platform]/[appId]/index.ts index 3d84236..b023a86 100644 --- a/src/pages/p/[platform]/[appId]/index.ts +++ b/src/pages/p/[platform]/[appId]/index.ts @@ -24,8 +24,8 @@ export const POST: APIRoute = async ({ params, redirect, currentLocale, clientAd const { token: analysisToken } = await startAnalysis(platform, appMeta.appId); - await e - .insert(e.Proceeding, { + try { + e.insert(e.Proceeding, { app: e .insert(e.App, { platform, @@ -51,8 +51,10 @@ export const POST: APIRoute = async ({ params, redirect, currentLocale, clientAd type: 'initial', token: analysisToken, }), - }) - .run(client); + }).run(client); + } catch (err) { + return new Response('Database Error', { status: 500 }); + } return redirect(`/p/${token}`); }; From 58634a2a321423feb0277dad1051c3cc0d777bb2 Mon Sep 17 00:00:00 2001 From: Lorenz Sieben Date: Thu, 2 Jan 2025 19:10:29 +0100 Subject: [PATCH 4/4] Fixed problem with failed analyses fix --- .circleci/config.yml | 2 +- dbschema/default.esdl | 11 ++++- dbschema/migrations/00006-m1o43u7.edgeql | 16 +++++++ src/pages/a/index.astro | 43 +++++++++++------- src/pages/p/[platform]/[appId]/index.ts | 56 ++++++++++++------------ src/pages/p/[token]/attachment/[type].ts | 2 +- src/pages/private-api/analysis-result.ts | 7 ++- 7 files changed, 89 insertions(+), 48 deletions(-) create mode 100644 dbschema/migrations/00006-m1o43u7.edgeql diff --git a/.circleci/config.yml b/.circleci/config.yml index 574e5b5..23d3f7f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -29,7 +29,7 @@ jobs: edgedb-server-5 --background --data-dir ~/edgedb/data --runstate-dir ~/edgedb/runstate --log-to ~/edgedb/logs --security insecure_dev_mode edgedb instance link --host localhost --port 5656 --user edgedb --branch main --non-interactive --trust-tls-cert tweasel-platform - edgedb migration apply + edgedb migrate - cypress/run-tests: start-command: 'yarn build && yarn npm-run-all -p mock-analysis-runner start' cypress-command: "npx wait-on 'http://localhost:4321' && if [ -z ${CYPRESS_RECORD_KEY+x} ]; then yarn cypress run; else yarn cypress run --record; fi" diff --git a/dbschema/default.esdl b/dbschema/default.esdl index 9a7a42c..44ccbba 100644 --- a/dbschema/default.esdl +++ b/dbschema/default.esdl @@ -103,6 +103,8 @@ module default { default := ProceedingState.needsInitialAnalysis; }; + # We need to use these triggers to create separate log entries because we need to compare the state after + # modification, but can not edit the triggering entry in a trigger. trigger logStateUpdate after update for each when (__old__.state != __new__.state) do ( @@ -119,6 +121,13 @@ module default { } ); + # This is to ensure there are no orphaned RequestedAnalysis + trigger removeRequestedAnalysis after update for each + when (__old__.requestedAnalysis not in __new__.requestedAnalysis) + do ( + delete __old__.requestedAnalysis + ); + required complaintState := ( ComplaintState.notYet if .state != ProceedingState.awaitingComplaint else ComplaintState.askIsUserOfApp if not exists(.complainantIsUserOfApp) else @@ -160,7 +169,7 @@ module default { multi uploads := . r.slice(0, 10))); -const resultsWithAnalysis = - searchResults && searchResults.length > 0 - ? await e - .group( - e.select(e.Analysis, (a) => ({ - filter: e.all( - e.set( - e.op(a.app.platform, '=', e.literal(e.Platform, platform)), - e.op(a.type, '=', e.literal(e.AnalysisType, 'initial')), - e.op(a.app.appId, 'in', e.set(...searchResults.map((r) => r.id))), +let resultsWithAnalysis: { + key: { app_id: string | null }; + elements: { id: string }[]; +}[] = []; + +try { + resultsWithAnalysis = + searchResults && searchResults.length > 0 + ? await e + .group( + e.select(e.Analysis, (a) => ({ + filter: e.all( + e.set( + e.op(a.app.platform, '=', e.literal(e.Platform, platform)), + e.op(a.type, '=', e.literal(e.AnalysisType, 'initial')), + e.op(a.app.appId, 'in', e.set(...searchResults.map((r) => r.id))), + ), ), - ), - })), - (a) => ({ id: true, by: { app_id: a.app.appId } }), - ) - .run(client) - : []; + })), + (a) => ({ id: true, by: { app_id: a.app.appId } }), + ) + .run(client) + : []; +} catch (error) { + console.log(error); + return new Response('Database Error', { status: 500 }); +} + const results = searchResults && searchResults.map((r) => ({ diff --git a/src/pages/p/[platform]/[appId]/index.ts b/src/pages/p/[platform]/[appId]/index.ts index b023a86..6c07b90 100644 --- a/src/pages/p/[platform]/[appId]/index.ts +++ b/src/pages/p/[platform]/[appId]/index.ts @@ -25,33 +25,35 @@ export const POST: APIRoute = async ({ params, redirect, currentLocale, clientAd const { token: analysisToken } = await startAnalysis(platform, appMeta.appId); try { - e.insert(e.Proceeding, { - app: e - .insert(e.App, { - platform, - appId: appMeta.appId, - adamId: appMeta.adamId, - }) - .unlessConflict((app) => ({ - on: e.tuple([app.platform, app.appId]), - else: app, - })), - - token, - reference: generateReference(new Date()), - - appName: appMeta.appName, - developerName: appMeta.developerName, - developerEmail: appMeta.developerEmail, - developerAddress: appMeta.developerAddress, - ...(appMeta.developerAddress && { developerAddressSourceUrl: appMeta.storeUrl }), - privacyPolicyUrl: appMeta.privacyPolicyUrl, - - requestedAnalysis: e.insert(e.RequestedAnalysis, { - type: 'initial', - token: analysisToken, - }), - }).run(client); + await e + .insert(e.Proceeding, { + app: e + .insert(e.App, { + platform, + appId: appMeta.appId, + adamId: appMeta.adamId, + }) + .unlessConflict((app) => ({ + on: e.tuple([app.platform, app.appId]), + else: app, + })), + + token, + reference: generateReference(new Date()), + + appName: appMeta.appName, + developerName: appMeta.developerName, + developerEmail: appMeta.developerEmail, + developerAddress: appMeta.developerAddress, + ...(appMeta.developerAddress && { developerAddressSourceUrl: appMeta.storeUrl }), + privacyPolicyUrl: appMeta.privacyPolicyUrl, + + requestedAnalysis: e.insert(e.RequestedAnalysis, { + type: 'initial', + token: analysisToken, + }), + }) + .run(client); } catch (err) { return new Response('Database Error', { status: 500 }); } diff --git a/src/pages/p/[token]/attachment/[type].ts b/src/pages/p/[token]/attachment/[type].ts index 050a844..ddc3c38 100644 --- a/src/pages/p/[token]/attachment/[type].ts +++ b/src/pages/p/[token]/attachment/[type].ts @@ -155,7 +155,7 @@ export const POST: APIRoute = async ({ params, currentLocale, request }) => { .formData({ complainantAddress: zfd.text(), complainantContactDetails: zfd.text(), - complainantAgreesToUnencryptedCommunication: zfd.text(z.enum(['yes', 'no'])), + complainantAgreesToUnencryptedCommunication: zfd.text(z.enum(['yes', 'no-letter'])), }) .parse(await request.formData()); diff --git a/src/pages/private-api/analysis-result.ts b/src/pages/private-api/analysis-result.ts index 1a9fd8e..73d85df 100644 --- a/src/pages/private-api/analysis-result.ts +++ b/src/pages/private-api/analysis-result.ts @@ -49,9 +49,12 @@ export const POST: APIRoute = async ({ request }) => { const deleteRequestedAnalysis = () => e - .delete(e.RequestedAnalysis, () => ({ + .update(e.Proceeding, () => ({ // eslint-disable-next-line camelcase - filter_single: { id: requestedAnalysis.id }, + filter_single: { id: proceeding.id }, + set: { + requestedAnalysis: null, + }, })) .run(client);