-
-
Notifications
You must be signed in to change notification settings - Fork 99
Replace url-template and uri-template-router with @fedify/uri-template
#758
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
base: main
Are you sure you want to change the base?
Changes from all commits
ad8d387
4b2e287
6429a2c
99a5854
b3772de
bb2890b
cbd2358
94fff72
075448d
93481fd
0e12d9a
924cb27
0aeb340
1d38ae6
1fd221e
31a1657
011363d
85f8a2d
aadf9ec
a3a12b7
3e973af
cd7bdc0
161e7e3
e0c1965
4e76477
2822715
b9a54ea
f06d731
c966c68
4579d54
96f4bed
ecaa080
c0f41b5
72a8945
e335fce
2f83766
b5157fe
0b0d828
6b00559
4088f51
b920d1b
a8c8c33
8af5855
a6afe59
37b158a
9e97462
12ef49d
2af7cc9
8ea3dd3
1cce8df
89e51c4
c9187c8
7ffac1b
4cd8125
80f6b04
0db4e14
c9e8fc9
4215d05
9b32a4e
715ada6
233d170
6dbb834
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,6 +123,7 @@ Here is the list of packages: | |
| | [@fedify/sqlite](/packages/sqlite/) | [JSR][jsr:@fedify/sqlite] | [npm][npm:@fedify/sqlite] | SQLite driver | | ||
| | [@fedify/sveltekit](/packages/sveltekit/) | [JSR][jsr:@fedify/sveltekit] | [npm][npm:@fedify/sveltekit] | SvelteKit integration | | ||
| | [@fedify/testing](/packages/testing/) | [JSR][jsr:@fedify/testing] | [npm][npm:@fedify/testing] | Testing utilities | | ||
| | [@fedify/uri-template](/packages/uri-template/) | [JSR][jsr:@fedify/uri-template] | [npm][npm:@fedify/uri-template] | RFC 6570 URI Template library | | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this PR adds |
||
| | [@fedify/vocab](/packages/vocab/) | [JSR][jsr:@fedify/vocab] | [npm][npm:@fedify/vocab] | Activity Vocabulary library | | ||
| | [@fedify/vocab-runtime](/packages/vocab-runtime/) | [JSR][jsr:@fedify/vocab-runtime] | [npm][npm:@fedify/vocab-runtime] | Runtime library for code-generated vocab | | ||
| | [@fedify/vocab-tools](/packages/vocab-tools/) | [JSR][jsr:@fedify/vocab-tools] | [npm][npm:@fedify/vocab-tools] | Code generation tools for Activity Vocab | | ||
|
|
@@ -176,6 +177,8 @@ Here is the list of packages: | |
| [npm:@fedify/sveltekit]: https://www.npmjs.com/package/@fedify/sveltekit | ||
| [jsr:@fedify/testing]: https://jsr.io/@fedify/testing | ||
| [npm:@fedify/testing]: https://www.npmjs.com/package/@fedify/testing | ||
| [jsr:@fedify/uri-template]: https://jsr.io/@fedify/uri-template | ||
| [npm:@fedify/uri-template]: https://www.npmjs.com/package/@fedify/uri-template | ||
| [jsr:@fedify/vocab]: https://jsr.io/@fedify/vocab | ||
| [npm:@fedify/vocab]: https://www.npmjs.com/package/@fedify/vocab | ||
| [jsr:@fedify/vocab-runtime]: https://jsr.io/@fedify/vocab-runtime | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import { test } from "@fedify/fixture"; | ||
| import { RouterError } from "@fedify/uri-template"; | ||
| import { Activity, Note, Person } from "@fedify/vocab"; | ||
| import { assertEquals, assertExists, assertThrows } from "@std/assert"; | ||
| import type { Protocol } from "../nodeinfo/types.ts"; | ||
|
|
@@ -13,7 +14,6 @@ import type { | |
| } from "./callback.ts"; | ||
| import { MemoryKvStore } from "./kv.ts"; | ||
| import type { FederationImpl } from "./middleware.ts"; | ||
| import { RouterError } from "./router.ts"; | ||
|
|
||
| test("FederationBuilder", async (t) => { | ||
| await t.step( | ||
|
|
@@ -160,6 +160,31 @@ test("FederationBuilder", async (t) => { | |
| }, | ||
| ); | ||
|
|
||
| await t.step("should snapshot router state on build", async () => { | ||
| const builder = createFederationBuilder<void>(); | ||
| const kv = new MemoryKvStore(); | ||
| const noteRouteName = `object:${Note.typeId.href}`; | ||
|
|
||
| builder.setActorDispatcher("/users/{identifier}", () => null); | ||
| const federation1 = await builder.build({ kv }); | ||
| const impl1 = federation1 as FederationImpl<void>; | ||
|
|
||
| builder.setObjectDispatcher(Note, "/notes/{id}", () => null); | ||
| assertEquals(impl1.router.route("/notes/1"), null); | ||
|
|
||
| const federation2 = await builder.build({ kv }); | ||
| const impl2 = federation2 as FederationImpl<void>; | ||
| assertEquals(impl2.router.route("/notes/1")?.name, noteRouteName); | ||
|
|
||
| impl1.router.add("/leaked/{id}", "leaked"); | ||
| assertEquals(impl1.router.route("/leaked/1")?.name, "leaked"); | ||
| assertEquals(impl2.router.route("/leaked/1"), null); | ||
|
|
||
| const federation3 = await builder.build({ kv }); | ||
| const impl3 = federation3 as FederationImpl<void>; | ||
| assertEquals(impl3.router.route("/leaked/1"), null); | ||
| }); | ||
|
|
||
| await t.step("should build with default options", async () => { | ||
| const builder = createFederationBuilder<void>(); | ||
| const kv = new MemoryKvStore(); | ||
|
|
@@ -211,10 +236,31 @@ test("FederationBuilder", async (t) => { | |
| ), | ||
| RouterError, | ||
| ); | ||
| assertThrows( | ||
| () => | ||
| builderAfterInvalid.setOutboxListeners( | ||
| "/users/{identifier:3}/outbox" as `${string}{identifier}${string}`, | ||
| ), | ||
| RouterError, | ||
| ); | ||
| assertThrows( | ||
| () => | ||
| builderAfterInvalid.setOutboxListeners( | ||
| "/users/{identifier*}/outbox" as `${string}{identifier}${string}`, | ||
| ), | ||
| RouterError, | ||
| ); | ||
| assertThrows( | ||
| () => | ||
| builderAfterInvalid.setOutboxListeners( | ||
| "/users/{identifier,identifier}/outbox" as `${string}{identifier}${string}`, | ||
| ), | ||
| RouterError, | ||
| ); | ||
| builderAfterInvalid.setOutboxListeners("/users/{identifier}/outbox"); | ||
|
|
||
| const builder2 = createFederationBuilder<void>(); | ||
| builder2.setOutboxListeners("/users{/identifier}/outbox"); | ||
| builder2.setOutboxListeners("/users/{identifier}/outbox"); | ||
|
|
||
| assertThrows( | ||
| () => | ||
|
|
@@ -231,6 +277,18 @@ test("FederationBuilder", async (t) => { | |
| RouterError, | ||
| ); | ||
|
|
||
| const builder3a = createFederationBuilder<void>(); | ||
| assertThrows( | ||
| () => builder3a.setOutboxListeners("/users{;identifier}/outbox"), | ||
| RouterError, | ||
| ); | ||
|
|
||
| const builder3b = createFederationBuilder<void>(); | ||
| assertThrows( | ||
| () => builder3b.setOutboxListeners("/users{.identifier}/outbox"), | ||
| RouterError, | ||
| ); | ||
|
|
||
| const builder4 = createFederationBuilder<void>(); | ||
| assertThrows( | ||
| () => | ||
|
|
@@ -240,8 +298,77 @@ test("FederationBuilder", async (t) => { | |
| ), | ||
| RouterError, | ||
| ); | ||
|
|
||
| const builder5 = createFederationBuilder<void>(); | ||
| assertThrows( | ||
| () => | ||
| builder5.setOutboxDispatcher( | ||
| "/users/{identifier:3}/outbox" as `${string}{identifier}${string}`, | ||
| () => ({ items: [] }), | ||
| ), | ||
| RouterError, | ||
| ); | ||
| }); | ||
|
|
||
| await t.step( | ||
| "should reject identifier paths that can match without an identifier", | ||
| () => { | ||
| // `{/identifier}` is path-style expansion that can match zero | ||
| // segments, so the route would match with an empty or missing | ||
| // `identifier`, violating the `identifier: string` callback contract. | ||
| // See https://github.com/fedify-dev/fedify/pull/758#discussion_r3252548632 | ||
| type IdPath = `${string}{identifier}${string}`; | ||
|
|
||
| assertThrows( | ||
| () => | ||
| createFederationBuilder<void>().setActorDispatcher( | ||
| "{/identifier}" as IdPath, | ||
| () => null, | ||
| ), | ||
| RouterError, | ||
| "Path for actor dispatcher must have one variable: {identifier}", | ||
| ); | ||
| assertThrows( | ||
| () => | ||
| createFederationBuilder<void>().setActorDispatcher( | ||
| "/users{/identifier}" as IdPath, | ||
| () => null, | ||
| ), | ||
| RouterError, | ||
| ); | ||
| assertThrows( | ||
| () => | ||
| createFederationBuilder<void>().setInboxListeners( | ||
| "{/identifier}/inbox" as IdPath, | ||
| ), | ||
| RouterError, | ||
| ); | ||
| assertThrows( | ||
| () => | ||
| createFederationBuilder<void>().setInboxListeners( | ||
| "/users{/identifier}/inbox" as IdPath, | ||
| ), | ||
| RouterError, | ||
| ); | ||
| assertThrows( | ||
| () => | ||
| createFederationBuilder<void>().setOutboxListeners( | ||
| "/users{/identifier}/outbox" as IdPath, | ||
| ), | ||
| RouterError, | ||
| ); | ||
|
|
||
| // Simple expansion `{identifier}` must keep working. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This keeps the common The previous router was not exactly safe here either, but this PR changes the observable value shape from a missing binding to an empty string. If the intended Fedify contract is that required actor/inbox/outbox identifiers are non-empty, this should be enforced either by the builder/middleware dispatch path or by a regression test that |
||
| createFederationBuilder<void>().setActorDispatcher( | ||
| "/users/{identifier}", | ||
| () => null, | ||
| ); | ||
| createFederationBuilder<void>().setInboxListeners( | ||
| "/users/{identifier}/inbox", | ||
| ); | ||
| }, | ||
| ); | ||
|
|
||
| await t.step("should pass build options correctly", async () => { | ||
| const builder = createFederationBuilder<number>(); | ||
| const kv = new MemoryKvStore(); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.