Compare commits

..

1 Commits

Author SHA1 Message Date
d949296104 fix: add source to session on cred update
When updateSourceCredentials was called for a source not yet in the
active session (e.g. because credentials were missing at config time),
the source was never instantiated despite being enabled in the DB.

Now, if the source row is enabled but absent from the session, the
source is added instead of skipped.

Co-authored-by: Ona <no-reply@ona.com>
2026-04-12 11:40:56 +00:00
2 changed files with 36 additions and 93 deletions

View File

@@ -44,15 +44,6 @@ function getEnabledSourceIds(userId: string): string[] {
*/
let mockFindResult: unknown | undefined
/**
* Spy for `upsertConfig` calls. Tests can inspect calls via
* `mockUpsertConfigCalls`.
*/
const mockUpsertConfigCalls: Array<{
sourceId: string
data: { enabled: boolean; config: unknown }
}> = []
/**
* Spy for `updateCredentials` calls. Tests can inspect calls via
* `mockUpdateCredentialsCalls` or override behavior.
@@ -90,9 +81,6 @@ mock.module("../sources/user-sources.ts", () => ({
updatedAt: now,
}
},
async upsertConfig(sourceId: string, data: { enabled: boolean; config: unknown }) {
mockUpsertConfigCalls.push({ sourceId, data })
},
async updateCredentials(sourceId: string, credentials: Buffer) {
if (mockUpdateCredentialsError) {
throw mockUpdateCredentialsError
@@ -150,7 +138,6 @@ const weatherProvider: FeedSourceProvider = {
beforeEach(() => {
enabledByUser.clear()
mockFindResult = undefined
mockUpsertConfigCalls.length = 0
mockUpdateCredentialsCalls.length = 0
mockUpdateCredentialsError = null
})
@@ -819,6 +806,31 @@ describe("UserSessionManager.updateSourceCredentials", () => {
expect(receivedCredentials).toEqual({ token: "refreshed" })
})
test("adds source to session when source is enabled but not yet in session", async () => {
// Simulate a source that was never added to the session (e.g. credentials
// were missing at config time), but is enabled in the DB.
setEnabledSources([]) // no sources during session creation
const factory = mock(async () => createStubSource("test"))
const provider: FeedSourceProvider = { sourceId: "test", feedSourceForUser: factory }
const manager = new UserSessionManager({
db: fakeDb,
providers: [provider],
credentialEncryptor: testEncryptor,
})
const session = await manager.getOrCreate("user-1")
// Source is NOT in the session
expect(session.hasSource("test")).toBe(false)
// mockFindResult returns an enabled row by default, so the source
// row exists and is enabled in the DB.
await manager.updateSourceCredentials("user-1", "test", { token: "new-token" })
// Source should now be added to the session
expect(session.hasSource("test")).toBe(true)
expect(factory).toHaveBeenCalledTimes(1)
})
test("persists credentials without session refresh when no active session", async () => {
setEnabledSources(["test"])
const factory = mock(async () => createStubSource("test"))
@@ -837,65 +849,3 @@ describe("UserSessionManager.updateSourceCredentials", () => {
expect(factory).not.toHaveBeenCalled()
})
})
describe("UserSessionManager.upsertSourceConfig", () => {
test("persists config to DB even when feedSourceForUser throws", async () => {
setEnabledSources(["test"])
let callCount = 0
const factory = mock(async (_userId: string, _config: unknown, _credentials: unknown) => {
callCount++
// Succeed on first call (session creation), throw on second (upsert refresh)
if (callCount > 1) {
throw new InvalidSourceCredentialsError("test", "credentials required")
}
return createStubSource("test")
})
const provider: FeedSourceProvider = { sourceId: "test", feedSourceForUser: factory }
const manager = new UserSessionManager({ db: fakeDb, providers: [provider] })
// Create a session so the session-refresh path is exercised
await manager.getOrCreate("user-1")
const spy = spyOn(console, "warn").mockImplementation(() => {})
// upsertSourceConfig with no existing credentials — provider will throw
await manager.upsertSourceConfig("user-1", "test", {
enabled: true,
config: { url: "https://example.com" },
})
// Config should still have been persisted to DB
expect(mockUpsertConfigCalls).toHaveLength(1)
expect(mockUpsertConfigCalls[0]!.sourceId).toBe("test")
expect(mockUpsertConfigCalls[0]!.data.enabled).toBe(true)
// The error should have been logged, not thrown
expect(spy).toHaveBeenCalled()
spy.mockRestore()
})
test("adds source to session when feedSourceForUser succeeds", async () => {
setEnabledSources(["test"])
const factory = mock(async () => createStubSource("test"))
const provider: FeedSourceProvider = { sourceId: "test", feedSourceForUser: factory }
const manager = new UserSessionManager({ db: fakeDb, providers: [provider] })
const session = await manager.getOrCreate("user-1")
await manager.upsertSourceConfig("user-1", "test", { enabled: true })
// Config persisted
expect(mockUpsertConfigCalls).toHaveLength(1)
// Source should be in the session (feedSourceForUser succeeded)
expect(session.getSource("test")).toBeDefined()
})
test("throws SourceNotFoundError for unknown provider", async () => {
setEnabledSources([])
const manager = new UserSessionManager({ db: fakeDb, providers: [] })
await expect(
manager.upsertSourceConfig("user-1", "unknown", { enabled: true }),
).rejects.toBeInstanceOf(SourceNotFoundError)
})
})

View File

@@ -210,22 +210,11 @@ export class UserSessionManager {
const credentials = existingRow?.credentials
? this.decryptCredentials(existingRow.credentials)
: null
try {
const source = await provider.feedSourceForUser(userId, config, credentials)
if (session.hasSource(sourceId)) {
session.replaceSource(sourceId, source)
} else {
session.addSource(source)
}
} catch (err) {
// Provider may fail when credentials are not yet available (e.g. new
// source added before updateSourceCredentials is called). The config
// is already persisted above; updateSourceCredentials will add the
// source to the session later.
console.warn(
`[UserSessionManager] feedSourceForUser("${sourceId}") failed during upsert for user ${userId}, skipping session update:`,
err,
)
const source = await provider.feedSourceForUser(userId, config, credentials)
if (session.hasSource(sourceId)) {
session.replaceSource(sourceId, source)
} else {
session.addSource(source)
}
}
}
@@ -260,11 +249,15 @@ export class UserSessionManager {
// the DB already has the new credentials but the session keeps the old
// source. The next session creation will pick up the persisted credentials.
const session = this.sessions.get(userId)
if (session && session.hasSource(sourceId)) {
if (session) {
const row = await sources(this.db, userId).find(sourceId)
if (row?.enabled) {
const source = await provider.feedSourceForUser(userId, row.config ?? {}, credentials)
session.replaceSource(sourceId, source)
if (session.hasSource(sourceId)) {
session.replaceSource(sourceId, source)
} else {
session.addSource(source)
}
}
}
}