Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Appview: ensure takedowns on modlist authors always apply #3192

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions packages/bsky/src/api/app/bsky/feed/getAuthorFeed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,19 @@ const noBlocksOrMutedReposts = (inputs: {
}): Skeleton => {
const { ctx, skeleton, hydration } = inputs
const relationship = hydration.profileViewers?.get(skeleton.actor.did)
if (relationship?.blocking || relationship?.blockingByList) {
if (
relationship &&
(relationship.blocking || ctx.views.blockingByList(relationship, hydration))
) {
throw new InvalidRequestError(
`Requester has blocked actor: ${skeleton.actor.did}`,
'BlockedActor',
)
}
if (relationship?.blockedBy || relationship?.blockedByList) {
if (
relationship &&
(relationship.blockedBy || ctx.views.blockedByList(relationship, hydration))
) {
throw new InvalidRequestError(
`Requester is blocked by actor: ${skeleton.actor.did}`,
'BlockedByActor',
Expand Down
104 changes: 64 additions & 40 deletions packages/bsky/src/data-plane/server/routes/relationships.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export default (db: Database): Partial<ServiceImpl<typeof Service>> => ({
async getBlockExistence(req) {
const { pairs } = req
if (pairs.length === 0) {
return { exists: [] }
return { exists: [], blocks: [] }
}
const { ref } = db.db.dynamic
const sourceRef = ref('pair.source')
Expand All @@ -101,48 +101,72 @@ export default (db: Database): Partial<ServiceImpl<typeof Service>> => ({
.select([
sql<string>`${sourceRef}`.as('source'),
sql<string>`${targetRef}`.as('target'),
(eb) =>
eb
.selectFrom('actor_block')
.whereRef('actor_block.creator', '=', sourceRef)
.whereRef('actor_block.subjectDid', '=', targetRef)
.select('uri')
.as('blocking'),
(eb) =>
eb
.selectFrom('actor_block')
.whereRef('actor_block.creator', '=', targetRef)
.whereRef('actor_block.subjectDid', '=', sourceRef)
.select('uri')
.as('blockedBy'),
(eb) =>
eb
.selectFrom('list_item')
.innerJoin(
'list_block',
'list_block.subjectUri',
'list_item.listUri',
)
.whereRef('list_block.creator', '=', sourceRef)
.whereRef('list_item.subjectDid', '=', targetRef)
.select('list_item.listUri')
.as('blockingByList'),
(eb) =>
eb
.selectFrom('list_item')
.innerJoin(
'list_block',
'list_block.subjectUri',
'list_item.listUri',
)
.whereRef('list_block.creator', '=', targetRef)
.whereRef('list_item.subjectDid', '=', sourceRef)
.select('list_item.listUri')
.as('blockedByList'),
])
.whereExists((qb) =>
qb
.selectFrom('actor_block')
.whereRef('actor_block.creator', '=', sourceRef)
.whereRef('actor_block.subjectDid', '=', targetRef)
.select('uri'),
)
.orWhereExists((qb) =>
qb
.selectFrom('actor_block')
.whereRef('actor_block.creator', '=', targetRef)
.whereRef('actor_block.subjectDid', '=', sourceRef)
.select('uri'),
)
.orWhereExists((qb) =>
qb
.selectFrom('list_item')
.innerJoin('list_block', 'list_block.subjectUri', 'list_item.listUri')
.whereRef('list_block.creator', '=', sourceRef)
.whereRef('list_item.subjectDid', '=', targetRef)
.select('list_item.listUri'),
)
.orWhereExists((qb) =>
qb
.selectFrom('list_item')
.innerJoin('list_block', 'list_block.subjectUri', 'list_item.listUri')
.whereRef('list_block.creator', '=', targetRef)
.whereRef('list_item.subjectDid', '=', sourceRef)
.select('list_item.listUri'),
)
.execute()
const existMap = res.reduce((acc, cur) => {
const key = [cur.source, cur.target].sort().join(',')
return acc.set(key, true)
}, new Map<string, boolean>())
const exists = pairs.map((pair) => {
const key = [pair.a, pair.b].sort().join(',')
return existMap.get(key) === true
})
const getKey = (a, b) => [a, b].sort().join(',')
const lookup = res.reduce((acc, cur) => {
const key = getKey(cur.source, cur.target)
return acc.set(key, cur)
}, new Map<string, (typeof res)[0]>())
return {
exists,
exists: pairs.map((pair) => {
const item = lookup.get(getKey(pair.a, pair.b))
if (!item) return false
return !!(
item.blocking ||
item.blockedBy ||
item.blockingByList ||
item.blockedByList
)
}),
blocks: pairs.map((pair) => {
const item = lookup.get(getKey(pair.a, pair.b))
if (!item) return {}
return {
blockedBy: item.blockedBy || undefined,
blocking: item.blocking || undefined,
blockedByList: item.blockedByList || undefined,
blockingByList: item.blockingByList || undefined,
}
}),
}
},
})
46 changes: 23 additions & 23 deletions packages/bsky/src/hydration/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,50 +46,46 @@ export type ListAggs = HydrationMap<ListAgg>
export type RelationshipPair = [didA: string, didB: string]

const dedupePairs = (pairs: RelationshipPair[]): RelationshipPair[] => {
const mapped = pairs.reduce(
(acc, cur) => {
const sorted = ([...cur] as RelationshipPair).sort()
acc[sorted.join('-')] = sorted
return acc
},
{} as Record<string, RelationshipPair>,
)
return Object.values(mapped)
const deduped = pairs.reduce((acc, pair) => {
return acc.set(Blocks.key(...pair), pair)
}, new Map<string, RelationshipPair>())
return [...deduped.values()]
}

export class Blocks {
_blocks: Map<string, boolean> = new Map()
_blocks: Map<string, BlockEntry> = new Map() // did:a,did:b -> block
constructor() {}

static key(didA: string, didB: string): string {
return [didA, didB].sort().join(',')
}

set(didA: string, didB: string, exists: boolean): Blocks {
set(didA: string, didB: string, block: BlockEntry): Blocks {
const key = Blocks.key(didA, didB)
this._blocks.set(key, exists)
this._blocks.set(key, block)
return this
}

has(didA: string, didB: string): boolean {
get(didA: string, didB: string): BlockEntry | null {
if (didA === didB) return null // ignore self-blocks
const key = Blocks.key(didA, didB)
return this._blocks.has(key)
}

isBlocked(didA: string, didB: string): boolean {
if (didA === didB) return false // ignore self-blocks
const key = Blocks.key(didA, didB)
return this._blocks.get(key) ?? false
return this._blocks.get(key) ?? null
}

merge(blocks: Blocks): Blocks {
blocks._blocks.forEach((exists, key) => {
this._blocks.set(key, exists)
blocks._blocks.forEach((block, key) => {
this._blocks.set(key, block)
})
return this
}
}

// No "blocking" vs. "blocked" directionality: only suitable for bidirectional block checks
export type BlockEntry = {
blockUri: string | undefined
rafaelbsky marked this conversation as resolved.
Show resolved Hide resolved
blockListUri: string | undefined
}

export class GraphHydrator {
constructor(public dataplane: DataPlaneClient) {}

Expand Down Expand Up @@ -162,7 +158,11 @@ export class GraphHydrator {
const blocks = new Blocks()
for (let i = 0; i < deduped.length; i++) {
const pair = deduped[i]
blocks.set(pair.a, pair.b, res.exists[i] ?? false)
const block = res.blocks[i]
blocks.set(pair.a, pair.b, {
blockUri: block.blockedBy || block.blocking || undefined,
blockListUri: block.blockedByList || block.blockingByList || undefined,
})
Comment on lines +161 to +165
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May consider defensively assuming block might not exist, even though it should. Could log a warn or error in that case.

}
return blocks
}
Expand Down
Loading
Loading