Skip to content

Commit 8035df7

Browse files
committed
Merge branch 'main' into settings-ui
# Conflicts: # src/controllers/admin/get-settings-controller.ts # src/controllers/admin/patch-settings-controller.ts # src/routes/admin/index.ts # src/schemas/admin-settings-schema.ts # src/utils/settings-config.ts # test/unit/routes/admin-settings.spec.ts
2 parents 1e37e30 + f70adf2 commit 8035df7

8 files changed

Lines changed: 117 additions & 28 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"nostream": patch
3+
---
4+
5+
fix: de-duplicate events returned by generic tag-filter subscriptions
6+
7+
`EventRepository.findByFilters()` left-joins `event_tags` for generic tag filters
8+
(`#e`, `#p`, etc.) without deduplicating the result. An event matching more than one
9+
tag row for the same filter (e.g. `{"#p": ["a", "b"]}` matching an event tagged with
10+
both) was returned once per matching `event_tags` row, so subscribers received the
11+
same `EVENT` message multiple times. The query now selects `DISTINCT events.*` for
12+
tag-filtered queries so each stored event is returned at most once. This also covers
13+
generic tag filters combined with a NIP-50 `search` term (e.g.
14+
`{"search": "...", "#p": ["a", "b"]}`), which take the search branch and are now
15+
de-duplicated as well.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"nostream": patch
3+
---
4+
5+
fix: include the actual error message in replaceable event rejection responses
6+
7+
`ReplaceableEventStrategy.execute()` sent clients a bare `error: ` command result
8+
(with no message body) whenever `eventRepository.upsert()` failed for a reason other
9+
than a duplicate event id. The underlying `error.message` was caught but never
10+
included in the response, leaving clients with no actionable information about why
11+
the event was rejected. The command result now includes `error.message`.

src/controllers/admin/get-settings-controller.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
import { Request, Response } from 'express'
22

33
import { IController } from '../../@types/controllers'
4-
import { loadMergedSettings } from '../../utils/settings-config'
4+
import { Settings } from '../../@types/settings'
5+
import { loadDefaults, loadMergedSettings, filterSettingsAgainstDefaults } from '../../utils/settings-config'
56
import { redactSettingsSecrets } from '../../utils/settings-redaction'
67

78
export class GetAdminSettingsController implements IController {
89
public async handleRequest(_request: Request, response: Response): Promise<void> {
9-
const settings = redactSettingsSecrets(loadMergedSettings())
10+
const merged = loadMergedSettings()
11+
const defaults = loadDefaults()
12+
const filtered = filterSettingsAgainstDefaults(merged, defaults) as Settings
13+
const settings = redactSettingsSecrets(filtered)
1014

1115
response.status(200).setHeader('content-type', 'application/json').send({ settings })
1216
}

src/handlers/event-strategies/replaceable-event-strategy.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ export class ReplaceableEventStrategy implements IEventStrategy<Event, Promise<v
3232
return
3333
}
3434

35-
this.webSocket.emit(WebSocketAdapterEvent.Message, createEventCommandResult(event.id, false, 'error: '))
35+
this.webSocket.emit(
36+
WebSocketAdapterEvent.Message,
37+
createEventCommandResult(event.id, false, `error: ${error.message}`),
38+
)
3639
}
3740
}
3841
}

