Compare commits
2 Commits
develop
...
cf6b78bca1
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
cf6b78bca1 | ||
|
|
c18abf84c6 |
14
CHANGELOG.md
14
CHANGELOG.md
@@ -28,6 +28,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||||||
answer was cut off and builds on it instead of restarting; the rest of the
|
answer was cut off and builds on it instead of restarting; the rest of the
|
||||||
queue still flushes normally afterward. (#198)
|
queue still flushes normally afterward. (#198)
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- **A shared page now keeps EXACTLY ONE custom address (`/l/:alias`).** Editing a
|
||||||
|
page's vanity slug previously inserted a second `share_aliases` row instead of
|
||||||
|
renaming the existing one, leaving the old `/l/<old>` link live forever and
|
||||||
|
making the share modal's lookup nondeterministic. Slug edits and confirmed
|
||||||
|
reassigns now rename/retarget the single row, and a new partial unique index on
|
||||||
|
`(workspace_id, page_id)` enforces the invariant in the database. **Upgrade
|
||||||
|
note:** the accompanying migration `20260627T120000` IRREVERSIBLY deletes the
|
||||||
|
orphaned duplicate alias rows the old bug created (keeping the newest per
|
||||||
|
page), so any previously-live duplicate `/l/<old>` link begins returning the
|
||||||
|
generic 404 after upgrade — intended, but not undoable by `down()`. (#226,
|
||||||
|
#227)
|
||||||
|
|
||||||
## [0.94.0] - 2026-06-26
|
## [0.94.0] - 2026-06-26
|
||||||
|
|
||||||
This release makes AI chat durable and fast: assistant turns are persisted to
|
This release makes AI chat durable and fast: assistant turns are persisted to
|
||||||
|
|||||||
@@ -7,13 +7,18 @@ import { ShareAliasService } from './share-alias.service';
|
|||||||
* request-time readable-target resolution (which re-runs the share boundary).
|
* request-time readable-target resolution (which re-runs the share boundary).
|
||||||
*/
|
*/
|
||||||
describe('ShareAliasService', () => {
|
describe('ShareAliasService', () => {
|
||||||
|
// Sentinel handed to repo calls so tests can assert they ran inside the tx.
|
||||||
|
const trx = { __trx: true };
|
||||||
|
|
||||||
function makeService() {
|
function makeService() {
|
||||||
const shareAliasRepo = {
|
const shareAliasRepo = {
|
||||||
findByAliasAndWorkspace: jest.fn(),
|
findByAliasAndWorkspace: jest.fn(),
|
||||||
findByPageId: jest.fn(),
|
findByPageId: jest.fn(),
|
||||||
findById: jest.fn(),
|
findById: jest.fn(),
|
||||||
insert: jest.fn(),
|
insert: jest.fn(),
|
||||||
|
updateAlias: jest.fn(),
|
||||||
updatePageId: jest.fn(),
|
updatePageId: jest.fn(),
|
||||||
|
deleteOthersForPage: jest.fn(),
|
||||||
delete: jest.fn(),
|
delete: jest.fn(),
|
||||||
};
|
};
|
||||||
const pageRepo = { findById: jest.fn() };
|
const pageRepo = { findById: jest.fn() };
|
||||||
@@ -21,12 +26,19 @@ describe('ShareAliasService', () => {
|
|||||||
resolveReadableSharePage: jest.fn(),
|
resolveReadableSharePage: jest.fn(),
|
||||||
isSharingAllowed: jest.fn(),
|
isSharingAllowed: jest.fn(),
|
||||||
};
|
};
|
||||||
|
// Fake kysely db: only .transaction().execute(cb) is used by setAlias.
|
||||||
|
const db = {
|
||||||
|
transaction: jest.fn(() => ({
|
||||||
|
execute: jest.fn(async (cb: any) => cb(trx)),
|
||||||
|
})),
|
||||||
|
};
|
||||||
const service = new ShareAliasService(
|
const service = new ShareAliasService(
|
||||||
shareAliasRepo as any,
|
shareAliasRepo as any,
|
||||||
pageRepo as any,
|
pageRepo as any,
|
||||||
shareService as any,
|
shareService as any,
|
||||||
|
db as any,
|
||||||
);
|
);
|
||||||
return { service, shareAliasRepo, pageRepo, shareService };
|
return { service, shareAliasRepo, pageRepo, shareService, db };
|
||||||
}
|
}
|
||||||
|
|
||||||
describe('setAlias', () => {
|
describe('setAlias', () => {
|
||||||
@@ -43,9 +55,10 @@ describe('ShareAliasService', () => {
|
|||||||
expect(shareAliasRepo.findByAliasAndWorkspace).not.toHaveBeenCalled();
|
expect(shareAliasRepo.findByAliasAndWorkspace).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('normalizes then inserts a brand-new alias', async () => {
|
it('normalizes then inserts a brand-new alias (page has none yet)', async () => {
|
||||||
const { service, shareAliasRepo } = makeService();
|
const { service, shareAliasRepo } = makeService();
|
||||||
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
|
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
|
||||||
|
shareAliasRepo.findByPageId.mockResolvedValue(undefined);
|
||||||
shareAliasRepo.insert.mockResolvedValue({ id: 'a-1', alias: 'my-page' });
|
shareAliasRepo.insert.mockResolvedValue({ id: 'a-1', alias: 'my-page' });
|
||||||
|
|
||||||
const res = await service.setAlias({
|
const res = await service.setAlias({
|
||||||
@@ -58,17 +71,70 @@ describe('ShareAliasService', () => {
|
|||||||
expect(shareAliasRepo.findByAliasAndWorkspace).toHaveBeenCalledWith(
|
expect(shareAliasRepo.findByAliasAndWorkspace).toHaveBeenCalledWith(
|
||||||
'my-page',
|
'my-page',
|
||||||
'ws-1',
|
'ws-1',
|
||||||
|
trx,
|
||||||
|
);
|
||||||
|
expect(shareAliasRepo.insert).toHaveBeenCalledWith(
|
||||||
|
{
|
||||||
|
workspaceId: 'ws-1',
|
||||||
|
alias: 'my-page',
|
||||||
|
pageId: 'p-1',
|
||||||
|
creatorId: 'u-1',
|
||||||
|
},
|
||||||
|
trx,
|
||||||
|
);
|
||||||
|
expect(shareAliasRepo.updateAlias).not.toHaveBeenCalled();
|
||||||
|
// self-heal still runs, keeping just the inserted row
|
||||||
|
expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
|
||||||
|
'p-1',
|
||||||
|
'a-1',
|
||||||
|
'ws-1',
|
||||||
|
trx,
|
||||||
);
|
);
|
||||||
expect(shareAliasRepo.insert).toHaveBeenCalledWith({
|
|
||||||
workspaceId: 'ws-1',
|
|
||||||
alias: 'my-page',
|
|
||||||
pageId: 'p-1',
|
|
||||||
creatorId: 'u-1',
|
|
||||||
});
|
|
||||||
expect(res).toMatchObject({ id: 'a-1' });
|
expect(res).toMatchObject({ id: 'a-1' });
|
||||||
});
|
});
|
||||||
|
|
||||||
it('is a no-op when the alias already points at the same page', async () => {
|
it('renames the existing row in place when editing to a free name (te -> ted)', async () => {
|
||||||
|
const { service, shareAliasRepo } = makeService();
|
||||||
|
// The new slug is free...
|
||||||
|
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
|
||||||
|
// ...but the page already owns an alias named `te`.
|
||||||
|
shareAliasRepo.findByPageId.mockResolvedValue({
|
||||||
|
id: 'a-1',
|
||||||
|
alias: 'te',
|
||||||
|
pageId: 'p-1',
|
||||||
|
});
|
||||||
|
shareAliasRepo.updateAlias.mockResolvedValue({
|
||||||
|
id: 'a-1',
|
||||||
|
alias: 'ted',
|
||||||
|
pageId: 'p-1',
|
||||||
|
});
|
||||||
|
|
||||||
|
const res = await service.setAlias({
|
||||||
|
workspaceId: 'ws-1',
|
||||||
|
pageId: 'p-1',
|
||||||
|
creatorId: 'u-1',
|
||||||
|
alias: 'ted',
|
||||||
|
});
|
||||||
|
|
||||||
|
// RENAME, not INSERT a second row.
|
||||||
|
expect(shareAliasRepo.insert).not.toHaveBeenCalled();
|
||||||
|
expect(shareAliasRepo.updateAlias).toHaveBeenCalledWith(
|
||||||
|
'a-1',
|
||||||
|
'ted',
|
||||||
|
'ws-1',
|
||||||
|
trx,
|
||||||
|
);
|
||||||
|
// ...and any other row for the page is reaped, so `te` cannot survive.
|
||||||
|
expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
|
||||||
|
'p-1',
|
||||||
|
'a-1',
|
||||||
|
'ws-1',
|
||||||
|
trx,
|
||||||
|
);
|
||||||
|
expect(res).toMatchObject({ id: 'a-1', alias: 'ted' });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is a no-op when the alias already points at the same page (and self-heals)', async () => {
|
||||||
const { service, shareAliasRepo } = makeService();
|
const { service, shareAliasRepo } = makeService();
|
||||||
const existing = { id: 'a-1', alias: 'foo', pageId: 'p-1' };
|
const existing = { id: 'a-1', alias: 'foo', pageId: 'p-1' };
|
||||||
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(existing);
|
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(existing);
|
||||||
@@ -82,7 +148,45 @@ describe('ShareAliasService', () => {
|
|||||||
|
|
||||||
expect(res).toBe(existing);
|
expect(res).toBe(existing);
|
||||||
expect(shareAliasRepo.insert).not.toHaveBeenCalled();
|
expect(shareAliasRepo.insert).not.toHaveBeenCalled();
|
||||||
|
expect(shareAliasRepo.updateAlias).not.toHaveBeenCalled();
|
||||||
expect(shareAliasRepo.updatePageId).not.toHaveBeenCalled();
|
expect(shareAliasRepo.updatePageId).not.toHaveBeenCalled();
|
||||||
|
// self-heal reaps any legacy duplicate rows for the page
|
||||||
|
expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
|
||||||
|
'p-1',
|
||||||
|
'a-1',
|
||||||
|
'ws-1',
|
||||||
|
trx,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('self-heals a page with pre-existing duplicate rows down to one', async () => {
|
||||||
|
const { service, shareAliasRepo } = makeService();
|
||||||
|
// Name free; the page already has a (legacy) alias row we rename.
|
||||||
|
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
|
||||||
|
shareAliasRepo.findByPageId.mockResolvedValue({
|
||||||
|
id: 'a-keep',
|
||||||
|
alias: 'old',
|
||||||
|
pageId: 'p-1',
|
||||||
|
});
|
||||||
|
shareAliasRepo.updateAlias.mockResolvedValue({
|
||||||
|
id: 'a-keep',
|
||||||
|
alias: 'new',
|
||||||
|
pageId: 'p-1',
|
||||||
|
});
|
||||||
|
|
||||||
|
await service.setAlias({
|
||||||
|
workspaceId: 'ws-1',
|
||||||
|
pageId: 'p-1',
|
||||||
|
creatorId: 'u-1',
|
||||||
|
alias: 'new',
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
|
||||||
|
'p-1',
|
||||||
|
'a-keep',
|
||||||
|
'ws-1',
|
||||||
|
trx,
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('throws 409 with current target when name is taken and not confirmed', async () => {
|
it('throws 409 with current target when name is taken and not confirmed', async () => {
|
||||||
@@ -134,15 +238,128 @@ describe('ShareAliasService', () => {
|
|||||||
'a-1',
|
'a-1',
|
||||||
'p-1',
|
'p-1',
|
||||||
'ws-1',
|
'ws-1',
|
||||||
|
trx,
|
||||||
);
|
);
|
||||||
|
// ORDER MATTERS: the target page's existing alias row(s) are reaped BEFORE
|
||||||
|
// the retarget, so the non-deferrable (workspace_id, page_id) index never
|
||||||
|
// sees two rows for the page mid-statement. There is no trailing self-heal.
|
||||||
|
expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
|
||||||
|
'p-1',
|
||||||
|
'a-1',
|
||||||
|
'ws-1',
|
||||||
|
trx,
|
||||||
|
);
|
||||||
|
expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledTimes(1);
|
||||||
|
const deleteOrder =
|
||||||
|
shareAliasRepo.deleteOthersForPage.mock.invocationCallOrder[0];
|
||||||
|
const updateOrder =
|
||||||
|
shareAliasRepo.updatePageId.mock.invocationCallOrder[0];
|
||||||
|
expect(deleteOrder).toBeLessThan(updateOrder);
|
||||||
expect(res).toMatchObject({ pageId: 'p-1' });
|
expect(res).toMatchObject({ pageId: 'p-1' });
|
||||||
});
|
});
|
||||||
|
|
||||||
it('maps a unique-violation race to 409', async () => {
|
it('maps a unique-violation race (no constraint info) to 409 "Alias already taken"', async () => {
|
||||||
const { service, shareAliasRepo } = makeService();
|
const { service, shareAliasRepo } = makeService();
|
||||||
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
|
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
|
||||||
shareAliasRepo.insert.mockRejectedValue({ code: '23505' });
|
shareAliasRepo.insert.mockRejectedValue({ code: '23505' });
|
||||||
|
|
||||||
|
try {
|
||||||
|
await service.setAlias({
|
||||||
|
workspaceId: 'ws-1',
|
||||||
|
pageId: 'p-1',
|
||||||
|
creatorId: 'u-1',
|
||||||
|
alias: 'foo',
|
||||||
|
});
|
||||||
|
fail('expected ConflictException');
|
||||||
|
} catch (err) {
|
||||||
|
expect(err).toBeInstanceOf(ConflictException);
|
||||||
|
expect((err as ConflictException).getResponse()).toMatchObject({
|
||||||
|
message: 'Alias already taken',
|
||||||
|
});
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it('maps the (workspace_id, alias) index violation to "Alias already taken"', async () => {
|
||||||
|
const { service, shareAliasRepo } = makeService();
|
||||||
|
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
|
||||||
|
// postgres@3.x driver exposes the index name as `constraint_name`.
|
||||||
|
shareAliasRepo.insert.mockRejectedValue({
|
||||||
|
code: '23505',
|
||||||
|
constraint_name: 'share_aliases_workspace_id_alias_unique',
|
||||||
|
});
|
||||||
|
|
||||||
|
try {
|
||||||
|
await service.setAlias({
|
||||||
|
workspaceId: 'ws-1',
|
||||||
|
pageId: 'p-1',
|
||||||
|
creatorId: 'u-1',
|
||||||
|
alias: 'foo',
|
||||||
|
});
|
||||||
|
fail('expected ConflictException');
|
||||||
|
} catch (err) {
|
||||||
|
expect((err as ConflictException).getResponse()).toMatchObject({
|
||||||
|
message: 'Alias already taken',
|
||||||
|
});
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it('maps the (workspace_id, page_id) index violation to a DISTINCT page-race outcome', async () => {
|
||||||
|
const { service, shareAliasRepo } = makeService();
|
||||||
|
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
|
||||||
|
shareAliasRepo.insert.mockRejectedValue({
|
||||||
|
code: '23505',
|
||||||
|
constraint_name: 'share_aliases_workspace_id_page_id_unique',
|
||||||
|
});
|
||||||
|
|
||||||
|
try {
|
||||||
|
await service.setAlias({
|
||||||
|
workspaceId: 'ws-1',
|
||||||
|
pageId: 'p-1',
|
||||||
|
creatorId: 'u-1',
|
||||||
|
alias: 'foo',
|
||||||
|
});
|
||||||
|
fail('expected ConflictException');
|
||||||
|
} catch (err) {
|
||||||
|
expect(err).toBeInstanceOf(ConflictException);
|
||||||
|
// NOT the misleading "Alias already taken" — a separate, page-scoped code.
|
||||||
|
expect((err as ConflictException).getResponse()).toMatchObject({
|
||||||
|
code: 'ALIAS_PAGE_RACE',
|
||||||
|
});
|
||||||
|
expect((err as ConflictException).getResponse()).not.toMatchObject({
|
||||||
|
message: 'Alias already taken',
|
||||||
|
});
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it('reads the index name from `.constraint` when `.constraint_name` is absent', async () => {
|
||||||
|
const { service, shareAliasRepo } = makeService();
|
||||||
|
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
|
||||||
|
// Fallback path for non-postgres@3.x drivers.
|
||||||
|
shareAliasRepo.insert.mockRejectedValue({
|
||||||
|
code: '23505',
|
||||||
|
constraint: 'share_aliases_workspace_id_page_id_unique',
|
||||||
|
});
|
||||||
|
|
||||||
|
try {
|
||||||
|
await service.setAlias({
|
||||||
|
workspaceId: 'ws-1',
|
||||||
|
pageId: 'p-1',
|
||||||
|
creatorId: 'u-1',
|
||||||
|
alias: 'foo',
|
||||||
|
});
|
||||||
|
fail('expected ConflictException');
|
||||||
|
} catch (err) {
|
||||||
|
expect((err as ConflictException).getResponse()).toMatchObject({
|
||||||
|
code: 'ALIAS_PAGE_RACE',
|
||||||
|
});
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it('maps a non-unique-violation db error to BadRequest (Failed to set alias)', async () => {
|
||||||
|
const { service, shareAliasRepo } = makeService();
|
||||||
|
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
|
||||||
|
shareAliasRepo.insert.mockRejectedValue({ code: '08006' }); // connection error
|
||||||
|
|
||||||
await expect(
|
await expect(
|
||||||
service.setAlias({
|
service.setAlias({
|
||||||
workspaceId: 'ws-1',
|
workspaceId: 'ws-1',
|
||||||
@@ -150,7 +367,7 @@ describe('ShareAliasService', () => {
|
|||||||
creatorId: 'u-1',
|
creatorId: 'u-1',
|
||||||
alias: 'foo',
|
alias: 'foo',
|
||||||
}),
|
}),
|
||||||
).rejects.toBeInstanceOf(ConflictException);
|
).rejects.toBeInstanceOf(BadRequestException);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -9,10 +9,25 @@ import { PageRepo } from '@docmost/db/repos/page/page.repo';
|
|||||||
import { ShareService } from './share.service';
|
import { ShareService } from './share.service';
|
||||||
import { Page, ShareAlias } from '@docmost/db/types/entity.types';
|
import { Page, ShareAlias } from '@docmost/db/types/entity.types';
|
||||||
import { isValidShareAlias, normalizeShareAlias } from './share-alias.util';
|
import { isValidShareAlias, normalizeShareAlias } from './share-alias.util';
|
||||||
|
import { InjectKysely } from 'nestjs-kysely';
|
||||||
|
import { KyselyDB } from '@docmost/db/types/kysely.types';
|
||||||
|
import { executeTx } from '@docmost/db/utils';
|
||||||
|
|
||||||
/** Postgres unique_violation; the (workspace_id, alias) constraint races here. */
|
/** Postgres unique_violation. Two unique indexes can raise it on this table. */
|
||||||
const PG_UNIQUE_VIOLATION = '23505';
|
const PG_UNIQUE_VIOLATION = '23505';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Unique index names from the share_aliases migrations. The `postgres@3.x`
|
||||||
|
* driver (kysely-postgres-js) surfaces the violated constraint as
|
||||||
|
* `err.constraint_name` (NOT `.constraint`); we keep `.constraint` only as a
|
||||||
|
* defensive fallback for other drivers.
|
||||||
|
* - ALIAS: `(workspace_id, alias)` -> the vanity NAME is taken.
|
||||||
|
* - PAGE_ID: partial `(workspace_id, page_id) WHERE page_id IS NOT NULL`
|
||||||
|
* -> a concurrent writer already gave THIS page an alias.
|
||||||
|
*/
|
||||||
|
const UNIQUE_ALIAS_INDEX = 'share_aliases_workspace_id_alias_unique';
|
||||||
|
const UNIQUE_PAGE_ID_INDEX = 'share_aliases_workspace_id_page_id_unique';
|
||||||
|
|
||||||
export interface ResolvedAliasTarget {
|
export interface ResolvedAliasTarget {
|
||||||
share: NonNullable<
|
share: NonNullable<
|
||||||
Awaited<ReturnType<ShareService['resolveReadableSharePage']>>
|
Awaited<ReturnType<ShareService['resolveReadableSharePage']>>
|
||||||
@@ -28,16 +43,30 @@ export class ShareAliasService {
|
|||||||
private readonly shareAliasRepo: ShareAliasRepo,
|
private readonly shareAliasRepo: ShareAliasRepo,
|
||||||
private readonly pageRepo: PageRepo,
|
private readonly pageRepo: PageRepo,
|
||||||
private readonly shareService: ShareService,
|
private readonly shareService: ShareService,
|
||||||
|
@InjectKysely() private readonly db: KyselyDB,
|
||||||
) {}
|
) {}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Create or retarget a vanity alias. The alias is workspace-scoped:
|
* Create, RENAME or retarget a page's vanity alias. INVARIANT: a page has
|
||||||
* - no row for this name -> INSERT a new pointer
|
* EXACTLY ONE custom address. The alias name is workspace-scoped:
|
||||||
* - row already points at pageId -> no-op (idempotent)
|
* - name free, page has no alias yet -> INSERT a new pointer
|
||||||
* - row points elsewhere -> the "swap". Without confirmReassign we
|
* - name free, page already has one -> RENAME that row in place (the slug
|
||||||
* throw 409 carrying the current target so the client can confirm; with
|
* edit, e.g. `te` -> `ted`); we never spawn a second row, so no orphan
|
||||||
* it we UPDATE the single row's page_id (every /l/<alias> link follows the
|
* `/l/<old>` link survives
|
||||||
* 302 to the new page instantly — no stale 301 cache).
|
* - name already points at pageId -> no-op (idempotent)
|
||||||
|
* - name points at ANOTHER page -> the "swap". Without confirmReassign
|
||||||
|
* we throw 409 carrying the current target so the client can confirm;
|
||||||
|
* with it we UPDATE the single row's page_id (every /l/<alias> link
|
||||||
|
* follows the 302 to the new page instantly — no stale cache).
|
||||||
|
*
|
||||||
|
* To keep the invariant self-healing we DELETE every other alias row still
|
||||||
|
* pointing at this page (a legacy duplicate, or the target page's own former
|
||||||
|
* alias during a swap). The whole thing runs in one transaction. Because the
|
||||||
|
* `(workspace_id, page_id)` unique index is NON-deferrable (checked at the end
|
||||||
|
* of each statement), the swap branch DELETEs the target page's existing row
|
||||||
|
* BEFORE retargeting, so the page is never transiently carried by two rows;
|
||||||
|
* the other branches self-heal AFTER their write. Either way the page never
|
||||||
|
* ends a statement with duplicate rows.
|
||||||
*
|
*
|
||||||
* Caller is responsible for authorizing the page (edit rights + public
|
* Caller is responsible for authorizing the page (edit rights + public
|
||||||
* readability); this method owns only the alias-name semantics.
|
* readability); this method owns only the alias-name semantics.
|
||||||
@@ -57,48 +86,121 @@ export class ShareAliasService {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
const existing = await this.shareAliasRepo.findByAliasAndWorkspace(
|
try {
|
||||||
alias,
|
return await executeTx(this.db, async (trx) => {
|
||||||
workspaceId,
|
const byName = await this.shareAliasRepo.findByAliasAndWorkspace(
|
||||||
);
|
|
||||||
|
|
||||||
if (!existing) {
|
|
||||||
try {
|
|
||||||
return await this.shareAliasRepo.insert({
|
|
||||||
workspaceId,
|
|
||||||
alias,
|
alias,
|
||||||
pageId,
|
workspaceId,
|
||||||
creatorId,
|
trx,
|
||||||
});
|
);
|
||||||
} catch (err: any) {
|
|
||||||
// Lost a uniqueness race: another request claimed the name first.
|
// The name is occupied by a DIFFERENT (or dangling) target page.
|
||||||
if (err?.code === PG_UNIQUE_VIOLATION) {
|
if (byName && byName.pageId !== pageId) {
|
||||||
throw new ConflictException({ message: 'Alias already taken' });
|
if (!confirmReassign) {
|
||||||
|
const currentPage = byName.pageId
|
||||||
|
? await this.pageRepo.findById(byName.pageId)
|
||||||
|
: null;
|
||||||
|
throw new ConflictException({
|
||||||
|
message: 'Alias already in use',
|
||||||
|
code: 'ALIAS_REASSIGN_REQUIRED',
|
||||||
|
currentPageId: byName.pageId,
|
||||||
|
currentPageTitle: currentPage?.title ?? null,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
// Confirmed swap. ORDER MATTERS: the partial unique index on
|
||||||
|
// `(workspace_id, page_id)` is NON-deferrable, so it is checked at the
|
||||||
|
// end of EVERY statement. If we retargeted `byName` onto `pageId`
|
||||||
|
// first while `pageId` still had its OWN alias row, there would
|
||||||
|
// momentarily be two rows with this page_id -> immediate 23505 and a
|
||||||
|
// rolled-back tx (a misleading "Alias already taken"). So we FIRST drop
|
||||||
|
// the target page's existing alias row(s), THEN retarget. `byName.id`
|
||||||
|
// still points at its old page here, so excluding it via `keepId` is
|
||||||
|
// harmless; after the retarget it is the page's only row, so no
|
||||||
|
// trailing self-heal is needed.
|
||||||
|
await this.shareAliasRepo.deleteOthersForPage(
|
||||||
|
pageId,
|
||||||
|
byName.id,
|
||||||
|
workspaceId,
|
||||||
|
trx,
|
||||||
|
);
|
||||||
|
return await this.shareAliasRepo.updatePageId(
|
||||||
|
byName.id,
|
||||||
|
pageId,
|
||||||
|
workspaceId,
|
||||||
|
trx,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
this.logger.error(err);
|
|
||||||
throw new BadRequestException('Failed to set alias');
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Already points at this page -> nothing to do.
|
// The name is FREE, or already points at THIS page. Ensure the page has
|
||||||
if (existing.pageId === pageId) {
|
// a single row carrying this name: rename its current one, or insert.
|
||||||
return existing;
|
const current =
|
||||||
}
|
byName ??
|
||||||
|
(await this.shareAliasRepo.findByPageId(pageId, workspaceId, trx));
|
||||||
|
|
||||||
// Name occupied by a different (or dangling) target: require confirmation.
|
let row: ShareAlias;
|
||||||
if (!confirmReassign) {
|
if (current) {
|
||||||
const currentPage = existing.pageId
|
row =
|
||||||
? await this.pageRepo.findById(existing.pageId)
|
current.alias === alias
|
||||||
: null;
|
? current // same-name no-op
|
||||||
throw new ConflictException({
|
: await this.shareAliasRepo.updateAlias(
|
||||||
message: 'Alias already in use',
|
current.id,
|
||||||
code: 'ALIAS_REASSIGN_REQUIRED',
|
alias,
|
||||||
currentPageId: existing.pageId,
|
workspaceId,
|
||||||
currentPageTitle: currentPage?.title ?? null,
|
trx,
|
||||||
|
);
|
||||||
|
} else {
|
||||||
|
row = await this.shareAliasRepo.insert(
|
||||||
|
{ workspaceId, alias, pageId, creatorId },
|
||||||
|
trx,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Self-heal: a page keeps EXACTLY ONE custom address.
|
||||||
|
await this.shareAliasRepo.deleteOthersForPage(
|
||||||
|
pageId,
|
||||||
|
row.id,
|
||||||
|
workspaceId,
|
||||||
|
trx,
|
||||||
|
);
|
||||||
|
return row;
|
||||||
});
|
});
|
||||||
|
} catch (err: any) {
|
||||||
|
if (
|
||||||
|
err instanceof ConflictException ||
|
||||||
|
err instanceof BadRequestException
|
||||||
|
) {
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
// A unique index fired. Which one decides the message — always log the
|
||||||
|
// constraint so the race is diagnosable.
|
||||||
|
if (err?.code === PG_UNIQUE_VIOLATION) {
|
||||||
|
const constraint: string | undefined =
|
||||||
|
err?.constraint_name ?? err?.constraint;
|
||||||
|
this.logger.warn(
|
||||||
|
`share alias unique violation on ${constraint ?? '<unknown>'}`,
|
||||||
|
);
|
||||||
|
// `(workspace_id, page_id)`: a concurrent request already gave this page
|
||||||
|
// an alias. The page still has exactly one custom address (the racing
|
||||||
|
// writer's), so this is not a user-facing name clash — surface a
|
||||||
|
// distinct, non-misleading message instead of "Alias already taken".
|
||||||
|
if (constraint === UNIQUE_PAGE_ID_INDEX) {
|
||||||
|
throw new ConflictException({
|
||||||
|
message: 'This page is being given an address by another request',
|
||||||
|
code: 'ALIAS_PAGE_RACE',
|
||||||
|
});
|
||||||
|
}
|
||||||
|
// `(workspace_id, alias)` (UNIQUE_ALIAS_INDEX) or any other/unknown
|
||||||
|
// unique index: treat as the vanity name being claimed first.
|
||||||
|
if (constraint && constraint !== UNIQUE_ALIAS_INDEX) {
|
||||||
|
this.logger.warn(
|
||||||
|
`unexpected unique index ${constraint} mapped to "Alias already taken"`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
throw new ConflictException({ message: 'Alias already taken' });
|
||||||
|
}
|
||||||
|
this.logger.error(err);
|
||||||
|
throw new BadRequestException('Failed to set alias');
|
||||||
}
|
}
|
||||||
|
|
||||||
return this.shareAliasRepo.updatePageId(existing.id, pageId, workspaceId);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Free a vanity name (no history kept). */
|
/** Free a vanity name (no history kept). */
|
||||||
|
|||||||
@@ -0,0 +1,48 @@
|
|||||||
|
import { type Kysely, sql } from 'kysely';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Enforce "a page has EXACTLY ONE custom address" at the DB level. The original
|
||||||
|
* `share_aliases` table only had a unique index on `(workspace_id, alias)`, so a
|
||||||
|
* page could accumulate several alias rows (every slug edit used to INSERT a new
|
||||||
|
* one), leaving orphan `/l/<old>` links live forever and making the share
|
||||||
|
* modal's `findByPageId` lookup nondeterministic.
|
||||||
|
*
|
||||||
|
* We first dedup any pre-existing rows (keeping the NEWEST per page — the same
|
||||||
|
* "current" choice the read path now makes), then add a PARTIAL unique index on
|
||||||
|
* `(workspace_id, page_id)`. It is partial (`WHERE page_id IS NOT NULL`) so that
|
||||||
|
* multiple DANGLING aliases (target page deleted -> `page_id` SET NULL) can
|
||||||
|
* still coexist without colliding.
|
||||||
|
*
|
||||||
|
* ⚠️ IRREVERSIBLE DATA LOSS (intended): the dedup DELETE below permanently drops
|
||||||
|
* every alias row but the newest per page. Those duplicates were live `/l/<old>`
|
||||||
|
* pointers (resolved by name via `findByAliasAndWorkspace`, not by page), so
|
||||||
|
* after this upgrade any such OLD vanity link starts returning the SPA 404. This
|
||||||
|
* is the point — it kills the orphan rows the pre-invariant bug accumulated —
|
||||||
|
* but `down()` only drops the unique index; it CANNOT restore the deleted rows.
|
||||||
|
*/
|
||||||
|
export async function up(db: Kysely<any>): Promise<void> {
|
||||||
|
// Reap legacy duplicates: for each (workspace_id, page_id) keep only the row
|
||||||
|
// with the greatest (created_at, id) — matches ShareAliasRepo.findByPageId.
|
||||||
|
await sql`
|
||||||
|
DELETE FROM share_aliases sa
|
||||||
|
USING share_aliases keep
|
||||||
|
WHERE sa.page_id IS NOT NULL
|
||||||
|
AND sa.workspace_id = keep.workspace_id
|
||||||
|
AND sa.page_id = keep.page_id
|
||||||
|
AND (keep.created_at, keep.id) > (sa.created_at, sa.id)
|
||||||
|
`.execute(db);
|
||||||
|
|
||||||
|
await db.schema
|
||||||
|
.createIndex('share_aliases_workspace_id_page_id_unique')
|
||||||
|
.on('share_aliases')
|
||||||
|
.columns(['workspace_id', 'page_id'])
|
||||||
|
.unique()
|
||||||
|
.where('page_id', 'is not', null)
|
||||||
|
.execute();
|
||||||
|
}
|
||||||
|
|
||||||
|
export async function down(db: Kysely<any>): Promise<void> {
|
||||||
|
await db.schema
|
||||||
|
.dropIndex('share_aliases_workspace_id_page_id_unique')
|
||||||
|
.execute();
|
||||||
|
}
|
||||||
@@ -10,16 +10,21 @@ import type { KyselyDB } from '../../types/kysely.types';
|
|||||||
describe('ShareAliasRepo', () => {
|
describe('ShareAliasRepo', () => {
|
||||||
function makeSelectRepo(result: unknown) {
|
function makeSelectRepo(result: unknown) {
|
||||||
const where = jest.fn();
|
const where = jest.fn();
|
||||||
|
const orderBy = jest.fn();
|
||||||
const builder: any = {
|
const builder: any = {
|
||||||
select: jest.fn(() => builder),
|
select: jest.fn(() => builder),
|
||||||
where: jest.fn((...args: unknown[]) => {
|
where: jest.fn((...args: unknown[]) => {
|
||||||
where(...args);
|
where(...args);
|
||||||
return builder;
|
return builder;
|
||||||
}),
|
}),
|
||||||
|
orderBy: jest.fn((...args: unknown[]) => {
|
||||||
|
orderBy(...args);
|
||||||
|
return builder;
|
||||||
|
}),
|
||||||
executeTakeFirst: jest.fn().mockResolvedValue(result),
|
executeTakeFirst: jest.fn().mockResolvedValue(result),
|
||||||
};
|
};
|
||||||
const db = { selectFrom: jest.fn(() => builder) } as unknown as KyselyDB;
|
const db = { selectFrom: jest.fn(() => builder) } as unknown as KyselyDB;
|
||||||
return { repo: new ShareAliasRepo(db), db, where, builder };
|
return { repo: new ShareAliasRepo(db), db, where, orderBy, builder };
|
||||||
}
|
}
|
||||||
|
|
||||||
it('findByAliasAndWorkspace scopes by alias AND workspace', async () => {
|
it('findByAliasAndWorkspace scopes by alias AND workspace', async () => {
|
||||||
@@ -34,11 +39,15 @@ describe('ShareAliasRepo', () => {
|
|||||||
expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1');
|
expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('findByPageId scopes by page AND workspace', async () => {
|
it('findByPageId scopes by page AND workspace, deterministically ordered', async () => {
|
||||||
const { repo, where } = makeSelectRepo(undefined);
|
const { repo, where, orderBy } = makeSelectRepo(undefined);
|
||||||
await repo.findByPageId('p-1', 'ws-1');
|
await repo.findByPageId('p-1', 'ws-1');
|
||||||
expect(where).toHaveBeenCalledWith('pageId', '=', 'p-1');
|
expect(where).toHaveBeenCalledWith('pageId', '=', 'p-1');
|
||||||
expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1');
|
expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1');
|
||||||
|
// Explicit ORDER BY removes the nondeterministic heap order for any legacy
|
||||||
|
// duplicate rows (newest createdAt wins, id as a stable tiebreak).
|
||||||
|
expect(orderBy).toHaveBeenCalledWith('createdAt', 'desc');
|
||||||
|
expect(orderBy).toHaveBeenCalledWith('id', 'desc');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('insert writes the provided columns and returns the row', async () => {
|
it('insert writes the provided columns and returns the row', async () => {
|
||||||
@@ -99,6 +108,56 @@ describe('ShareAliasRepo', () => {
|
|||||||
expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1');
|
expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('updateAlias renames a single row scoped by id + workspace', async () => {
|
||||||
|
const set = jest.fn();
|
||||||
|
const where = jest.fn();
|
||||||
|
const builder: any = {
|
||||||
|
set: jest.fn((s: unknown) => {
|
||||||
|
set(s);
|
||||||
|
return builder;
|
||||||
|
}),
|
||||||
|
where: jest.fn((...args: unknown[]) => {
|
||||||
|
where(...args);
|
||||||
|
return builder;
|
||||||
|
}),
|
||||||
|
returning: jest.fn(() => builder),
|
||||||
|
executeTakeFirst: jest.fn().mockResolvedValue({ id: 'a-1', alias: 'ted' }),
|
||||||
|
};
|
||||||
|
const db = { updateTable: jest.fn(() => builder) } as unknown as KyselyDB;
|
||||||
|
const repo = new ShareAliasRepo(db);
|
||||||
|
|
||||||
|
const res = await repo.updateAlias('a-1', 'ted', 'ws-1');
|
||||||
|
|
||||||
|
expect(db.updateTable).toHaveBeenCalledWith('shareAliases');
|
||||||
|
expect(set.mock.calls[0][0].alias).toBe('ted');
|
||||||
|
expect(set.mock.calls[0][0].updatedAt).toBeInstanceOf(Date);
|
||||||
|
// a rename must NOT touch page_id (the page's pointer is preserved)
|
||||||
|
expect(set.mock.calls[0][0]).not.toHaveProperty('pageId');
|
||||||
|
expect(where).toHaveBeenCalledWith('id', '=', 'a-1');
|
||||||
|
expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1');
|
||||||
|
expect(res).toMatchObject({ alias: 'ted' });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('deleteOthersForPage reaps every row for the page except keepId', async () => {
|
||||||
|
const where = jest.fn();
|
||||||
|
const builder: any = {
|
||||||
|
where: jest.fn((...args: unknown[]) => {
|
||||||
|
where(...args);
|
||||||
|
return builder;
|
||||||
|
}),
|
||||||
|
execute: jest.fn().mockResolvedValue(undefined),
|
||||||
|
};
|
||||||
|
const db = { deleteFrom: jest.fn(() => builder) } as unknown as KyselyDB;
|
||||||
|
const repo = new ShareAliasRepo(db);
|
||||||
|
|
||||||
|
await repo.deleteOthersForPage('p-1', 'a-keep', 'ws-1');
|
||||||
|
|
||||||
|
expect(db.deleteFrom).toHaveBeenCalledWith('shareAliases');
|
||||||
|
expect(where).toHaveBeenCalledWith('pageId', '=', 'p-1');
|
||||||
|
expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1');
|
||||||
|
expect(where).toHaveBeenCalledWith('id', '!=', 'a-keep');
|
||||||
|
});
|
||||||
|
|
||||||
it('delete scopes by id + workspace', async () => {
|
it('delete scopes by id + workspace', async () => {
|
||||||
const where = jest.fn();
|
const where = jest.fn();
|
||||||
const builder: any = {
|
const builder: any = {
|
||||||
|
|||||||
@@ -41,7 +41,14 @@ export class ShareAliasRepo {
|
|||||||
.executeTakeFirst();
|
.executeTakeFirst();
|
||||||
}
|
}
|
||||||
|
|
||||||
/** The alias currently pointing at a page (for the share modal). */
|
/**
|
||||||
|
* The alias currently pointing at a page (for the share modal). The service
|
||||||
|
* enforces a single alias row per page, but legacy rows (pre-invariant) may
|
||||||
|
* still exist until self-healed; the explicit ORDER BY makes the "current"
|
||||||
|
* choice DETERMINISTIC (newest wins — i.e. the most recently created address,
|
||||||
|
* which is the one the user last asked for) instead of an arbitrary Postgres
|
||||||
|
* heap order.
|
||||||
|
*/
|
||||||
async findByPageId(
|
async findByPageId(
|
||||||
pageId: string,
|
pageId: string,
|
||||||
workspaceId: string,
|
workspaceId: string,
|
||||||
@@ -52,6 +59,8 @@ export class ShareAliasRepo {
|
|||||||
.select(this.baseFields)
|
.select(this.baseFields)
|
||||||
.where('pageId', '=', pageId)
|
.where('pageId', '=', pageId)
|
||||||
.where('workspaceId', '=', workspaceId)
|
.where('workspaceId', '=', workspaceId)
|
||||||
|
.orderBy('createdAt', 'desc')
|
||||||
|
.orderBy('id', 'desc')
|
||||||
.executeTakeFirst();
|
.executeTakeFirst();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -79,6 +88,45 @@ export class ShareAliasRepo {
|
|||||||
.executeTakeFirst();
|
.executeTakeFirst();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Rename an existing alias row in place (the vanity-slug edit, e.g.
|
||||||
|
* `te` -> `ted`). Keeps the row's id/page_id/creator so the page's single
|
||||||
|
* alias pointer is preserved — only the human-readable name changes.
|
||||||
|
*/
|
||||||
|
async updateAlias(
|
||||||
|
id: string,
|
||||||
|
alias: string,
|
||||||
|
workspaceId: string,
|
||||||
|
trx?: KyselyTransaction,
|
||||||
|
): Promise<ShareAlias> {
|
||||||
|
return dbOrTx(this.db, trx)
|
||||||
|
.updateTable('shareAliases')
|
||||||
|
.set({ alias, updatedAt: new Date() })
|
||||||
|
.where('id', '=', id)
|
||||||
|
.where('workspaceId', '=', workspaceId)
|
||||||
|
.returning(this.baseFields)
|
||||||
|
.executeTakeFirst();
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Self-heal helper: drop every OTHER alias row still pointing at a page,
|
||||||
|
* keeping only `keepId`. Enforces the "exactly one custom address per page"
|
||||||
|
* invariant after a rename/retarget and reaps any legacy duplicates.
|
||||||
|
*/
|
||||||
|
async deleteOthersForPage(
|
||||||
|
pageId: string,
|
||||||
|
keepId: string,
|
||||||
|
workspaceId: string,
|
||||||
|
trx?: KyselyTransaction,
|
||||||
|
): Promise<void> {
|
||||||
|
await dbOrTx(this.db, trx)
|
||||||
|
.deleteFrom('shareAliases')
|
||||||
|
.where('pageId', '=', pageId)
|
||||||
|
.where('workspaceId', '=', workspaceId)
|
||||||
|
.where('id', '!=', keepId)
|
||||||
|
.execute();
|
||||||
|
}
|
||||||
|
|
||||||
/** Retarget an existing alias to a new page (the "swap" operation). */
|
/** Retarget an existing alias to a new page (the "swap" operation). */
|
||||||
async updatePageId(
|
async updatePageId(
|
||||||
id: string,
|
id: string,
|
||||||
|
|||||||
@@ -0,0 +1,344 @@
|
|||||||
|
import { Kysely, sql } from 'kysely';
|
||||||
|
import { randomUUID } from 'node:crypto';
|
||||||
|
import { BadRequestException, ConflictException } from '@nestjs/common';
|
||||||
|
import { ShareAliasRepo } from '@docmost/db/repos/share-alias/share-alias.repo';
|
||||||
|
import { ShareAliasService } from 'src/core/share/share-alias.service';
|
||||||
|
import * as onePerPageMigration from 'src/database/migrations/20260627T120000-share-aliases-one-per-page';
|
||||||
|
import {
|
||||||
|
getTestDb,
|
||||||
|
destroyTestDb,
|
||||||
|
createWorkspace,
|
||||||
|
createSpace,
|
||||||
|
createPage,
|
||||||
|
} from './db';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Issue #226 (regression of #205): "a page has EXACTLY ONE custom address".
|
||||||
|
* Exercises against real Postgres:
|
||||||
|
* - the partial unique index `(workspace_id, page_id) WHERE page_id IS NOT NULL`
|
||||||
|
* (migration 20260627T120000) — one alias per page, but dangling aliases
|
||||||
|
* (page_id NULL) may coexist;
|
||||||
|
* - the migration's dedup DELETE keeps the NEWEST row per page;
|
||||||
|
* - ShareAliasService.setAlias renames in place (te -> ted) instead of
|
||||||
|
* spawning a second row, and self-heals the page down to one alias.
|
||||||
|
*/
|
||||||
|
describe('share_aliases one-per-page invariant [integration]', () => {
|
||||||
|
let db: Kysely<any>;
|
||||||
|
let repo: ShareAliasRepo;
|
||||||
|
let service: ShareAliasService;
|
||||||
|
let wsId: string;
|
||||||
|
let spaceId: string;
|
||||||
|
|
||||||
|
// setAlias only consults pageRepo on the unconfirmed-reassign (409) path.
|
||||||
|
const pageRepo = {
|
||||||
|
findById: async (id: string) => ({ id, title: `title-${id}` }),
|
||||||
|
};
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
db = getTestDb();
|
||||||
|
repo = new ShareAliasRepo(db as any);
|
||||||
|
service = new ShareAliasService(
|
||||||
|
repo as any,
|
||||||
|
pageRepo as any,
|
||||||
|
{} as any, // shareService — unused by setAlias
|
||||||
|
db as any,
|
||||||
|
);
|
||||||
|
wsId = (await createWorkspace(db)).id;
|
||||||
|
spaceId = (await createSpace(db, wsId)).id;
|
||||||
|
});
|
||||||
|
|
||||||
|
afterAll(async () => {
|
||||||
|
await destroyTestDb();
|
||||||
|
});
|
||||||
|
|
||||||
|
const newPage = async (): Promise<string> =>
|
||||||
|
(await createPage(db, { workspaceId: wsId, spaceId })).id;
|
||||||
|
|
||||||
|
const aliasRowsForWs = (pageId: string, workspaceId: string) =>
|
||||||
|
db
|
||||||
|
.selectFrom('shareAliases')
|
||||||
|
.select(['id', 'alias'])
|
||||||
|
.where('pageId', '=', pageId)
|
||||||
|
.where('workspaceId', '=', workspaceId)
|
||||||
|
.orderBy('alias')
|
||||||
|
.execute();
|
||||||
|
|
||||||
|
const aliasRowsFor = (pageId: string) => aliasRowsForWs(pageId, wsId);
|
||||||
|
|
||||||
|
it('partial unique index rejects a second alias for the same page (23505)', async () => {
|
||||||
|
const pageId = await newPage();
|
||||||
|
await repo.insert({ workspaceId: wsId, alias: 'first', pageId });
|
||||||
|
|
||||||
|
let code: string | undefined;
|
||||||
|
try {
|
||||||
|
await repo.insert({ workspaceId: wsId, alias: 'second', pageId });
|
||||||
|
} catch (err: any) {
|
||||||
|
code = err?.code ?? err?.cause?.code;
|
||||||
|
}
|
||||||
|
expect(code).toBe('23505');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('allows multiple DANGLING aliases (page_id NULL) — partial index excludes them', async () => {
|
||||||
|
const a = await repo.insert({
|
||||||
|
workspaceId: wsId,
|
||||||
|
alias: `dangling-${randomUUID().slice(0, 8)}`,
|
||||||
|
pageId: null as any,
|
||||||
|
});
|
||||||
|
const b = await repo.insert({
|
||||||
|
workspaceId: wsId,
|
||||||
|
alias: `dangling-${randomUUID().slice(0, 8)}`,
|
||||||
|
pageId: null as any,
|
||||||
|
});
|
||||||
|
expect(a.id).toBeDefined();
|
||||||
|
expect(b.id).toBeDefined();
|
||||||
|
expect(a.id).not.toBe(b.id);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("migration dedup DELETE keeps the page's NEWEST alias row", async () => {
|
||||||
|
const pageId = await newPage();
|
||||||
|
// Temporarily drop the guard so we can seed the legacy duplicate shape.
|
||||||
|
await sql`DROP INDEX share_aliases_workspace_id_page_id_unique`.execute(db);
|
||||||
|
try {
|
||||||
|
const mk = async (alias: string, createdAt: string): Promise<string> => {
|
||||||
|
const id = randomUUID();
|
||||||
|
await db
|
||||||
|
.insertInto('shareAliases')
|
||||||
|
.values({ id, workspaceId: wsId, alias, pageId, createdAt })
|
||||||
|
.execute();
|
||||||
|
return id;
|
||||||
|
};
|
||||||
|
await mk('oldest', '2026-01-01T00:00:00Z');
|
||||||
|
await mk('middle', '2026-02-01T00:00:00Z');
|
||||||
|
const newest = await mk('newest', '2026-03-01T00:00:00Z');
|
||||||
|
|
||||||
|
// Exact dedup statement from the migration.
|
||||||
|
await sql`
|
||||||
|
DELETE FROM share_aliases sa
|
||||||
|
USING share_aliases keep
|
||||||
|
WHERE sa.page_id IS NOT NULL
|
||||||
|
AND sa.workspace_id = keep.workspace_id
|
||||||
|
AND sa.page_id = keep.page_id
|
||||||
|
AND (keep.created_at, keep.id) > (sa.created_at, sa.id)
|
||||||
|
`.execute(db);
|
||||||
|
|
||||||
|
const rows = await aliasRowsFor(pageId);
|
||||||
|
expect(rows).toHaveLength(1);
|
||||||
|
expect(rows[0]).toMatchObject({ id: newest, alias: 'newest' });
|
||||||
|
} finally {
|
||||||
|
await sql`
|
||||||
|
CREATE UNIQUE INDEX share_aliases_workspace_id_page_id_unique
|
||||||
|
ON share_aliases (workspace_id, page_id)
|
||||||
|
WHERE page_id IS NOT NULL
|
||||||
|
`.execute(db);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it('migration up() dedups per page and leaves OTHER pages and workspaces untouched', async () => {
|
||||||
|
// Seed legacy duplicates for two pages in this workspace AND a page in a
|
||||||
|
// SECOND workspace, then run the real migration up() (not an inlined copy of
|
||||||
|
// its SQL) and assert it scopes the DELETE to (workspace_id, page_id).
|
||||||
|
const ws2 = (await createWorkspace(db)).id;
|
||||||
|
const space2 = (await createSpace(db, ws2)).id;
|
||||||
|
const pageA = await newPage();
|
||||||
|
const pageB = await newPage();
|
||||||
|
const pageC = (await createPage(db, { workspaceId: ws2, spaceId: space2 }))
|
||||||
|
.id;
|
||||||
|
|
||||||
|
// Drop the guard so we can seed the pre-invariant duplicate shape.
|
||||||
|
await sql`DROP INDEX share_aliases_workspace_id_page_id_unique`.execute(db);
|
||||||
|
const seed = async (
|
||||||
|
workspaceId: string,
|
||||||
|
pageId: string,
|
||||||
|
alias: string,
|
||||||
|
createdAt: string,
|
||||||
|
): Promise<string> => {
|
||||||
|
const id = randomUUID();
|
||||||
|
await db
|
||||||
|
.insertInto('shareAliases')
|
||||||
|
.values({ id, workspaceId, alias, pageId, createdAt })
|
||||||
|
.execute();
|
||||||
|
return id;
|
||||||
|
};
|
||||||
|
await seed(wsId, pageA, 'a-old', '2026-01-01T00:00:00Z');
|
||||||
|
const aNew = await seed(wsId, pageA, 'a-new', '2026-03-01T00:00:00Z');
|
||||||
|
await seed(wsId, pageB, 'b-old', '2026-01-01T00:00:00Z');
|
||||||
|
const bNew = await seed(wsId, pageB, 'b-new', '2026-03-01T00:00:00Z');
|
||||||
|
await seed(ws2, pageC, 'c-old', '2026-01-01T00:00:00Z');
|
||||||
|
const cNew = await seed(ws2, pageC, 'c-new', '2026-03-01T00:00:00Z');
|
||||||
|
|
||||||
|
// Run the migration. It dedups AND recreates the unique index.
|
||||||
|
await onePerPageMigration.up(db as any);
|
||||||
|
|
||||||
|
const aliasesOf = async (pageId: string) =>
|
||||||
|
(await aliasRowsForWs(pageId, wsId)).map((r) => r.alias);
|
||||||
|
const aRows = await aliasRowsForWs(pageA, wsId);
|
||||||
|
expect(aRows).toEqual([{ id: aNew, alias: 'a-new' }]);
|
||||||
|
const bRows = await aliasRowsForWs(pageB, wsId);
|
||||||
|
expect(bRows).toEqual([{ id: bNew, alias: 'b-new' }]);
|
||||||
|
// The other workspace's page keeps only ITS newest row, untouched by wsId.
|
||||||
|
const cRows = await aliasRowsForWs(pageC, ws2);
|
||||||
|
expect(cRows).toEqual([{ id: cNew, alias: 'c-new' }]);
|
||||||
|
expect(await aliasesOf(pageA)).toEqual(['a-new']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('setAlias renames te -> ted in place: page ends with ONE row named ted', async () => {
|
||||||
|
const pageId = await newPage();
|
||||||
|
const creatorId = null as any;
|
||||||
|
const first = await service.setAlias({
|
||||||
|
workspaceId: wsId,
|
||||||
|
pageId,
|
||||||
|
creatorId,
|
||||||
|
alias: 'te',
|
||||||
|
});
|
||||||
|
expect(first.alias).toBe('te');
|
||||||
|
|
||||||
|
const renamed = await service.setAlias({
|
||||||
|
workspaceId: wsId,
|
||||||
|
pageId,
|
||||||
|
creatorId,
|
||||||
|
alias: 'ted',
|
||||||
|
});
|
||||||
|
// Same row id — a RENAME, not a new insert.
|
||||||
|
expect(renamed.id).toBe(first.id);
|
||||||
|
expect(renamed.alias).toBe('ted');
|
||||||
|
|
||||||
|
const rows = await aliasRowsFor(pageId);
|
||||||
|
expect(rows).toHaveLength(1);
|
||||||
|
expect(rows[0].alias).toBe('ted'); // the stale `te` row is gone
|
||||||
|
|
||||||
|
// The modal read resolves the current (only) row deterministically.
|
||||||
|
const shown = await service.getAliasForPage(pageId, wsId);
|
||||||
|
expect(shown?.alias).toBe('ted');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('setAlias inserts the first alias, then is a no-op for the same name', async () => {
|
||||||
|
const pageId = await newPage();
|
||||||
|
const inserted = await service.setAlias({
|
||||||
|
workspaceId: wsId,
|
||||||
|
pageId,
|
||||||
|
creatorId: null as any,
|
||||||
|
alias: 'hello',
|
||||||
|
});
|
||||||
|
const again = await service.setAlias({
|
||||||
|
workspaceId: wsId,
|
||||||
|
pageId,
|
||||||
|
creatorId: null as any,
|
||||||
|
alias: 'hello',
|
||||||
|
});
|
||||||
|
expect(again.id).toBe(inserted.id);
|
||||||
|
expect(await aliasRowsFor(pageId)).toHaveLength(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('a mid-transaction error becomes BadRequestException and rolls back cleanly', async () => {
|
||||||
|
// A non-23505 failure inside the tx must surface as BadRequest AND leave NO
|
||||||
|
// partial alias state behind (the whole executeTx unit rolls back).
|
||||||
|
const pageId = await newPage();
|
||||||
|
const boom = new Error('disk on fire'); // not a unique-violation
|
||||||
|
// Wrap the real repo so the INSERT succeeds but the trailing self-heal
|
||||||
|
// throws — the row inserted earlier in the tx must not survive.
|
||||||
|
const flakyRepo = Object.create(repo);
|
||||||
|
flakyRepo.deleteOthersForPage = async () => {
|
||||||
|
throw boom;
|
||||||
|
};
|
||||||
|
const flakyService = new ShareAliasService(
|
||||||
|
flakyRepo as any,
|
||||||
|
pageRepo as any,
|
||||||
|
{} as any,
|
||||||
|
db as any,
|
||||||
|
);
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
flakyService.setAlias({
|
||||||
|
workspaceId: wsId,
|
||||||
|
pageId,
|
||||||
|
creatorId: null as any,
|
||||||
|
alias: 'rollback-me',
|
||||||
|
}),
|
||||||
|
).rejects.toBeInstanceOf(BadRequestException);
|
||||||
|
|
||||||
|
// Rolled back: neither the page nor the name has any row.
|
||||||
|
expect(await aliasRowsFor(pageId)).toHaveLength(0);
|
||||||
|
expect(
|
||||||
|
await db
|
||||||
|
.selectFrom('shareAliases')
|
||||||
|
.select('id')
|
||||||
|
.where('alias', '=', 'rollback-me')
|
||||||
|
.where('workspaceId', '=', wsId)
|
||||||
|
.execute(),
|
||||||
|
).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('cross-page collision throws 409, and confirmReassign moves the single row', async () => {
|
||||||
|
const pageA = await newPage();
|
||||||
|
const pageB = await newPage();
|
||||||
|
await service.setAlias({
|
||||||
|
workspaceId: wsId,
|
||||||
|
pageId: pageA,
|
||||||
|
creatorId: null as any,
|
||||||
|
alias: 'shared',
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
service.setAlias({
|
||||||
|
workspaceId: wsId,
|
||||||
|
pageId: pageB,
|
||||||
|
creatorId: null as any,
|
||||||
|
alias: 'shared',
|
||||||
|
}),
|
||||||
|
).rejects.toBeInstanceOf(ConflictException);
|
||||||
|
|
||||||
|
const moved = await service.setAlias({
|
||||||
|
workspaceId: wsId,
|
||||||
|
pageId: pageB,
|
||||||
|
creatorId: null as any,
|
||||||
|
alias: 'shared',
|
||||||
|
confirmReassign: true,
|
||||||
|
});
|
||||||
|
expect(moved.alias).toBe('shared');
|
||||||
|
|
||||||
|
// The name now belongs to pageB only; pageA has no alias.
|
||||||
|
expect(await aliasRowsFor(pageA)).toHaveLength(0);
|
||||||
|
const bRows = await aliasRowsFor(pageB);
|
||||||
|
expect(bRows).toHaveLength(1);
|
||||||
|
expect(bRows[0].alias).toBe('shared');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('confirmReassign onto a page that ALREADY has its own alias: target ends with ONE row', async () => {
|
||||||
|
// Regression guard for the operation-order bug: A has `shared`, B has its
|
||||||
|
// OWN alias `bee`. Moving `shared` onto B must FIRST drop B's `bee` row,
|
||||||
|
// THEN retarget, or the NON-deferrable (workspace_id, page_id) index fires a
|
||||||
|
// 23505 mid-transaction (two rows momentarily carry page_id = B) and the tx
|
||||||
|
// rolls back into a misleading "Alias already taken".
|
||||||
|
// Distinct names: the workspace is shared across tests, so reuse of an
|
||||||
|
// earlier test's `shared` would trip the 409 guard before we get here.
|
||||||
|
const pageA = await newPage();
|
||||||
|
const pageB = await newPage();
|
||||||
|
await service.setAlias({
|
||||||
|
workspaceId: wsId,
|
||||||
|
pageId: pageA,
|
||||||
|
creatorId: null as any,
|
||||||
|
alias: 'shared-target',
|
||||||
|
});
|
||||||
|
await service.setAlias({
|
||||||
|
workspaceId: wsId,
|
||||||
|
pageId: pageB,
|
||||||
|
creatorId: null as any,
|
||||||
|
alias: 'bee',
|
||||||
|
});
|
||||||
|
|
||||||
|
const moved = await service.setAlias({
|
||||||
|
workspaceId: wsId,
|
||||||
|
pageId: pageB,
|
||||||
|
creatorId: null as any,
|
||||||
|
alias: 'shared-target',
|
||||||
|
confirmReassign: true,
|
||||||
|
});
|
||||||
|
expect(moved.alias).toBe('shared-target');
|
||||||
|
|
||||||
|
// B now carries exactly `shared-target` (its old `bee` is gone); A has none.
|
||||||
|
const bRows = await aliasRowsFor(pageB);
|
||||||
|
expect(bRows).toHaveLength(1);
|
||||||
|
expect(bRows[0].alias).toBe('shared-target');
|
||||||
|
expect(await aliasRowsFor(pageA)).toHaveLength(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user