Skip to content

Commit

Permalink
Merge pull request #1184 from serlo/remove-cache
Browse files Browse the repository at this point in the history
refactor!: Remove ununsed cache mutations set and update
  • Loading branch information
hugotiburtino authored Dec 5, 2023
2 parents 50017c3 + 269f4a3 commit 3ae3f66
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 305 deletions.
91 changes: 3 additions & 88 deletions __tests__/schema/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,63 +2,13 @@ import { option } from 'fp-ts'
import gql from 'graphql-tag'

import { user } from '../../__fixtures__'
import { assertErrorEvent, Client } from '../__utils__'
import { Client } from '../__utils__'
import { Service } from '~/internals/authentication'

beforeEach(() => {
process.env.ENVIRONMENT = 'staging'
})

describe('set', () => {
const key = 'de.serlo.org/api/uuid/1'
const mutation = new Client({ service: Service.Serlo })
.prepareQuery({
query: gql`
mutation ($input: CacheSetInput!) {
_cache {
set(input: $input) {
success
}
}
}
`,
})
.withInput({ key, value: user })

test('sets a certain cache value', async () => {
const now = global.timer.now()

await mutation.shouldReturnData({ _cache: { set: { success: true } } })

const cachedValue = await global.cache.get({ key })
expect(option.isSome(cachedValue) && cachedValue.value).toEqual({
lastModified: now,
value: user,
source: 'Legacy serlo.org listener',
})
})

test('is forbidden when service is not legacy serlo.org', async () => {
await mutation
.withContext({ service: Service.SerloCloudflareWorker })
.shouldFailWithError('FORBIDDEN')
})

test('reports an error when value was invalid', async () => {
const invalidValue = { value: 'Some invalid value' }

await mutation
.withInput({ key, value: invalidValue })
.shouldFailWithError('INTERNAL_SERVER_ERROR')

await assertErrorEvent({
message: 'Invalid value received from listener.',
fingerprint: ['invalid-value', 'listener', key],
errorContext: { key, invalidValueFromListener: invalidValue },
})
})
})