src/repositories/event-repository.ts

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -73,24 +73,25 @@ export class EventRepository implements IEventRepository {
7373
const maxLen = nip50Settings?.nip50?.maxQueryLength ?? DEFAULT_MAX_SEARCH_QUERY_LENGTH
7474
const searchQuery = currentFilter.search.trim().slice(0, maxLen)
7575
const limit = typeof currentFilter.limit === 'number' ? currentFilter.limit : DEFAULT_FILTER_LIMIT
76-
builder
77-
.select(
78-
this.readReplicaDbClient.raw(
79-
'events.*, ts_rank(to_tsvector(?::regconfig, event_content), plainto_tsquery(?::regconfig, ?)) AS search_rank',
80-
[tsConfig, tsConfig, searchQuery],
81-
),
82-
)
83-
.limit(limit)
84-
.orderBy('search_rank', 'DESC')
85-
.orderBy('event_id', 'asc')
76+
const searchSelection = this.readReplicaDbClient.raw(
77+
'events.*, ts_rank(to_tsvector(?::regconfig, event_content), plainto_tsquery(?::regconfig, ?)) AS search_rank',
78+
[tsConfig, tsConfig, searchQuery],
79+
)
80+
// De-duplicate rows multiplied by the event_tags left join when search is combined with a generic tag filter
81+
if (isTagQuery) {
82+
builder.distinct(searchSelection)
83+
} else {
84+
builder.select(searchSelection)
85+
}
86+
builder.limit(limit).orderBy('search_rank', 'DESC').orderBy('event_id', 'asc')
8687
} else if (typeof currentFilter.limit === 'number') {
8788
builder.limit(currentFilter.limit).orderBy('event_created_at', 'DESC').orderBy('event_id', 'asc')
8889
} else {
8990
builder.limit(DEFAULT_FILTER_LIMIT).orderBy('event_created_at', 'asc').orderBy('event_id', 'asc')
9091
}
9192

9293
if (isTagQuery && !isSearchQuery) {
93-
builder.select('events.*')
94+
builder.distinct('events.*')
9495
}
9596

9697
return builder

src/utils/settings-config.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,10 @@ const pathExistsInSchema = (schema: unknown, tokens: PathToken[]): boolean => {
270270
return false
271271
}
272272

273+
if (current.length === 0) {
274+
return true
275+
}
276+
273277
current = current[0]
274278
}
275279

@@ -305,6 +309,45 @@ export const loadMergedSettings = (): Settings => {
305309
return mergeDeepRight(loadDefaults(), loadUserSettings()) as Settings
306310
}
307311

312+
export const filterSettingsAgainstDefaults = (settings: unknown, defaults: unknown): unknown => {
313+
if (Array.isArray(settings)) {
314+
if (!Array.isArray(defaults)) {
315+
return []
316+
}
317+
if (defaults.length === 0) {
318+
return [...settings]
319+
}
320+
321+
const itemSchema = defaults[0]
322+
return settings.map((item) => filterSettingsAgainstDefaults(item, itemSchema))
323+
}
324+
325+
if (isPlainObject(settings)) {
326+
if (!isPlainObject(defaults)) {
327+
return {}
328+
}
329+
330+
const filtered: Record<string, unknown> = {}
331+
for (const key of Object.keys(settings)) {
332+
if (hasOwn(defaults, key) || key === 'passwordHash') {
333+
// If it's a known non-schema key like passwordHash, we don't have a default schema for its children.
334+
// We can pass {} to allow it to be preserved.
335+
const defaultSubSchema = hasOwn(defaults, key)
336+
? (defaults as Record<string, unknown>)[key]
337+
: {}
338+
339+
filtered[key] = filterSettingsAgainstDefaults(
340+
(settings as Record<string, unknown>)[key],
341+
defaultSubSchema
342+
)
343+
}
344+
}
345+
return filtered
346+
}
347+
348+
return settings
349+
}
350+
308351
export const saveSettings = (settings: Settings): void => {
309352
ensureSettingsExists()
310353
const serialized = yaml.dump(toSerializable(settings), { lineWidth: 120 })

test/unit/handlers/event-strategies/replaceable-event-strategy.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ describe('ReplaceableEventStrategy', () => {
8888
})
8989

9090
it('rejects if unable to upsert event', async () => {
91-
const error = new Error()
91+
const error = new Error('connection lost')
9292
eventRepositoryUpsertStub.rejects(error)
9393

9494
await strategy.execute(event)
@@ -98,7 +98,7 @@ describe('ReplaceableEventStrategy', () => {
9898
'OK',
9999
'id',
100100
false,
101-
'error: ',
101+
'error: connection lost',
102102
])
103103
})
104104
})

