From 4f25b0fd1299a3359d15cf929347924f8609edb4 Mon Sep 17 00:00:00 2001 From: devin ivy Date: Mon, 5 Feb 2024 15:05:31 -0500 Subject: [PATCH] Track blob cids on ozone mod events (#2135) track blob cids on ozone mod events --- .../src/api/admin/emitModerationEvent.ts | 14 ++-- .../20240201T051104136Z-mod-event-blobs.ts | 15 ++++ packages/ozone/src/db/migrations/index.ts | 1 + .../ozone/src/db/schema/moderation_event.ts | 1 + packages/ozone/src/db/types.ts | 7 +- packages/ozone/src/mod-service/index.ts | 24 ++++-- packages/ozone/src/mod-service/status.ts | 5 +- packages/ozone/src/mod-service/subject.ts | 11 ++- packages/ozone/src/mod-service/views.ts | 2 +- .../ozone/tests/moderation-events.test.ts | 75 +++++++++++++++++-- .../ozone/tests/moderation-statuses.test.ts | 55 ++++++++++++++ 11 files changed, 183 insertions(+), 27 deletions(-) create mode 100644 packages/ozone/src/db/migrations/20240201T051104136Z-mod-event-blobs.ts diff --git a/packages/ozone/src/api/admin/emitModerationEvent.ts b/packages/ozone/src/api/admin/emitModerationEvent.ts index eb1bc71a180..bc198398013 100644 --- a/packages/ozone/src/api/admin/emitModerationEvent.ts +++ b/packages/ozone/src/api/admin/emitModerationEvent.ts @@ -51,17 +51,21 @@ export default function (server: Server, ctx: AppContext) { } if (isTakedownEvent || isReverseTakedownEvent) { - const isSubjectTakendown = await moderationService.isSubjectTakendown( - subject, - ) + const status = await moderationService.getStatus(subject) - if (isSubjectTakendown && isTakedownEvent) { + if (status?.takendown && isTakedownEvent) { throw new InvalidRequestError(`Subject is already taken down`) } - if (!isSubjectTakendown && isReverseTakedownEvent) { + if (!status?.takendown && isReverseTakedownEvent) { throw new InvalidRequestError(`Subject is not taken down`) } + + if (status?.takendown && isReverseTakedownEvent && subject.isRecord()) { + // due to the way blob status is modeled, we should reverse takedown on all + // blobs for the record being restored, which aren't taken down on another record. + subject.blobCids = status.blobCids ?? [] + } } const moderationEvent = await db.transaction(async (dbTxn) => { diff --git a/packages/ozone/src/db/migrations/20240201T051104136Z-mod-event-blobs.ts b/packages/ozone/src/db/migrations/20240201T051104136Z-mod-event-blobs.ts new file mode 100644 index 00000000000..21b5e893b23 --- /dev/null +++ b/packages/ozone/src/db/migrations/20240201T051104136Z-mod-event-blobs.ts @@ -0,0 +1,15 @@ +import { Kysely } from 'kysely' + +export async function up(db: Kysely): Promise { + await db.schema + .alterTable('moderation_event') + .addColumn('subjectBlobCids', 'jsonb') + .execute() +} + +export async function down(db: Kysely): Promise { + await db.schema + .alterTable('moderation_event') + .dropColumn('subjectBlobCids') + .execute() +} diff --git a/packages/ozone/src/db/migrations/index.ts b/packages/ozone/src/db/migrations/index.ts index d00857bf575..7560c5bf482 100644 --- a/packages/ozone/src/db/migrations/index.ts +++ b/packages/ozone/src/db/migrations/index.ts @@ -4,3 +4,4 @@ export * as _20231219T205730722Z from './20231219T205730722Z-init' export * as _20240116T085607200Z from './20240116T085607200Z-communication-template' +export * as _20240201T051104136Z from './20240201T051104136Z-mod-event-blobs' diff --git a/packages/ozone/src/db/schema/moderation_event.ts b/packages/ozone/src/db/schema/moderation_event.ts index 0cf7d07c1e5..1692a3546fb 100644 --- a/packages/ozone/src/db/schema/moderation_event.ts +++ b/packages/ozone/src/db/schema/moderation_event.ts @@ -19,6 +19,7 @@ export interface ModerationEvent { subjectDid: string subjectUri: string | null subjectCid: string | null + subjectBlobCids: string[] | null createLabelVals: string | null negateLabelVals: string | null comment: string | null diff --git a/packages/ozone/src/db/types.ts b/packages/ozone/src/db/types.ts index c38271ee119..b7a2b9bb8ec 100644 --- a/packages/ozone/src/db/types.ts +++ b/packages/ozone/src/db/types.ts @@ -1,5 +1,5 @@ import { Pool as PgPool } from 'pg' -import { DynamicModule, RawBuilder, SelectQueryBuilder } from 'kysely' +import { DynamicModule, RawBuilder, SelectQueryBuilder, sql } from 'kysely' export type DbRef = RawBuilder | ReturnType @@ -13,3 +13,8 @@ export type PgOptions = { poolMaxUses?: number poolIdleTimeoutMs?: number } + +export const jsonb = (val: T) => { + if (val === null) return sql`null` + return sql`${JSON.stringify(val)}::jsonb` +} diff --git a/packages/ozone/src/mod-service/index.ts b/packages/ozone/src/mod-service/index.ts index 0ce130bf8d2..21c930a4f74 100644 --- a/packages/ozone/src/mod-service/index.ts +++ b/packages/ozone/src/mod-service/index.ts @@ -41,6 +41,7 @@ import { import { BlobPushEvent } from '../db/schema/blob_push_event' import { BackgroundQueue } from '../background' import { EventPusher } from '../daemon' +import { jsonb } from '../db/types' export type ModerationServiceCreator = (db: Database) => ModerationService @@ -223,6 +224,8 @@ export class ModerationService { meta.subjectLine = event.subjectLine } + const subjectInfo = subject.info() + const modEvent = await this.db.db .insertInto('moderation_event') .values({ @@ -241,7 +244,11 @@ export class ModerationService { event.durationInHours ? addHoursToDate(event.durationInHours, createdAt).toISOString() : undefined, - ...subject.info(), + subjectType: subjectInfo.subjectType, + subjectDid: subjectInfo.subjectDid, + subjectUri: subjectInfo.subjectUri, + subjectCid: subjectInfo.subjectCid, + subjectBlobCids: jsonb(subjectInfo.subjectBlobCids), }) .returningAll() .executeTakeFirstOrThrow() @@ -713,15 +720,16 @@ export class ModerationService { } } - async isSubjectTakendown(subject: ModSubject): Promise { - const builder = this.db.db + async getStatus( + subject: ModSubject, + ): Promise { + const result = await this.db.db .selectFrom('moderation_subject_status') .where('did', '=', subject.did) - .where('recordPath', '=', subject.recordPath || '') - - const result = await builder.select('takendown').executeTakeFirst() - - return !!result?.takendown + .where('recordPath', '=', subject.recordPath ?? '') + .selectAll() + .executeTakeFirst() + return result ?? null } async formatAndCreateLabels( diff --git a/packages/ozone/src/mod-service/status.ts b/packages/ozone/src/mod-service/status.ts index 598ebe20712..359228a313d 100644 --- a/packages/ozone/src/mod-service/status.ts +++ b/packages/ozone/src/mod-service/status.ts @@ -12,6 +12,7 @@ import { ModerationEventRow, ModerationSubjectStatusRow } from './types' import { HOUR } from '@atproto/common' import { sql } from 'kysely' import { REASONAPPEAL } from '../lexicon/types/com/atproto/moderation/defs' +import { jsonb } from '../db/types' const getSubjectStatusForModerationEvent = ({ action, @@ -191,9 +192,9 @@ export const adjustModerationSubjectStatus = async ( } if (blobCids?.length) { - const newBlobCids = sql`${JSON.stringify( + const newBlobCids = jsonb( blobCids, - )}` as unknown as ModerationSubjectStatusRow['blobCids'] + ) as unknown as ModerationSubjectStatusRow['blobCids'] newStatus.blobCids = newBlobCids subjectStatus.blobCids = newBlobCids } diff --git a/packages/ozone/src/mod-service/subject.ts b/packages/ozone/src/mod-service/subject.ts index 2ea41eed42e..82c8f15f1d5 100644 --- a/packages/ozone/src/mod-service/subject.ts +++ b/packages/ozone/src/mod-service/subject.ts @@ -37,7 +37,11 @@ export const subjectFromEventRow = (row: ModerationEventRow): ModSubject => { row.subjectUri && row.subjectCid ) { - return new RecordSubject(row.subjectUri, row.subjectCid) + return new RecordSubject( + row.subjectUri, + row.subjectCid, + row.subjectBlobCids ?? [], + ) } else { return new RepoSubject(row.subjectDid) } @@ -50,7 +54,7 @@ export const subjectFromStatusRow = ( // Not too intuitive but the recordpath is basically / // which is what the last 2 params of .make() arguments are const uri = AtUri.make(row.did, ...row.recordPath.split('/')).toString() - return new RecordSubject(uri.toString(), row.recordCid) + return new RecordSubject(uri.toString(), row.recordCid, row.blobCids ?? []) } else { return new RepoSubject(row.did) } @@ -61,6 +65,7 @@ type SubjectInfo = { subjectDid: string subjectUri: string | null subjectCid: string | null + subjectBlobCids: string[] | null } export interface ModSubject { @@ -89,6 +94,7 @@ export class RepoSubject implements ModSubject { subjectDid: this.did, subjectUri: null, subjectCid: null, + subjectBlobCids: null, } } lex(): RepoRef { @@ -124,6 +130,7 @@ export class RecordSubject implements ModSubject { subjectDid: this.did, subjectUri: this.uri, subjectCid: this.cid, + subjectBlobCids: this.blobCids ?? [], } } lex(): StrongRef { diff --git a/packages/ozone/src/mod-service/views.ts b/packages/ozone/src/mod-service/views.ts index 1ae32126b8f..4c15a00218d 100644 --- a/packages/ozone/src/mod-service/views.ts +++ b/packages/ozone/src/mod-service/views.ts @@ -88,7 +88,7 @@ export class ModerationViews { comment: event.comment ?? undefined, }, subject: subjectFromEventRow(event).lex(), - subjectBlobCids: [], + subjectBlobCids: event.subjectBlobCids ?? [], createdBy: event.createdBy, createdAt: event.createdAt, subjectHandle: event.subjectHandle ?? undefined, diff --git a/packages/ozone/tests/moderation-events.test.ts b/packages/ozone/tests/moderation-events.test.ts index 73149dc06d8..53cae38e8f2 100644 --- a/packages/ozone/tests/moderation-events.test.ts +++ b/packages/ozone/tests/moderation-events.test.ts @@ -1,5 +1,9 @@ +import assert from 'node:assert' import { TestNetwork, SeedClient, basicSeed } from '@atproto/dev-env' -import AtpAgent, { ComAtprotoAdminDefs } from '@atproto/api' +import AtpAgent, { + ComAtprotoAdminDefs, + ComAtprotoAdminEmitModerationEvent, +} from '@atproto/api' import { forSnapshot } from './_util' import { REASONMISLEADING, @@ -12,7 +16,9 @@ describe('moderation-events', () => { let pdsAgent: AtpAgent let sc: SeedClient - const emitModerationEvent = async (eventData) => { + const emitModerationEvent = async ( + eventData: ComAtprotoAdminEmitModerationEvent.InputSchema, + ) => { return pdsAgent.api.com.atproto.admin.emitModerationEvent(eventData, { encoding: 'application/json', headers: network.bsky.adminAuthHeaders('moderator'), @@ -206,15 +212,68 @@ describe('moderation-events', () => { describe('get event', () => { it('gets an event by specific id', async () => { const { data } = await pdsAgent.api.com.atproto.admin.getModerationEvent( - { - id: 1, + { id: 1 }, + { headers: network.bsky.adminAuthHeaders('moderator') }, + ) + expect(forSnapshot(data)).toMatchSnapshot() + }) + }) + + describe('blobs', () => { + it('are tracked on takedown event', async () => { + const post = sc.posts[sc.dids.carol][0] + assert(post.images.length > 1) + await emitModerationEvent({ + event: { + $type: 'com.atproto.admin.defs#modEventTakedown', }, - { - headers: network.bsky.adminAuthHeaders('moderator'), + subject: { + $type: 'com.atproto.repo.strongRef', + uri: post.ref.uriStr, + cid: post.ref.cidStr, }, - ) + subjectBlobCids: [post.images[0].image.ref.toString()], + createdBy: sc.dids.alice, + }) + const { data: result } = + await pdsAgent.api.com.atproto.admin.queryModerationEvents( + { subject: post.ref.uriStr }, + { headers: network.ozone.adminAuthHeaders('moderator') }, + ) + expect(result.events[0]).toMatchObject({ + createdBy: sc.dids.alice, + event: { + $type: 'com.atproto.admin.defs#modEventTakedown', + }, + subjectBlobCids: [post.images[0].image.ref.toString()], + }) + }) - expect(forSnapshot(data)).toMatchSnapshot() + it("are tracked on reverse-takedown event even if they aren't specified", async () => { + const post = sc.posts[sc.dids.carol][0] + await emitModerationEvent({ + event: { + $type: 'com.atproto.admin.defs#modEventReverseTakedown', + }, + subject: { + $type: 'com.atproto.repo.strongRef', + uri: post.ref.uriStr, + cid: post.ref.cidStr, + }, + createdBy: sc.dids.alice, + }) + const { data: result } = + await pdsAgent.api.com.atproto.admin.queryModerationEvents( + { subject: post.ref.uriStr }, + { headers: network.ozone.adminAuthHeaders('moderator') }, + ) + expect(result.events[0]).toMatchObject({ + createdBy: sc.dids.alice, + event: { + $type: 'com.atproto.admin.defs#modEventReverseTakedown', + }, + subjectBlobCids: [post.images[0].image.ref.toString()], + }) }) }) }) diff --git a/packages/ozone/tests/moderation-statuses.test.ts b/packages/ozone/tests/moderation-statuses.test.ts index 5f63dbfe9a4..b4a2ed110a3 100644 --- a/packages/ozone/tests/moderation-statuses.test.ts +++ b/packages/ozone/tests/moderation-statuses.test.ts @@ -1,3 +1,4 @@ +import assert from 'node:assert' import { TestNetwork, SeedClient, basicSeed } from '@atproto/dev-env' import AtpAgent, { ComAtprotoAdminDefs, @@ -141,4 +142,58 @@ describe('moderation-statuses', () => { expect(listReviewedFirst.length).toEqual(list.length) }) }) + + describe('blobs', () => { + it('are tracked on takendown subject', async () => { + const post = sc.posts[sc.dids.carol][0] + assert(post.images.length > 1) + await emitModerationEvent({ + event: { + $type: 'com.atproto.admin.defs#modEventTakedown', + }, + subject: { + $type: 'com.atproto.repo.strongRef', + uri: post.ref.uriStr, + cid: post.ref.cidStr, + }, + subjectBlobCids: [post.images[0].image.ref.toString()], + createdBy: sc.dids.alice, + }) + const { data: result } = + await pdsAgent.api.com.atproto.admin.queryModerationStatuses( + { subject: post.ref.uriStr }, + { headers: network.ozone.adminAuthHeaders('moderator') }, + ) + expect(result.subjectStatuses.length).toBe(1) + expect(result.subjectStatuses[0]).toMatchObject({ + takendown: true, + subjectBlobCids: [post.images[0].image.ref.toString()], + }) + }) + + it('are tracked on reverse-takendown subject based on previous status', async () => { + const post = sc.posts[sc.dids.carol][0] + await emitModerationEvent({ + event: { + $type: 'com.atproto.admin.defs#modEventReverseTakedown', + }, + subject: { + $type: 'com.atproto.repo.strongRef', + uri: post.ref.uriStr, + cid: post.ref.cidStr, + }, + createdBy: sc.dids.alice, + }) + const { data: result } = + await pdsAgent.api.com.atproto.admin.queryModerationStatuses( + { subject: post.ref.uriStr }, + { headers: network.ozone.adminAuthHeaders('moderator') }, + ) + expect(result.subjectStatuses.length).toBe(1) + expect(result.subjectStatuses[0]).toMatchObject({ + takendown: false, + subjectBlobCids: [post.images[0].image.ref.toString()], + }) + }) + }) })