describe('remove', () => {
const key = 'de.serlo.org/api/uuid/1'
const mutation = new Client({ service: Service.Serlo })
Expand All @@ -79,16 +29,9 @@ describe('remove', () => {
await global.cache.set({ key, source: '', value: user })
})

test('removes a cache value (authenticated via Serlo Service)', async () => {
await mutation.shouldReturnData({ _cache: { remove: { success: true } } })

const cachedValue = await global.cache.get({ key })
expect(option.isNone(cachedValue)).toBe(true)
})

test('removes a cache value (authenticated as CarolinJaser)', async () => {
test('removes a cache value (authenticated as Kulla)', async () => {
await mutation
.withContext({ service: Service.SerloCloudflareWorker, userId: 178145 })
.withContext({ service: Service.SerloCloudflareWorker, userId: 26217 })
.shouldReturnData({ _cache: { remove: { success: true } } })

const cachedValue = await global.cache.get({ key })
Expand All @@ -107,31 +50,3 @@ describe('remove', () => {
.shouldFailWithError('FORBIDDEN')
})
})

describe('update', () => {
const mutation = new Client({ service: Service.SerloCacheWorker })
.prepareQuery({
query: gql`
mutation ($input: CacheUpdateInput!) {
_cache {
update(input: $input) {
success
}
}
}
`,
})
.withInput({ keys: [`de.serlo.org/api/uuid/${user.id}`] })

test('updates the key when service is cache-worker', async () => {
await mutation.shouldReturnData({ _cache: { update: { success: true } } })

// TODO: Test that key is passed on the SWR-Queue
})

test('is forbidden when service is not legacy serlo.org', async () => {
await mutation
.withContext({ service: Service.SerloCloudflareWorker })
.shouldFailWithError('FORBIDDEN')
})
})
52 changes: 0 additions & 52 deletions packages/server/src/internals/data-source.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { RESTDataSource } from 'apollo-datasource-rest'
import { option as O, either as E } from 'fp-ts'
import reporter from 'io-ts-reporters'

import { isQuery } from './data-source-helper'
import { Environment } from '~/internals/environment'
import {
createGoogleSpreadsheetApiModel,
Expand Down Expand Up @@ -38,53 +35,4 @@ export class ModelDataSource extends RESTDataSource {
await this.environment.cache.remove({ key })
}
}

public async setCacheValue({ key, value }: { key: string; value: unknown }) {
// TODO: The following code is also used in the SWR queue / worker
// Both codes should be merged together
for (const model of [this.serlo, this.googleSpreadsheetApi]) {
for (const query of Object.values(model)) {
if (
isQuery(query) &&
O.isSome(query._querySpec.getPayload(key)) &&
query._querySpec.decoder !== undefined
) {
const decoded = query._querySpec.decoder.decode(value)

if (E.isLeft(decoded)) {
throw new InvalidValueFromListener({
key,
invalidValueFromListener: value,
decoder: query._querySpec.decoder.name,
validationErrors: reporter.report(decoded),
})
}
}
}
}

await this.environment.cache.set({
key,
value,
source: 'Legacy serlo.org listener',
})
}

public async updateCacheValue({ key }: { key: string }) {
await this.environment.swrQueue.queue({ key })
}
}

// TODO: Find a better place for this error (maybe together with InvalidCurrentValueError)
export class InvalidValueFromListener extends Error {
constructor(
public errorContext: {
key: string
invalidValueFromListener: unknown
decoder: string
validationErrors: string[]
},
) {
super('Invalid value received from listener.')
}
}
12 changes: 0 additions & 12 deletions packages/server/src/internals/sentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import type {
import * as Sentry from '@sentry/node'
import * as R from 'ramda'

import { InvalidValueFromListener } from './data-source'
import { InvalidCurrentValueError } from './data-source-helper'
import { Context } from '~/internals/graphql'

Expand Down Expand Up @@ -90,17 +89,6 @@ export function createSentryPlugin(): ApolloServerPlugin {
scope.setContext('error', errorContext)
}

if (originalError instanceof InvalidValueFromListener) {
const { errorContext } = originalError

scope.setFingerprint([
'invalid-value',
'listener',
errorContext.key,
])
scope.setContext('error', errorContext)
}

return scope
})
}
Expand Down
64 changes: 9 additions & 55 deletions packages/server/src/schema/cache/resolvers.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
import { ForbiddenError } from '~/errors'
import { Service } from '~/internals/authentication'
import { createNamespace, Mutations } from '~/internals/graphql'

const allowedUserIds = [
26217, // kulla
15473, // inyono
131536, // dal
32543, // botho
178145, // CarolinJaser
178807, // HugoBT
245844, // MoeHome
]
Expand All @@ -17,61 +14,18 @@ export const resolvers: Mutations<'_cache'> = {
_cache: createNamespace(),
},
_cacheMutation: {
async set(_parent, payload, { dataSources, service, userId }) {
const { key, value } = payload.input
checkPermission({
service,
userId,
operation: 'set',
allowedServices: [Service.Serlo],
})
await dataSources.model.setCacheValue({ key, value })
return { success: true, query: {} }
},
async remove(_parent, { input }, { dataSources, service, userId }) {
checkPermission({
service,
userId,
operation: 'remove',
allowedServices: [Service.Serlo],
})
async remove(_parent, { input }, { dataSources, userId }) {
if (
process.env.ENVIRONMENT !== 'local' &&
(userId === null || !allowedUserIds.includes(userId))
) {
throw new ForbiddenError(
`You do not have the permissions to remove the cache`,
)
}

await dataSources.model.removeCacheValue({ keys: input.keys })
return { success: true, query: {} }
},
async update(_parent, { input }, { dataSources, service, userId }) {
checkPermission({
service,
userId,
operation: 'update',
allowedServices: [Service.Serlo, Service.SerloCacheWorker],
})
await Promise.all(
input.keys.map((key) => dataSources.model.updateCacheValue({ key })),
)
return { success: true }
},
},
}

function checkPermission({
service,
allowedServices,
userId,
operation,
}: {
service: Service
allowedServices: Service[]
userId: number | null
operation: string
}) {
if (
process.env.ENVIRONMENT !== 'local' &&
!allowedServices.includes(service) &&
(userId === null || !allowedUserIds.includes(userId))
) {
throw new ForbiddenError(
`You do not have the permissions to ${operation} the cache`,
)
}
}
27 changes: 1 addition & 26 deletions packages/server/src/schema/cache/types.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,9 @@ type Mutation {
}

type _cacheMutation {
set(input: CacheSetInput!): CacheSetResponse!
remove(input: CacheRemoveInput!): CacheRemoveResponse!
update(input: CacheUpdateInput!): CacheUpdateResponse!
}

input CacheSetInput {
key: String!
value: JSON!
}

type CacheSetResponse {
success: Boolean!
query: Query!
remove(input: CacheRemoveInput!): DefaultResponse!
}

input CacheRemoveInput {
keys: [String!]!
}

type CacheRemoveResponse {
success: Boolean!
query: Query!
}

input CacheUpdateInput {
keys: [String!]!
}

type CacheUpdateResponse {
success: Boolean!
}
Loading

0 comments on commit 3ae3f66

Please sign in to comment.