test/unit/repositories/event-repository.spec.ts

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ describe('EventRepository', () => {
295295
const query = repository.findByFilters(filters).toString()
296296

297297
expect(query).to.equal(
298-
'select "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (1 = 0) order by "event_created_at" asc, "event_id" asc limit 500',
298+
'select distinct "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (1 = 0) order by "event_created_at" asc, "event_id" asc limit 500',
299299
)
300300
})
301301

@@ -305,7 +305,7 @@ describe('EventRepository', () => {
305305
const query = repository.findByFilters(filters).toString()
306306

307307
expect(query).to.equal(
308-
'select "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'e\' AND event_tags.tag_value = \'aaaaaa\') order by "event_created_at" asc, "event_id" asc limit 500',
308+
'select distinct "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'e\' AND event_tags.tag_value = \'aaaaaa\') order by "event_created_at" asc, "event_id" asc limit 500',
309309
)
310310
})
311311

@@ -315,7 +315,7 @@ describe('EventRepository', () => {
315315
const query = repository.findByFilters(filters).toString()
316316

317317
expect(query).to.equal(
318-
'select "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'e\' AND event_tags.tag_value = \'aaaaaa\' or event_tags.tag_name = \'e\' AND event_tags.tag_value = \'bbbbbb\') order by "event_created_at" asc, "event_id" asc limit 500',
318+
'select distinct "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'e\' AND event_tags.tag_value = \'aaaaaa\' or event_tags.tag_name = \'e\' AND event_tags.tag_value = \'bbbbbb\') order by "event_created_at" asc, "event_id" asc limit 500',
319319
)
320320
})
321321
})
@@ -327,7 +327,7 @@ describe('EventRepository', () => {
327327
const query = repository.findByFilters(filters).toString()
328328

329329
expect(query).to.equal(
330-
'select "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'g\' AND event_tags.tag_value LIKE \'u4pruyd%\') order by "event_created_at" asc, "event_id" asc limit 500',
330+
'select distinct "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'g\' AND event_tags.tag_value LIKE \'u4pruyd%\') order by "event_created_at" asc, "event_id" asc limit 500',
331331
)
332332
})
333333

@@ -337,7 +337,7 @@ describe('EventRepository', () => {
337337
const query = repository.findByFilters(filters).toString()
338338

339339
expect(query).to.equal(
340-
'select "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'g\' AND event_tags.tag_value = \'u4pruyd\') order by "event_created_at" asc, "event_id" asc limit 500',
340+
'select distinct "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'g\' AND event_tags.tag_value = \'u4pruyd\') order by "event_created_at" asc, "event_id" asc limit 500',
341341
)
342342
})
343343
})
@@ -349,7 +349,7 @@ describe('EventRepository', () => {
349349
const query = repository.findByFilters(filters).toString()
350350

351351
expect(query).to.equal(
352-
'select "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (1 = 0) order by "event_created_at" asc, "event_id" asc limit 500',
352+
'select distinct "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (1 = 0) order by "event_created_at" asc, "event_id" asc limit 500',
353353
)
354354
})
355355

@@ -359,7 +359,7 @@ describe('EventRepository', () => {
359359
const query = repository.findByFilters(filters).toString()
360360

361361
expect(query).to.equal(
362-
'select "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'p\' AND event_tags.tag_value = \'aaaaaa\') order by "event_created_at" asc, "event_id" asc limit 500',
362+
'select distinct "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'p\' AND event_tags.tag_value = \'aaaaaa\') order by "event_created_at" asc, "event_id" asc limit 500',
363363
)
364364
})
365365

