Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin' into eric/fix-1197-reposted-rep…
Browse files Browse the repository at this point in the history
…lies

* origin:
  Increase CI test matrix size (bluesky-social#1490)
  Fix condition for viewing soft-deleted followers (bluesky-social#1485)
  ✨ Expose takendown profile, their follows and followers to mods (bluesky-social#1456)
  Handle revalidation (bluesky-social#1474)
  Handle db pool errors on appview (bluesky-social#1483)
  Handle db client errors on appview (bluesky-social#1481)
  • Loading branch information
estrattonbailey committed Aug 17, 2023
2 parents 1c50428 + 2413f48 commit 9082a9a
Show file tree
Hide file tree
Showing 24 changed files with 303 additions and 103 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/repo.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
test:
strategy:
matrix:
shard: [1/4, 2/4, 3/4, 4/4]
shard: [1/8, 2/8, 3/8, 4/8, 5/8, 6/8, 7/8, 8/8]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
Expand All @@ -30,7 +30,7 @@ jobs:
node-version: 18
cache: "yarn"
- run: yarn install --frozen-lockfile
- run: yarn test:withFlags --maxWorkers=2 --shard=${{ matrix.shard }} --passWithNoTests
- run: yarn test:withFlags --maxWorkers=1 --shard=${{ matrix.shard }} --passWithNoTests
verify:
runs-on: ubuntu-latest
steps:
Expand Down
9 changes: 6 additions & 3 deletions packages/bsky/src/api/app/bsky/actor/getProfile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import { setRepoRev } from '../../../util'

export default function (server: Server, ctx: AppContext) {
server.app.bsky.actor.getProfile({
auth: ctx.authOptionalVerifier,
auth: ctx.authOptionalAccessOrRoleVerifier,
handler: async ({ auth, params, res }) => {
const { actor } = params
const requester = auth.credentials.did
const requester = 'did' in auth.credentials ? auth.credentials.did : null
const canViewTakendownProfile =
auth.credentials.type === 'role' && auth.credentials.triage
const db = ctx.db.getReplica()
const actorService = ctx.services.actor(db)

Expand All @@ -22,7 +24,7 @@ export default function (server: Server, ctx: AppContext) {
if (!actorRes) {
throw new InvalidRequestError('Profile not found')
}
if (softDeleted(actorRes)) {
if (!canViewTakendownProfile && softDeleted(actorRes)) {
throw new InvalidRequestError(
'Account has been taken down',
'AccountTakedown',
Expand All @@ -31,6 +33,7 @@ export default function (server: Server, ctx: AppContext) {
const profile = await actorService.views.profileDetailed(
actorRes,
requester,
{ includeSoftDeleted: canViewTakendownProfile },
)
if (!profile) {
throw new InvalidRequestError('Profile not found')
Expand Down
23 changes: 17 additions & 6 deletions packages/bsky/src/api/app/bsky/graph/getFollowers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,22 @@ import { notSoftDeletedClause } from '../../../../db/util'

export default function (server: Server, ctx: AppContext) {
server.app.bsky.graph.getFollowers({
auth: ctx.authOptionalVerifier,
auth: ctx.authOptionalAccessOrRoleVerifier,
handler: async ({ params, auth }) => {
const { actor, limit, cursor } = params
const requester = auth.credentials.did
const requester = 'did' in auth.credentials ? auth.credentials.did : null
const canViewTakendownProfile =
auth.credentials.type === 'role' && auth.credentials.triage
const db = ctx.db.getReplica()
const { ref } = db.db.dynamic

const actorService = ctx.services.actor(db)
const graphService = ctx.services.graph(db)

const subjectRes = await actorService.getActor(actor)
const subjectRes = await actorService.getActor(
actor,
canViewTakendownProfile,
)
if (!subjectRes) {
throw new InvalidRequestError(`Actor not found: ${actor}`)
}
Expand All @@ -25,7 +30,9 @@ export default function (server: Server, ctx: AppContext) {
.selectFrom('follow')
.where('follow.subjectDid', '=', subjectRes.did)
.innerJoin('actor as creator', 'creator.did', 'follow.creator')
.where(notSoftDeletedClause(ref('creator')))
.if(!canViewTakendownProfile, (qb) =>
qb.where(notSoftDeletedClause(ref('creator'))),
)
.whereNotExists(
graphService.blockQb(requester, [ref('follow.creator')]),
)
Expand All @@ -47,8 +54,12 @@ export default function (server: Server, ctx: AppContext) {

const followersRes = await followersReq.execute()
const [followers, subject] = await Promise.all([
actorService.views.hydrateProfiles(followersRes, requester),
actorService.views.profile(subjectRes, requester),
actorService.views.hydrateProfiles(followersRes, requester, {
includeSoftDeleted: canViewTakendownProfile,
}),
actorService.views.profile(subjectRes, requester, {
includeSoftDeleted: canViewTakendownProfile,
}),
])
if (!subject) {
throw new InvalidRequestError(`Actor not found: ${actor}`)
Expand Down
23 changes: 17 additions & 6 deletions packages/bsky/src/api/app/bsky/graph/getFollows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,22 @@ import { notSoftDeletedClause } from '../../../../db/util'

export default function (server: Server, ctx: AppContext) {
server.app.bsky.graph.getFollows({
auth: ctx.authOptionalVerifier,
auth: ctx.authOptionalAccessOrRoleVerifier,
handler: async ({ params, auth }) => {
const { actor, limit, cursor } = params
const requester = auth.credentials.did
const requester = 'did' in auth.credentials ? auth.credentials.did : null
const canViewTakendownProfile =
auth.credentials.type === 'role' && auth.credentials.triage
const db = ctx.db.getReplica()
const { ref } = db.db.dynamic

const actorService = ctx.services.actor(db)
const graphService = ctx.services.graph(db)

const creatorRes = await actorService.getActor(actor)
const creatorRes = await actorService.getActor(
actor,
canViewTakendownProfile,
)
if (!creatorRes) {
throw new InvalidRequestError(`Actor not found: ${actor}`)
}
Expand All @@ -25,7 +30,9 @@ export default function (server: Server, ctx: AppContext) {
.selectFrom('follow')
.where('follow.creator', '=', creatorRes.did)
.innerJoin('actor as subject', 'subject.did', 'follow.subjectDid')
.where(notSoftDeletedClause(ref('subject')))
.if(!canViewTakendownProfile, (qb) =>
qb.where(notSoftDeletedClause(ref('subject'))),
)
.whereNotExists(
graphService.blockQb(requester, [ref('follow.subjectDid')]),
)
Expand All @@ -47,8 +54,12 @@ export default function (server: Server, ctx: AppContext) {

const followsRes = await followsReq.execute()
const [follows, subject] = await Promise.all([
actorService.views.hydrateProfiles(followsRes, requester),
actorService.views.profile(creatorRes, requester),
actorService.views.hydrateProfiles(followsRes, requester, {
includeSoftDeleted: canViewTakendownProfile,
}),
actorService.views.profile(creatorRes, requester, {
includeSoftDeleted: canViewTakendownProfile,
}),
])
if (!subject) {
throw new InvalidRequestError(`Actor not found: ${actor}`)
Expand Down
36 changes: 36 additions & 0 deletions packages/bsky/src/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,42 @@ export const authOptionalVerifier =
return authVerifier(idResolver, opts)(reqCtx)
}

export const authOptionalAccessOrRoleVerifier = (
idResolver: IdResolver,
cfg: ServerConfig,
) => {
const verifyAccess = authVerifier(idResolver, { aud: cfg.serverDid })
const verifyRole = roleVerifier(cfg)
return async (ctx: { req: express.Request; res: express.Response }) => {
const defaultUnAuthorizedCredentials = {
credentials: { did: null, type: 'unauthed' as const },
}
if (!ctx.req.headers.authorization) {
return defaultUnAuthorizedCredentials
}
// For non-admin tokens, we don't want to consider alternative verifiers and let it fail if it fails
const isRoleAuthToken = ctx.req.headers.authorization?.startsWith(BASIC)
if (isRoleAuthToken) {
const result = await verifyRole(ctx)
return {
...result,
credentials: {
type: 'role' as const,
...result.credentials,
},
}
}
const result = await verifyAccess(ctx)
return {
...result,
credentials: {
type: 'access' as const,
...result.credentials,
},
}
}
}

export const roleVerifier =
(cfg: ServerConfig) =>
async (reqCtx: { req: express.Request; res: express.Response }) => {
Expand Down
11 changes: 10 additions & 1 deletion packages/bsky/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export interface ServerConfigValues {
didPlcUrl: string
didCacheStaleTTL: number
didCacheMaxTTL: number
handleResolveNameservers?: string[]
imgUriEndpoint?: string
blobCacheLocation?: string
labelerDid: string
Expand Down Expand Up @@ -45,6 +46,9 @@ export class ServerConfig {
process.env.DID_CACHE_MAX_TTL,
DAY,
)
const handleResolveNameservers = process.env.HANDLE_RESOLVE_NAMESERVERS
? process.env.HANDLE_RESOLVE_NAMESERVERS.split(',')
: []
const imgUriEndpoint = process.env.IMG_URI_ENDPOINT
const blobCacheLocation = process.env.BLOB_CACHE_LOC
const dbPrimaryPostgresUrl =
Expand Down Expand Up @@ -90,6 +94,7 @@ export class ServerConfig {
didPlcUrl,
didCacheStaleTTL,
didCacheMaxTTL,
handleResolveNameservers,
imgUriEndpoint,
blobCacheLocation,
labelerDid,
Expand Down Expand Up @@ -159,7 +164,11 @@ export class ServerConfig {
}

get didCacheMaxTTL() {
return this.cfg.didCacheStaleTTL
return this.cfg.didCacheMaxTTL
}

get handleResolveNameservers() {
return this.cfg.handleResolveNameservers
}

get didPlcUrl() {
Expand Down
4 changes: 4 additions & 0 deletions packages/bsky/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ export class AppContext {
})
}

get authOptionalAccessOrRoleVerifier() {
return auth.authOptionalAccessOrRoleVerifier(this.idResolver, this.cfg)
}

get roleVerifier() {
return auth.roleVerifier(this.cfg)
}
Expand Down
6 changes: 6 additions & 0 deletions packages/bsky/src/db/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Kysely, PostgresDialect } from 'kysely'
import { Pool as PgPool, types as pgTypes } from 'pg'
import DatabaseSchema, { DatabaseSchemaType } from './database-schema'
import { PgOptions } from './types'
import { dbLogger } from '../logger'

export class Database {
pool: PgPool
Expand Down Expand Up @@ -40,7 +41,9 @@ export class Database {
throw new Error(`Postgres schema must only contain [A-Za-z_]: ${schema}`)
}

pool.on('error', onPoolError)
pool.on('connect', (client) => {
client.on('error', onClientError)
// Used for trigram indexes, e.g. on actor search
client.query('SET pg_trgm.word_similarity_threshold TO .4;')
if (schema) {
Expand Down Expand Up @@ -83,3 +86,6 @@ export class Database {
}

export default Database

const onPoolError = (err: Error) => dbLogger.error({ err }, 'db pool error')
const onClientError = (err: Error) => dbLogger.error({ err }, 'db client error')
6 changes: 5 additions & 1 deletion packages/bsky/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,11 @@ export class BskyAppView {
config.didCacheStaleTTL,
config.didCacheMaxTTL,
)
const idResolver = new IdResolver({ plcUrl: config.didPlcUrl, didCache })
const idResolver = new IdResolver({
plcUrl: config.didPlcUrl,
didCache,
backupNameservers: config.handleResolveNameservers,
})

const imgUriBuilder = new ImageUriBuilder(
config.imgUriEndpoint || `${config.publicUrl}/img`,
Expand Down
9 changes: 9 additions & 0 deletions packages/bsky/src/indexer/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export interface IndexerConfigValues {
didPlcUrl: string
didCacheStaleTTL: number
didCacheMaxTTL: number
handleResolveNameservers?: string[]
labelerDid: string
hiveApiKey?: string
labelerKeywords: Record<string, string>
Expand Down Expand Up @@ -54,6 +55,9 @@ export class IndexerConfig {
process.env.DID_CACHE_MAX_TTL,
DAY,
)
const handleResolveNameservers = process.env.HANDLE_RESOLVE_NAMESERVERS
? process.env.HANDLE_RESOLVE_NAMESERVERS.split(',')
: []
const labelerDid = process.env.LABELER_DID || 'did:example:labeler'
const labelerPushUrl =
overrides?.labelerPushUrl || process.env.LABELER_PUSH_URL || undefined
Expand Down Expand Up @@ -86,6 +90,7 @@ export class IndexerConfig {
didPlcUrl,
didCacheStaleTTL,
didCacheMaxTTL,
handleResolveNameservers,
labelerDid,
labelerPushUrl,
hiveApiKey,
Expand Down Expand Up @@ -139,6 +144,10 @@ export class IndexerConfig {
return this.cfg.didCacheMaxTTL
}

get handleResolveNameservers() {
return this.cfg.handleResolveNameservers
}

get labelerDid() {
return this.cfg.labelerDid
}
Expand Down
6 changes: 5 additions & 1 deletion packages/bsky/src/indexer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ export class BskyIndexer {
cfg.didCacheStaleTTL,
cfg.didCacheMaxTTL,
)
const idResolver = new IdResolver({ plcUrl: cfg.didPlcUrl, didCache })
const idResolver = new IdResolver({
plcUrl: cfg.didPlcUrl,
didCache,
backupNameservers: cfg.handleResolveNameservers,
})
const backgroundQueue = new BackgroundQueue(db)
let labeler: Labeler
if (cfg.hiveApiKey) {
Expand Down
22 changes: 14 additions & 8 deletions packages/bsky/src/services/indexing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
} from '@atproto/repo'
import { AtUri } from '@atproto/uri'
import { IdResolver, getPds } from '@atproto/identity'
import { DAY, chunkArray } from '@atproto/common'
import { DAY, HOUR, chunkArray } from '@atproto/common'
import { ValidationError } from '@atproto/lexicon'
import { PrimaryDatabase } from '../../db'
import * as Post from './plugins/post'
Expand All @@ -28,6 +28,7 @@ import { subLogger } from '../../logger'
import { retryHttp } from '../../util/retry'
import { Labeler } from '../../labeler'
import { BackgroundQueue } from '../../background'
import { Actor } from '../../db/tables/actor'

export class IndexingService {
records: {
Expand Down Expand Up @@ -118,13 +119,7 @@ export class IndexingService {
.where('did', '=', did)
.selectAll()
.executeTakeFirst()
const timestampAt = new Date(timestamp)
const lastIndexedAt = actor && new Date(actor.indexedAt)
const needsReindex =
force ||
!lastIndexedAt ||
timestampAt.getTime() - lastIndexedAt.getTime() > DAY
if (!needsReindex) {
if (!force && !needsHandleReindex(actor, timestamp)) {
return
}
const atpData = await this.idResolver.did.resolveAtprotoData(did, true)
Expand Down Expand Up @@ -357,3 +352,14 @@ function* walkContentsWithCids(contents: RepoContentsWithCids) {
}
}
}

const needsHandleReindex = (actor: Actor | undefined, timestamp: string) => {
if (!actor) return true
const timeDiff =
new Date(timestamp).getTime() - new Date(actor.indexedAt).getTime()
// revalidate daily
if (timeDiff > DAY) return true
// revalidate more aggressively for invalidated handles
if (actor.handle === null && timeDiff > HOUR) return true
return false
}
Loading

0 comments on commit 9082a9a

Please sign in to comment.