@@ -369,7 +369,7 @@ describe('EventRepository', () => {
369369
const query = repository.findByFilters(filters).toString()
370370

371371
expect(query).to.equal(
372-
'select "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'p\' AND event_tags.tag_value = \'aaaaaa\' or event_tags.tag_name = \'p\' AND event_tags.tag_value = \'bbbbbb\') order by "event_created_at" asc, "event_id" asc limit 500',
372+
'select distinct "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'p\' AND event_tags.tag_value = \'aaaaaa\' or event_tags.tag_name = \'p\' AND event_tags.tag_value = \'bbbbbb\') order by "event_created_at" asc, "event_id" asc limit 500',
373373
)
374374
})
375375
})
@@ -381,7 +381,7 @@ describe('EventRepository', () => {
381381
const query = repository.findByFilters(filters).toString()
382382

383383
expect(query).to.equal(
384-
'select "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (1 = 0) order by "event_created_at" asc, "event_id" asc limit 500',
384+
'select distinct "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (1 = 0) order by "event_created_at" asc, "event_id" asc limit 500',
385385
)
386386
})
387387

@@ -391,7 +391,7 @@ describe('EventRepository', () => {
391391
const query = repository.findByFilters(filters).toString()
392392

393393
expect(query).to.equal(
394-
'select "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'r\' AND event_tags.tag_value = \'aaaaaa\') order by "event_created_at" asc, "event_id" asc limit 500',
394+
'select distinct "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'r\' AND event_tags.tag_value = \'aaaaaa\') order by "event_created_at" asc, "event_id" asc limit 500',
395395
)
396396
})
397397

@@ -401,7 +401,7 @@ describe('EventRepository', () => {
401401
const query = repository.findByFilters(filters).toString()
402402

403403
expect(query).to.equal(
404-
'select "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'r\' AND event_tags.tag_value = \'aaaaaa\' or event_tags.tag_name = \'r\' AND event_tags.tag_value = \'bbbbbb\') order by "event_created_at" asc, "event_id" asc limit 500',
404+
'select distinct "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'r\' AND event_tags.tag_value = \'aaaaaa\' or event_tags.tag_name = \'r\' AND event_tags.tag_value = \'bbbbbb\') order by "event_created_at" asc, "event_id" asc limit 500',
405405
)
406406
})
407407
})
@@ -413,7 +413,7 @@ describe('EventRepository', () => {
413413
const query = repository.findByFilters(filters).toString()
414414

415415
expect(query).to.equal(
416-
'select "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'d\' AND event_tags.tag_value = \'\') order by "event_created_at" asc, "event_id" asc limit 500',
416+
'select distinct "events".* from "events" left join "event_tags" on "events"."event_id" = "event_tags"."event_id" where (event_tags.tag_name = \'d\' AND event_tags.tag_value = \'\') order by "event_created_at" asc, "event_id" asc limit 500',
417417
)
418418
})
419419
})
@@ -501,6 +501,18 @@ describe('EventRepository', () => {
501501
expect(query).to.include('"event_kind" in (1)')
502502
})
503503

504+
it('de-duplicates results when search is combined with a generic tag filter', () => {
505+
const filters = [{ search: 'bitcoin', '#p': ['a', 'b'] }]
506+
507+
const query = searchEnabledRepository.findByFilters(filters).toString()
508+
509+
expect(query).to.include('select distinct events.*')
510+
expect(query).to.include('ts_rank(')
511+
expect(query).to.include('left join "event_tags" on "events"."event_id" = "event_tags"."event_id"')
512+
expect(query).to.include("plainto_tsquery('simple'::regconfig, 'bitcoin')")
513+
expect(query).to.include("event_tags.tag_name = 'p'")
514+
})
515+
504516
it('ignores search filter when NIP-50 is disabled', () => {
505517
const disabledRepository = new EventRepository(dbClient, rrDbClient, () => ({
506518
nip50: { enabled: false },

0 commit comments

Comments
 (0)