diff --git a/sdk/dart/CHANGELOG.md b/sdk/dart/CHANGELOG.md index 2822b51..d67d8f3 100644 --- a/sdk/dart/CHANGELOG.md +++ b/sdk/dart/CHANGELOG.md @@ -1,5 +1,22 @@ # Changelog +## 2.0.0 (2026-05-07) + +**Breaking change to `update()` methods.** `LinksResource.update` and `BundlesResource.update` now take a `Link` / `Bundle` object instead of an id with optional named parameters. To edit a record, call `copyWith` on the value returned by `get()` (or any other read), then pass it to `update()`. + +```dart +// 1.x +await client.links.update(42, label: null); + +// 2.0 +final link = await client.links.get(42); +await client.links.update(link.copyWith(label: null)); +``` + +`Link`, `Bundle`, and `BundleWithSummary` now expose a `copyWith` method. Pass `null` to a nullable field to clear it; omit the parameter to preserve the current value. The disambiguation between "omit" and "explicit null" is encapsulated in `copyWith` via a private sentinel, so callers never see it. `update()` always sends the full writable payload, which sidesteps the omit-vs-clear ambiguity at the wire level. + +The Python and TypeScript SDKs keep their existing partial-update shapes; each language implements the same set/clear/omit feature in its own idiom. + ## 1.0.1 (2026-04-30) Packaging and documentation only. No public surface changes. diff --git a/sdk/dart/lib/src/models.dart b/sdk/dart/lib/src/models.dart index 4bddde8..b026f85 100644 --- a/sdk/dart/lib/src/models.dart +++ b/sdk/dart/lib/src/models.dart @@ -3,6 +3,17 @@ import 'package:meta/meta.dart'; +// ---- copyWith sentinel ---- + +// Distinguishes "argument not passed" from "argument explicitly null" inside +// copyWith. A private named class is used (not `const Object()`) so the +// sentinel cannot be matched by any value an external caller could construct. +class _Unset { + const _Unset(); +} + +const _Unset _unset = _Unset(); + // ---- Enum types ---- /// Accent color applied to a bundle. @@ -240,6 +251,36 @@ class Link { /// Click count change as a percentage versus the previous equivalent period. /// Absent when comparison data is unavailable. final double? deltaPct; + + /// Returns a copy with selected fields replaced. + /// + /// Pass `null` to a nullable field to clear it. Omit the parameter to + /// preserve the current value. + Link copyWith({ + int? id, + String? url, + Object? label = _unset, + int? createdAt, + Object? expiresAt = _unset, + Object? createdVia = _unset, + String? createdBy, + List? slugs, + int? totalClicks, + Object? deltaPct = _unset, + }) { + return Link( + id: id ?? this.id, + url: url ?? this.url, + label: identical(label, _unset) ? this.label : label as String?, + createdAt: createdAt ?? this.createdAt, + expiresAt: identical(expiresAt, _unset) ? this.expiresAt : expiresAt as int?, + createdVia: identical(createdVia, _unset) ? this.createdVia : createdVia as String?, + createdBy: createdBy ?? this.createdBy, + slugs: slugs ?? this.slugs, + totalClicks: totalClicks ?? this.totalClicks, + deltaPct: identical(deltaPct, _unset) ? this.deltaPct : deltaPct as double?, + ); + } } /// A collection of links grouped to show combined engagement. @@ -302,6 +343,36 @@ class Bundle { /// Unix seconds when the bundle was last modified. final int updatedAt; + + /// Returns a copy with selected fields replaced. + /// + /// Pass `null` to a nullable field to clear it. Omit the parameter to + /// preserve the current value. + Bundle copyWith({ + int? id, + String? name, + Object? description = _unset, + Object? icon = _unset, + BundleAccent? accent, + Object? archivedAt = _unset, + Object? createdVia = _unset, + String? createdBy, + int? createdAt, + int? updatedAt, + }) { + return Bundle( + id: id ?? this.id, + name: name ?? this.name, + description: identical(description, _unset) ? this.description : description as String?, + icon: identical(icon, _unset) ? this.icon : icon as String?, + accent: accent ?? this.accent, + archivedAt: identical(archivedAt, _unset) ? this.archivedAt : archivedAt as int?, + createdVia: identical(createdVia, _unset) ? this.createdVia : createdVia as String?, + createdBy: createdBy ?? this.createdBy, + createdAt: createdAt ?? this.createdAt, + updatedAt: updatedAt ?? this.updatedAt, + ); + } } /// Bundle enriched with range-scoped summary data. @@ -367,6 +438,47 @@ class BundleWithSummary extends Bundle { /// Top member links by click count. final List topLinks; + + /// Returns a copy with selected fields replaced. + /// + /// Pass `null` to a nullable field to clear it. Omit the parameter to + /// preserve the current value. + @override + BundleWithSummary copyWith({ + int? id, + String? name, + Object? description = _unset, + Object? icon = _unset, + BundleAccent? accent, + Object? archivedAt = _unset, + Object? createdVia = _unset, + String? createdBy, + int? createdAt, + int? updatedAt, + int? linkCount, + int? totalClicks, + Object? deltaPct = _unset, + List? sparkline, + List? topLinks, + }) { + return BundleWithSummary( + id: id ?? this.id, + name: name ?? this.name, + description: identical(description, _unset) ? this.description : description as String?, + icon: identical(icon, _unset) ? this.icon : icon as String?, + accent: accent ?? this.accent, + archivedAt: identical(archivedAt, _unset) ? this.archivedAt : archivedAt as int?, + createdVia: identical(createdVia, _unset) ? this.createdVia : createdVia as String?, + createdBy: createdBy ?? this.createdBy, + createdAt: createdAt ?? this.createdAt, + updatedAt: updatedAt ?? this.updatedAt, + linkCount: linkCount ?? this.linkCount, + totalClicks: totalClicks ?? this.totalClicks, + deltaPct: identical(deltaPct, _unset) ? this.deltaPct : deltaPct as double?, + sparkline: sparkline ?? this.sparkline, + topLinks: topLinks ?? this.topLinks, + ); + } } /// Top-link entry preview shown in a [BundleWithSummary]. diff --git a/sdk/dart/lib/src/resources/bundles.dart b/sdk/dart/lib/src/resources/bundles.dart index 71144ef..f698769 100644 --- a/sdk/dart/lib/src/resources/bundles.dart +++ b/sdk/dart/lib/src/resources/bundles.dart @@ -52,21 +52,24 @@ class BundlesResource { return Bundle.fromJson(json! as Map); } - /// Update a bundle's name, description, icon, or accent. - Future update( - int id, { - String? name, - String? description, - String? icon, - BundleAccent? accent, - }) async { - final body = {}; - if (name != null) body['name'] = name; - if (description != null) body['description'] = description; - if (icon != null) body['icon'] = icon; - if (accent != null) body['accent'] = accent.wireValue; + /// Update a bundle with the values held by [bundle]. + /// + /// The writable fields ([Bundle.name], [Bundle.description], [Bundle.icon], + /// [Bundle.accent]) are always sent on the wire; pass a [Bundle] produced + /// by [Bundle.copyWith] with the desired changes (use `null` on a nullable + /// field to clear it). [BundleWithSummary] is also accepted because it + /// extends [Bundle]; only the writable fields above are put on the wire, + /// so the summary fields never reach the server (whose update schema is + /// strict and would reject them). + Future update(Bundle bundle) async { + final body = { + 'name': bundle.name, + 'description': bundle.description, + 'icon': bundle.icon, + 'accent': bundle.accent.wireValue, + }; final json = - await _http.requestJson('PUT', '/_/api/bundles/$id', body: body); + await _http.requestJson('PUT', '/_/api/bundles/${bundle.id}', body: body); return Bundle.fromJson(json! as Map); } diff --git a/sdk/dart/lib/src/resources/links.dart b/sdk/dart/lib/src/resources/links.dart index 3435602..56eda96 100644 --- a/sdk/dart/lib/src/resources/links.dart +++ b/sdk/dart/lib/src/resources/links.dart @@ -50,18 +50,21 @@ class LinksResource { return Link.fromJson(json! as Map); } - /// Update a link's URL, label, or expiry. - Future update( - int id, { - String? url, - String? label, - int? expiresAt, - }) async { - final body = {}; - if (url != null) body['url'] = url; - if (label != null) body['label'] = label; - if (expiresAt != null) body['expires_at'] = expiresAt; - final json = await _http.requestJson('PUT', '/_/api/links/$id', body: body); + /// Update a link with the values held by [link]. + /// + /// The writable fields ([Link.url], [Link.label], [Link.expiresAt]) are + /// always sent on the wire; pass a [Link] produced by [Link.copyWith] with + /// the desired changes (use `null` on a nullable field to clear it). + /// Read-only fields like [Link.id] are used for routing; server-managed + /// fields like slugs and click counts are not included in the request body + /// (the server's update schema is strict and would reject them). + Future update(Link link) async { + final body = { + 'url': link.url, + 'label': link.label, + 'expires_at': link.expiresAt, + }; + final json = await _http.requestJson('PUT', '/_/api/links/${link.id}', body: body); return Link.fromJson(json! as Map); } diff --git a/sdk/dart/pubspec.yaml b/sdk/dart/pubspec.yaml index 829a5e5..168c76d 100644 --- a/sdk/dart/pubspec.yaml +++ b/sdk/dart/pubspec.yaml @@ -1,10 +1,10 @@ name: shrtnr description: Dart client for the shrtnr URL shortener API. Create short links, manage custom slugs, and read click analytics. -version: 1.0.1 +version: 2.0.0 homepage: https://oddb.it/shrtnr-website-pub repository: https://github.com/oddbit/shrtnr issue_tracker: https://github.com/oddbit/shrtnr/issues -# x-spec-hash: sha256:3b6e9163bac619b9bbef7ba774b2cd06a9a968f1223b85ba302a018c4bee3b57 +# x-spec-hash: sha256:0d54031654375b3a269cd23d05d66ff44a4b10a894d09b55ed2c1c66405c25b8 # Stored as a comment rather than a top-level key to avoid a pana score deduction for unknown manifest fields. topics: diff --git a/sdk/dart/test/client_test.dart b/sdk/dart/test/client_test.dart index 147ebb3..d4d6b2a 100644 --- a/sdk/dart/test/client_test.dart +++ b/sdk/dart/test/client_test.dart @@ -308,13 +308,78 @@ void main() { // ---- 6. links.update ---- group('links.update', () { - test('PUTs /_/api/links/:id with patch body', () async { + test('PUTs /_/api/links/:id with full writable payload', () async { final m = _mock(status: 200, body: _linkJson(url: 'https://new.com')); - await m.client.links.update(1, url: 'https://new.com'); + final original = Link.fromJson(_linkJson(id: 1, url: 'https://old.com')); + await m.client.links.update(original.copyWith(url: 'https://new.com')); expect(m.capture.request!.url.toString(), '$_base/_/api/links/1'); expect(m.capture.request!.method, 'PUT'); final body = jsonDecode(m.capture.request!.body) as Map; expect(body['url'], 'https://new.com'); + // All writable fields are present in the wire payload. + expect(body.containsKey('label'), isTrue); + expect(body.containsKey('expires_at'), isTrue); + }); + + test('copyWith(label: null) sends label: null to clear', () async { + final m = _mock(status: 200, body: _linkJson()); + final original = Link.fromJson({ + ..._linkJson(id: 1), + 'label': 'old label', + }); + await m.client.links.update(original.copyWith(label: null)); + final body = jsonDecode(m.capture.request!.body) as Map; + expect(body['label'], isNull); + }); + + test('copyWith preserves unchanged label so the server keeps the old value', + () async { + final m = _mock(status: 200, body: _linkJson()); + final original = Link.fromJson({ + ..._linkJson(id: 1), + 'label': 'preserve me', + }); + await m.client.links.update(original.copyWith(url: 'https://new.com')); + final body = jsonDecode(m.capture.request!.body) as Map; + expect(body['label'], 'preserve me'); + }); + }); + + // ---- 6a. Link.copyWith semantics ---- + + group('Link.copyWith', () { + test('omitted parameters preserve the existing value', () { + final link = Link.fromJson({ + ..._linkJson(id: 5, url: 'https://orig.com'), + 'label': 'keep', + 'expires_at': 9999999, + }); + final copy = link.copyWith(); + expect(copy.id, 5); + expect(copy.url, 'https://orig.com'); + expect(copy.label, 'keep'); + expect(copy.expiresAt, 9999999); + }); + + test('passing null clears nullable fields', () { + final link = Link.fromJson({ + ..._linkJson(id: 5), + 'label': 'old', + 'expires_at': 9999999, + }); + final copy = link.copyWith(label: null, expiresAt: null); + expect(copy.label, isNull); + expect(copy.expiresAt, isNull); + }); + + test('passing a value overrides only that field', () { + final link = Link.fromJson({ + ..._linkJson(id: 5, url: 'https://orig.com'), + 'label': 'keep', + }); + final copy = link.copyWith(url: 'https://new.com'); + expect(copy.url, 'https://new.com'); + expect(copy.label, 'keep'); }); }); @@ -641,13 +706,90 @@ void main() { // ---- 22. bundles.update ---- group('bundles.update', () { - test('PUTs /_/api/bundles/:id with patch body', () async { + test('PUTs /_/api/bundles/:id with full writable payload', () async { final m = _mock(status: 200, body: _bundleJson(name: 'Updated')); - await m.client.bundles.update(42, name: 'Updated'); + final original = Bundle.fromJson(_bundleJson(id: 42, name: 'Old')); + await m.client.bundles.update(original.copyWith(name: 'Updated')); expect(m.capture.request!.url.toString(), '$_base/_/api/bundles/42'); expect(m.capture.request!.method, 'PUT'); final body = jsonDecode(m.capture.request!.body) as Map; expect(body['name'], 'Updated'); + // Full writable payload: all four fields present. + expect(body.containsKey('description'), isTrue); + expect(body.containsKey('icon'), isTrue); + expect(body['accent'], 'orange'); + }); + + test('copyWith(description: null) sends description: null to clear', () async { + final m = _mock(status: 200, body: _bundleJson()); + final original = Bundle.fromJson({ + ..._bundleJson(id: 42), + 'description': 'old desc', + }); + await m.client.bundles.update(original.copyWith(description: null)); + final body = jsonDecode(m.capture.request!.body) as Map; + expect(body['description'], isNull); + }); + + test('copyWith preserves the description so the server keeps the old value', + () async { + final m = _mock(status: 200, body: _bundleJson()); + final original = Bundle.fromJson({ + ..._bundleJson(id: 42), + 'description': 'preserve me', + }); + await m.client.bundles.update(original.copyWith(name: 'Renamed')); + final body = jsonDecode(m.capture.request!.body) as Map; + expect(body['description'], 'preserve me'); + }); + + test('accepts a BundleWithSummary (covariant)', () async { + final m = _mock(status: 200, body: _bundleJson(name: 'Updated')); + final summary = BundleWithSummary.fromJson(_bundleWithSummaryJson(id: 42)); + await m.client.bundles.update(summary.copyWith(name: 'Updated')); + final body = jsonDecode(m.capture.request!.body) as Map; + expect(body['name'], 'Updated'); + // Summary fields like link_count are not part of the wire payload. + expect(body.containsKey('link_count'), isFalse); + }); + }); + + // ---- 22a. Bundle.copyWith semantics ---- + + group('Bundle.copyWith', () { + test('omitted parameters preserve the existing value', () { + final bundle = Bundle.fromJson({ + ..._bundleJson(id: 42, name: 'orig', accent: 'blue'), + 'description': 'desc', + 'icon': 'star', + }); + final copy = bundle.copyWith(); + expect(copy.id, 42); + expect(copy.name, 'orig'); + expect(copy.description, 'desc'); + expect(copy.icon, 'star'); + expect(copy.accent, BundleAccent.blue); + }); + + test('passing null clears nullable fields', () { + final bundle = Bundle.fromJson({ + ..._bundleJson(id: 42), + 'description': 'old', + 'icon': 'star', + }); + final copy = bundle.copyWith(description: null, icon: null); + expect(copy.description, isNull); + expect(copy.icon, isNull); + }); + + test('BundleWithSummary.copyWith returns BundleWithSummary preserving summary', + () { + final summary = BundleWithSummary.fromJson(_bundleWithSummaryJson(id: 42)); + final copy = summary.copyWith(name: 'Renamed'); + expect(copy, isA()); + expect(copy.name, 'Renamed'); + expect(copy.linkCount, 3); + expect(copy.totalClicks, 100); }); }); diff --git a/sdk/python/pyproject.toml b/sdk/python/pyproject.toml index 2c79562..c5806bb 100644 --- a/sdk/python/pyproject.toml +++ b/sdk/python/pyproject.toml @@ -92,7 +92,7 @@ warn_redundant_casts = true warn_unreachable = true [tool.shrtnr] -spec_hash = "sha256:3b6e9163bac619b9bbef7ba774b2cd06a9a968f1223b85ba302a018c4bee3b57" +spec_hash = "sha256:0d54031654375b3a269cd23d05d66ff44a4b10a894d09b55ed2c1c66405c25b8" [tool.pytest.ini_options] testpaths = ["tests"] diff --git a/sdk/python/src/shrtnr/_base.py b/sdk/python/src/shrtnr/_base.py index 2599be4..36e7c65 100644 --- a/sdk/python/src/shrtnr/_base.py +++ b/sdk/python/src/shrtnr/_base.py @@ -20,6 +20,23 @@ DEFAULT_TIMEOUT = 30.0 +class _UnsetType: + """Sentinel distinguishing 'not provided' from an explicit ``None``.""" + + _instance: _UnsetType | None = None + + def __new__(cls) -> _UnsetType: + if cls._instance is None: + cls._instance = super().__new__(cls) + return cls._instance + + def __repr__(self) -> str: + return "UNSET" + + +UNSET: Any = _UnsetType() + + def _build_auth_headers(api_key: str) -> dict[str, str]: return {"Authorization": f"Bearer {api_key}"} diff --git a/sdk/python/src/shrtnr/resources/bundles.py b/sdk/python/src/shrtnr/resources/bundles.py index b5a739f..6d121d9 100644 --- a/sdk/python/src/shrtnr/resources/bundles.py +++ b/sdk/python/src/shrtnr/resources/bundles.py @@ -15,6 +15,7 @@ import httpx from .._base import ( + UNSET, _build_auth_headers, _build_url, parse_json_response, @@ -97,17 +98,21 @@ def update( id: int, *, name: str | None = None, - description: str | None = None, - icon: str | None = None, + description: str | None = UNSET, + icon: str | None = UNSET, accent: BundleAccent | None = None, ) -> Bundle: - """Update a bundle's name, description, icon, or accent.""" + """Update a bundle's name, description, icon, or accent. + + Pass ``None`` for *description* or *icon* to clear the field. + Omitting a parameter leaves the field unchanged. + """ body: dict[str, Any] = {} if name is not None: body["name"] = name - if description is not None: + if description is not UNSET: body["description"] = description - if icon is not None: + if icon is not UNSET: body["icon"] = icon if accent is not None: body["accent"] = accent @@ -219,17 +224,21 @@ async def update( id: int, *, name: str | None = None, - description: str | None = None, - icon: str | None = None, + description: str | None = UNSET, + icon: str | None = UNSET, accent: BundleAccent | None = None, ) -> Bundle: - """Update a bundle's name, description, icon, or accent.""" + """Update a bundle's name, description, icon, or accent. + + Pass ``None`` for *description* or *icon* to clear the field. + Omitting a parameter leaves the field unchanged. + """ body: dict[str, Any] = {} if name is not None: body["name"] = name - if description is not None: + if description is not UNSET: body["description"] = description - if icon is not None: + if icon is not UNSET: body["icon"] = icon if accent is not None: body["accent"] = accent diff --git a/sdk/python/src/shrtnr/resources/links.py b/sdk/python/src/shrtnr/resources/links.py index d55767d..a50c835 100644 --- a/sdk/python/src/shrtnr/resources/links.py +++ b/sdk/python/src/shrtnr/resources/links.py @@ -15,6 +15,7 @@ import httpx from .._base import ( + UNSET, _build_auth_headers, _build_url, parse_json_response, @@ -102,16 +103,20 @@ def update( id: int, *, url: str | None = None, - label: str | None = None, - expires_at: int | None = None, + label: str | None = UNSET, + expires_at: int | None = UNSET, ) -> Link: - """Update a link's URL, label, or expiry.""" + """Update a link's URL, label, or expiry. + + Pass ``None`` for *label* or *expires_at* to clear the field. + Omitting a parameter leaves the field unchanged. + """ body: dict[str, Any] = {} if url is not None: body["url"] = url - if label is not None: + if label is not UNSET: body["label"] = label - if expires_at is not None: + if expires_at is not UNSET: body["expires_at"] = expires_at req_url = self._url(f"/_/api/links/{id}") return Link.from_dict( @@ -233,16 +238,20 @@ async def update( id: int, *, url: str | None = None, - label: str | None = None, - expires_at: int | None = None, + label: str | None = UNSET, + expires_at: int | None = UNSET, ) -> Link: - """Update a link's URL, label, or expiry.""" + """Update a link's URL, label, or expiry. + + Pass ``None`` for *label* or *expires_at* to clear the field. + Omitting a parameter leaves the field unchanged. + """ body: dict[str, Any] = {} if url is not None: body["url"] = url - if label is not None: + if label is not UNSET: body["label"] = label - if expires_at is not None: + if expires_at is not UNSET: body["expires_at"] = expires_at req_url = self._url(f"/_/api/links/{id}") return Link.from_dict( diff --git a/sdk/python/tests/test_async_client.py b/sdk/python/tests/test_async_client.py index b2ef2c4..cc64ffa 100644 --- a/sdk/python/tests/test_async_client.py +++ b/sdk/python/tests/test_async_client.py @@ -118,6 +118,19 @@ async def test_async_links_update(client: AsyncShrtnr) -> None: assert link.id == 1 +@respx.mock +async def test_async_links_update_clears_label_with_none(client: AsyncShrtnr) -> None: + import json as _json + + route = respx.put(f"{BASE_URL}/_/api/links/1").mock( + return_value=httpx.Response(200, json=make_link_dict()), + ) + await client.links.update(1, label=None) + body = _json.loads(route.calls[0].request.content) + assert "label" in body + assert body["label"] is None + + @respx.mock async def test_async_links_disable(client: AsyncShrtnr) -> None: route = respx.post(f"{BASE_URL}/_/api/links/1/disable").mock( @@ -274,6 +287,19 @@ async def test_async_bundles_update(client: AsyncShrtnr) -> None: assert b.id == 42 +@respx.mock +async def test_async_bundles_update_clears_description_with_none(client: AsyncShrtnr) -> None: + import json as _json + + route = respx.put(f"{BASE_URL}/_/api/bundles/42").mock( + return_value=httpx.Response(200, json=make_bundle_dict()), + ) + await client.bundles.update(42, description=None) + body = _json.loads(route.calls[0].request.content) + assert "description" in body + assert body["description"] is None + + @respx.mock async def test_async_bundles_delete(client: AsyncShrtnr) -> None: respx.delete(f"{BASE_URL}/_/api/bundles/42").mock( diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index a90704c..0a22991 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -182,6 +182,38 @@ def test_links_update(client: Shrtnr) -> None: assert body == {"url": "https://new.com"} +@respx.mock +def test_links_update_clears_label_with_none(client: Shrtnr) -> None: + route = respx.put(f"{BASE_URL}/_/api/links/1").mock( + return_value=httpx.Response(200, json=make_link_dict(link_id=1)), + ) + client.links.update(1, label=None) + body = json.loads(route.calls[0].request.content) + assert "label" in body + assert body["label"] is None + + +@respx.mock +def test_links_update_omits_label_when_not_provided(client: Shrtnr) -> None: + route = respx.put(f"{BASE_URL}/_/api/links/1").mock( + return_value=httpx.Response(200, json=make_link_dict(link_id=1)), + ) + client.links.update(1, url="https://new.com") + body = json.loads(route.calls[0].request.content) + assert "label" not in body + + +@respx.mock +def test_links_update_clears_expires_at_with_none(client: Shrtnr) -> None: + route = respx.put(f"{BASE_URL}/_/api/links/1").mock( + return_value=httpx.Response(200, json=make_link_dict(link_id=1)), + ) + client.links.update(1, expires_at=None) + body = json.loads(route.calls[0].request.content) + assert "expires_at" in body + assert body["expires_at"] is None + + # ---- links.disable ---- @@ -455,6 +487,27 @@ def test_bundles_update(client: Shrtnr) -> None: assert body == {"description": "edited"} +@respx.mock +def test_bundles_update_clears_description_with_none(client: Shrtnr) -> None: + route = respx.put(f"{BASE_URL}/_/api/bundles/42").mock( + return_value=httpx.Response(200, json=make_bundle_dict()), + ) + client.bundles.update(42, description=None) + body = json.loads(route.calls[0].request.content) + assert "description" in body + assert body["description"] is None + + +@respx.mock +def test_bundles_update_omits_description_when_not_provided(client: Shrtnr) -> None: + route = respx.put(f"{BASE_URL}/_/api/bundles/42").mock( + return_value=httpx.Response(200, json=make_bundle_dict()), + ) + client.bundles.update(42, name="New Name") + body = json.loads(route.calls[0].request.content) + assert "description" not in body + + # ---- bundles.delete ---- diff --git a/sdk/typescript/package.json b/sdk/typescript/package.json index 320944b..1471c5c 100644 --- a/sdk/typescript/package.json +++ b/sdk/typescript/package.json @@ -51,5 +51,5 @@ "click-analytics", "custom-slug" ], - "x-spec-hash": "sha256:3b6e9163bac619b9bbef7ba774b2cd06a9a968f1223b85ba302a018c4bee3b57" + "x-spec-hash": "sha256:0d54031654375b3a269cd23d05d66ff44a4b10a894d09b55ed2c1c66405c25b8" } diff --git a/sdk/typescript/src/resources/links.ts b/sdk/typescript/src/resources/links.ts index 7cd7158..871f781 100644 --- a/sdk/typescript/src/resources/links.ts +++ b/sdk/typescript/src/resources/links.ts @@ -70,10 +70,10 @@ export class LinksResource { } /** Get QR code SVG for a link. Returns the SVG string. */ - qr(id: number, options: { slug?: string; size?: string } = {}): Promise { + qr(id: number, options: { slug?: string; size?: number } = {}): Promise { return this.http.requestText("GET", `/_/api/links/${id}/qr`, { slug: options.slug, - size: options.size, + size: options.size !== undefined ? String(options.size) : undefined, }); } diff --git a/sdk/typescript/tests/client.test.ts b/sdk/typescript/tests/client.test.ts index d867614..43cf1e3 100644 --- a/sdk/typescript/tests/client.test.ts +++ b/sdk/typescript/tests/client.test.ts @@ -254,6 +254,24 @@ describe("links.update", () => { expect(url).toBe(`${BASE}/_/api/links/1`); expect(init.method).toBe("PUT"); }); + + it("sends label: null to clear the label", async () => { + mockFetch(200, stubLink); + await client().links.update(1, { label: null }); + const { init } = lastCall(); + const body = JSON.parse(init.body as string) as Record; + expect(body).toHaveProperty("label"); + expect(body["label"]).toBeNull(); + }); + + it("sends expires_at: null to clear expiry", async () => { + mockFetch(200, stubLink); + await client().links.update(1, { expiresAt: null }); + const { init } = lastCall(); + const body = JSON.parse(init.body as string) as Record; + expect(body).toHaveProperty("expires_at"); + expect(body["expires_at"]).toBeNull(); + }); }); describe("links.disable", () => { @@ -357,9 +375,9 @@ describe("links.qr", () => { expect(lastCall().url).toBe(`${BASE}/_/api/links/5/qr`); }); - it("appends slug and size when provided", async () => { + it("appends slug and size when provided, converting number to string", async () => { mockFetch(200, "", "image/svg+xml"); - await client().links.qr(5, { slug: "promo", size: "256" }); + await client().links.qr(5, { slug: "promo", size: 256 }); expect(lastCall().url).toBe(`${BASE}/_/api/links/5/qr?slug=promo&size=256`); }); }); @@ -515,6 +533,15 @@ describe("bundles.update", () => { expect(url).toBe(`${BASE}/_/api/bundles/42`); expect(init.method).toBe("PUT"); }); + + it("sends description: null to clear the description", async () => { + mockFetch(200, stubBundle); + await client().bundles.update(42, { description: null }); + const { init } = lastCall(); + const body = JSON.parse(init.body as string) as Record; + expect(body).toHaveProperty("description"); + expect(body["description"]).toBeNull(); + }); }); describe("bundles.delete", () => { diff --git a/src/__tests__/handler/api-links.test.ts b/src/__tests__/handler/api-links.test.ts index 8ba0ca8..559af3f 100644 --- a/src/__tests__/handler/api-links.test.ts +++ b/src/__tests__/handler/api-links.test.ts @@ -1180,3 +1180,111 @@ describe("Link+slug access model (design): anyone reads, anyone adds slugs, only expect(enable.status).toBe(403); }); }); + +describe("GET /_/api/links/:id/qr size validation", () => { + async function createTestLink(): Promise { + const res = await SELF.fetch( + authed("/_/admin/api/links", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ url: "https://example.com" }), + }) + ); + expect(res.status).toBe(201); + const body = await res.json() as { id: number }; + return body.id; + } + + it("rejects size=0 with 400", async () => { + const id = await createTestLink(); + const key = await seedApiKey(env.DB, "read"); + const res = await SELF.fetch(new Request(`https://shrtnr.test/_/api/links/${id}/qr?size=0`, { + headers: { Authorization: `Bearer ${key}` }, + })); + expect(res.status).toBe(400); + }); + + it("rejects negative size with 400", async () => { + const id = await createTestLink(); + const res = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr?size=-5`)); + expect(res.status).toBe(400); + }); + + it("rejects non-numeric size with 400", async () => { + const id = await createTestLink(); + const res = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr?size=abc`)); + expect(res.status).toBe(400); + }); + + it("rejects size above 2048 with 400 on the OpenAPI route", async () => { + const id = await createTestLink(); + const key = await seedApiKey(env.DB, "read"); + const res = await SELF.fetch(new Request(`https://shrtnr.test/_/api/links/${id}/qr?size=2049`, { + headers: { Authorization: `Bearer ${key}` }, + })); + expect(res.status).toBe(400); + }); + + it("rejects size above 2048 with 400 on the admin route", async () => { + const id = await createTestLink(); + const res = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr?size=2049`)); + expect(res.status).toBe(400); + }); + + it("rejects empty slug with 400 on the admin route", async () => { + const id = await createTestLink(); + const res = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr?slug=`)); + expect(res.status).toBe(400); + }); + + it("accepts size=300 and returns SVG sized to 300", async () => { + const id = await createTestLink(); + const res = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr?size=300`)); + expect(res.status).toBe(200); + const svg = await res.text(); + expect(svg).toMatch(/width="300"/); + }); + + it("accepts no size and returns SVG sized to default 220", async () => { + const id = await createTestLink(); + const res = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr`)); + expect(res.status).toBe(200); + const svg = await res.text(); + expect(svg).toMatch(/width="220"/); + }); + + it("defaults to the link's primary slug, not just any custom slug", async () => { + // Setup: link with auto-slug ("primary"), plus a custom slug that is + // explicitly NOT primary. Default-pick must pick the auto-slug. + const createRes = await SELF.fetch( + authed("/_/admin/api/links", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ url: "https://example.com/primary-pick" }), + }) + ); + expect(createRes.status).toBe(201); + const created = await createRes.json() as { id: number; slugs: { slug: string }[] }; + const id = created.id; + const autoSlug = created.slugs[0].slug; + + const addRes = await SELF.fetch(authed(`/_/admin/api/links/${id}/slugs`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ slug: "my-custom-pick" }), + })); + expect(addRes.status).toBe(201); + + // addCustom flipped primary to "my-custom-pick"; flip it back so the + // test exercises the bug shape: primary != custom. + await env.DB.prepare("UPDATE slugs SET is_primary = (slug = ?) WHERE link_id = ?") + .bind(autoSlug, id) + .run(); + + const noSlug = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr`)); + const explicitPrimary = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr?slug=${autoSlug}`)); + expect(noSlug.status).toBe(200); + expect(explicitPrimary.status).toBe(200); + expect(await noSlug.text()).toBe(await explicitPrimary.text()); + }); +}); diff --git a/src/__tests__/handler/redirect-kv.test.ts b/src/__tests__/handler/redirect-kv.test.ts index 0cc6a62..1c72352 100644 --- a/src/__tests__/handler/redirect-kv.test.ts +++ b/src/__tests__/handler/redirect-kv.test.ts @@ -94,11 +94,12 @@ describe("Redirect with KV cache", () => { describe("KV write-through on mutations", () => { it("populates KV when a link is created", async () => { + // label suppresses autoLabelLink's outbound fetch; without it the next beforeEach times out on CI. const res = await SELF.fetch( new Request("https://shrtnr.test/_/admin/api/links", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ url: "https://new-link.com/page" }), + body: JSON.stringify({ url: "https://new-link.com/page", label: "preset" }), }), ); expect(res.status).toBe(201); diff --git a/src/__tests__/service/link-service.test.ts b/src/__tests__/service/link-service.test.ts index bda1d9d..0ba7362 100644 --- a/src/__tests__/service/link-service.test.ts +++ b/src/__tests__/service/link-service.test.ts @@ -300,3 +300,41 @@ describe("URL normalization in createLink", () => { expect(result.data.url).toBe("https://example.com/path"); }); }); + +describe("URL normalization in updateLink", () => { + it("strips trailing slash from updated URL", async () => { + const created = await createLink(env as any, { url: "https://example.com" }); + expect(created.ok).toBe(true); + if (!created.ok) return; + + const updated = await updateLink(env as any, created.data.id, { url: "https://example.com/new-path/" }); + expect(updated.ok).toBe(true); + if (updated.ok) { + expect(updated.data.url).toBe("https://example.com/new-path"); + } + }); + + it("strips trailing question mark from updated URL", async () => { + const created = await createLink(env as any, { url: "https://example.com" }); + expect(created.ok).toBe(true); + if (!created.ok) return; + + const updated = await updateLink(env as any, created.data.id, { url: "https://example.com/page?" }); + expect(updated.ok).toBe(true); + if (updated.ok) { + expect(updated.data.url).toBe("https://example.com/page"); + } + }); + + it("can clear the label by passing null", async () => { + const created = await createLink(env as any, { url: "https://example.com", label: "Old Label" }); + expect(created.ok).toBe(true); + if (!created.ok) return; + + const updated = await updateLink(env as any, created.data.id, { label: null }); + expect(updated.ok).toBe(true); + if (updated.ok) { + expect(updated.data.label).toBeNull(); + } + }); +}); diff --git a/src/__tests__/unit/api-schemas.test.ts b/src/__tests__/unit/api-schemas.test.ts index 0eb2089..8484ce1 100644 --- a/src/__tests__/unit/api-schemas.test.ts +++ b/src/__tests__/unit/api-schemas.test.ts @@ -25,7 +25,8 @@ describe("CustomSlugStringSchema", () => { ["trailing-", false], ["with space", false], ["sl/ash", false], - ["a".repeat(65), false], + ["a".repeat(128), true], + ["a".repeat(129), false], ])("safeParse(%j) -> ok=%s", (value, expectedOk) => { const result = CustomSlugStringSchema.safeParse(value); expect(result.success).toBe(expectedOk); diff --git a/src/__tests__/unit/qr.test.ts b/src/__tests__/unit/qr.test.ts index 790c33d..6f136b8 100644 --- a/src/__tests__/unit/qr.test.ts +++ b/src/__tests__/unit/qr.test.ts @@ -2,7 +2,53 @@ // SPDX-License-Identifier: Apache-2.0 import { describe, it, expect } from "vitest"; -import { makeQR } from "../../qr"; +import { makeQR, renderQrSvg } from "../../qr"; + +function extractSvgWidth(svg: string): number { + const m = svg.match(/width="([^"]+)"/); + return m ? parseFloat(m[1]) : NaN; +} + +describe("renderQrSvg", () => { + it("respects the size option: larger size produces larger SVG", () => { + const svg200 = renderQrSvg("https://example.com", { size: 200 }); + const svg400 = renderQrSvg("https://example.com", { size: 400 }); + expect(svg200).not.toBeNull(); + expect(svg400).not.toBeNull(); + expect(extractSvgWidth(svg200!)).toBeCloseTo(200, 0); + expect(extractSvgWidth(svg400!)).toBeCloseTo(400, 0); + }); + + it("uses 220 as default size when no size provided", () => { + const svg = renderQrSvg("https://example.com"); + expect(svg).not.toBeNull(); + expect(extractSvgWidth(svg!)).toBeCloseTo(220, 0); + }); + + it("returns null for non-positive or non-integer sizes", () => { + expect(renderQrSvg("https://example.com", { size: 0 })).toBeNull(); + expect(renderQrSvg("https://example.com", { size: -5 })).toBeNull(); + expect(renderQrSvg("https://example.com", { size: 1.5 })).toBeNull(); + expect(renderQrSvg("https://example.com", { size: NaN })).toBeNull(); + }); + + it("returns null for sizes above MAX_QR_SIZE (2048)", () => { + expect(renderQrSvg("https://example.com", { size: 2049 })).toBeNull(); + expect(renderQrSvg("https://example.com", { size: 100000 })).toBeNull(); + }); + + it("accepts size = 1 (smallest valid)", () => { + const svg = renderQrSvg("https://example.com", { size: 1 }); + expect(svg).not.toBeNull(); + expect(extractSvgWidth(svg!)).toBeCloseTo(1, 0); + }); + + it("accepts size = 2048 (largest valid)", () => { + const svg = renderQrSvg("https://example.com", { size: 2048 }); + expect(svg).not.toBeNull(); + expect(extractSvgWidth(svg!)).toBeCloseTo(2048, 0); + }); +}); describe("makeQR", () => { it("returns a non-empty square boolean matrix for short input", () => { diff --git a/src/api/analytics.ts b/src/api/analytics.ts index b9355e7..4af4a35 100644 --- a/src/api/analytics.ts +++ b/src/api/analytics.ts @@ -9,8 +9,9 @@ import { } from "../services/link-management"; import { resolveClickFilters } from "../services/admin-management"; import { fromServiceResult } from "./response"; +import { TIMELINE_RANGES } from "./schemas"; -const VALID_RANGES = new Set(["24h", "7d", "30d", "90d", "1y", "all"]); +const VALID_RANGES = new Set(TIMELINE_RANGES); function parseRange(rangeParam: string | null | undefined): TimelineRange | undefined { return VALID_RANGES.has(rangeParam as TimelineRange) ? (rangeParam as TimelineRange) : undefined; diff --git a/src/api/bundles.ts b/src/api/bundles.ts index f86526f..be5acf6 100644 --- a/src/api/bundles.ts +++ b/src/api/bundles.ts @@ -32,10 +32,11 @@ import { IdParamSchema, LinkSchema, RangeQuerySchema, + TIMELINE_RANGES, UpdateBundleBodySchema, paramHook, } from "./schemas"; -const VALID_RANGES = new Set(["24h", "7d", "30d", "90d", "1y", "all"]); +const VALID_RANGES = new Set(TIMELINE_RANGES); function parseRange(raw: string | undefined, fallback: TimelineRange = "30d"): TimelineRange { return VALID_RANGES.has(raw as TimelineRange) ? (raw as TimelineRange) : fallback; diff --git a/src/api/links.ts b/src/api/links.ts index 7180d18..10f0891 100644 --- a/src/api/links.ts +++ b/src/api/links.ts @@ -25,6 +25,7 @@ import { handlePublicLinkAnalytics, handlePublicLinkTimeline } from "./analytics import { fetchPageTitle } from "../title-fetch"; import { fromServiceResult, json } from "./response"; import { requireScope } from "./scope"; +import { DEFAULT_QR_SIZE, MAX_QR_SIZE, MIN_QR_SIZE } from "../constants"; import type { Env, TimelineRange } from "../types"; import { AddSlugBodySchema, @@ -348,8 +349,8 @@ const linkQrRoute = createRoute({ query: z.object({ slug: z.string().regex(/^[a-zA-Z0-9_-]+$/).optional() .openapi({ description: "Optional specific slug. Defaults to the link's primary slug." }), - size: z.string().regex(/^\d+$/).optional() - .openapi({ description: "PNG dimensions in pixels (square). Default per server config." }), + size: z.coerce.number().int().min(MIN_QR_SIZE).max(MAX_QR_SIZE).optional() + .openapi({ description: `QR pixel dimensions (square). Integer ${MIN_QR_SIZE}-${MAX_QR_SIZE}; defaults to ${DEFAULT_QR_SIZE}.` }), }), }, responses: { diff --git a/src/api/qr.ts b/src/api/qr.ts index 48f5934..15af509 100644 --- a/src/api/qr.ts +++ b/src/api/qr.ts @@ -1,28 +1,42 @@ // Copyright 2026 Oddbit (https://oddbit.id) // SPDX-License-Identifier: Apache-2.0 +import { MAX_QR_SIZE, MIN_QR_SIZE } from "../constants"; import { Env } from "../types"; import { getLink } from "../services/link-management"; import { renderQrSvg } from "../qr"; import { json } from "./response"; export async function handleLinkQr(request: Request, env: Env, linkId: number): Promise { + const url = new URL(request.url); + const requestedSlug = url.searchParams.get("slug") ?? undefined; + const sizeParam = url.searchParams.get("size"); + + if (requestedSlug === "") { + return json({ error: "slug must not be empty" }, 400); + } + + let size: number | undefined; + if (sizeParam !== null) { + const parsed = Number(sizeParam); + if (!Number.isFinite(parsed) || !Number.isInteger(parsed) || parsed < MIN_QR_SIZE || parsed > MAX_QR_SIZE) { + return json({ error: `size must be an integer between ${MIN_QR_SIZE} and ${MAX_QR_SIZE}` }, 400); + } + size = parsed; + } + const result = await getLink(env, linkId); if (!result.ok) return json({ error: result.error }, result.status); const link = result.data; - const url = new URL(request.url); - const requestedSlug = url.searchParams.get("slug"); - const slug = requestedSlug ? link.slugs.find((s) => s.slug === requestedSlug) - : link.slugs.find((s) => s.is_custom) ?? link.slugs[0]; + : link.slugs.find((s) => s.is_primary) ?? link.slugs[0]; if (!slug) return json({ error: "Slug not found" }, 404); - const origin = url.origin; - const qrUrl = `${origin}/${slug.slug}?utm_medium=qr`; - const svg = renderQrSvg(qrUrl); + const qrUrl = `${url.origin}/${slug.slug}?utm_medium=qr`; + const svg = renderQrSvg(qrUrl, { size }); if (!svg) return json({ error: "Failed to generate QR code" }, 500); diff --git a/src/api/schemas.ts b/src/api/schemas.ts index 1f4e466..c3e8316 100644 --- a/src/api/schemas.ts +++ b/src/api/schemas.ts @@ -4,6 +4,7 @@ import { z } from "@hono/zod-openapi"; import type { Hook } from "@hono/zod-openapi"; import { formatZodError } from "./response"; +import { MIN_SLUG_LENGTH, MAX_SLUG_LENGTH } from "../constants"; // ---- Common ---- @@ -62,7 +63,7 @@ export const CreateLinkBodySchema = z .object({ url: z.string().url().max(2048), label: z.string().optional(), - slug_length: z.number().int().min(3).max(16).optional(), + slug_length: z.number().int().min(MIN_SLUG_LENGTH).max(MAX_SLUG_LENGTH).optional(), expires_at: z.number().int().nonnegative().optional(), allow_duplicate: z.boolean().optional(), }) @@ -86,7 +87,7 @@ export const UpdateLinkBodySchema = z export const CustomSlugStringSchema = z .string() .min(1) - .max(64) + .max(MAX_SLUG_LENGTH) .regex(/^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$/i); export const AddSlugBodySchema = z diff --git a/src/constants.ts b/src/constants.ts index 130205f..edfb7a6 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -2,4 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 export const MIN_SLUG_LENGTH = 3; +export const MAX_SLUG_LENGTH = 128; export const DEFAULT_SLUG_LENGTH = MIN_SLUG_LENGTH; + +export const MIN_QR_SIZE = 1; +export const MAX_QR_SIZE = 2048; +export const DEFAULT_QR_SIZE = 220; diff --git a/src/mcp/server.ts b/src/mcp/server.ts index ec3e1b5..4fba5f9 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -62,6 +62,7 @@ import { CustomSlugStringSchema, TIMELINE_RANGES, } from "../api/schemas"; +import { MIN_SLUG_LENGTH, MAX_SLUG_LENGTH, DEFAULT_SLUG_LENGTH } from "../constants"; import pkg from "../../package.json"; type ToolResult = { @@ -216,7 +217,7 @@ export class ShrtnrMCP extends McpAgent, Props> { inputSchema: { url: z.string().url().describe("Destination URL to shorten"), label: z.string().optional().describe("Human-readable label for the link"), - slug_length: z.number().int().min(3).max(16).optional().describe("Length of the random slug (default: 3)"), + slug_length: z.number().int().min(MIN_SLUG_LENGTH).max(MAX_SLUG_LENGTH).optional().describe(`Length of the random slug (default: ${DEFAULT_SLUG_LENGTH})`), custom_slug: z .union([CustomSlugStringSchema, z.array(CustomSlugStringSchema)]) .optional() @@ -468,7 +469,7 @@ export class ShrtnrMCP extends McpAgent, Props> { description: "Get a QR code SVG for a short link. The QR encodes the short URL with a ?qr tracking parameter.", inputSchema: { link_id: z.number().int().positive().describe("Numeric ID of the link"), - slug: z.string().optional().describe("Specific slug to use (defaults to custom slug or primary)"), + slug: z.string().optional().describe("Specific slug to use (defaults to the link's primary slug)"), base_url: z.string().url().describe("Base URL of the shrtnr instance, e.g. https://oddb.it"), }, annotations: { title: "Link QR code", ...READ_ONLY }, @@ -480,7 +481,7 @@ export class ShrtnrMCP extends McpAgent, Props> { const target = requestedSlug ? link.slugs.find((s) => s.slug === requestedSlug) - : (link.slugs.find((s) => s.is_custom) ?? link.slugs[0]); + : (link.slugs.find((s) => s.is_primary) ?? link.slugs[0]); if (!target) return fail("Slug not found"); diff --git a/src/qr.ts b/src/qr.ts index b64c33c..504017c 100644 --- a/src/qr.ts +++ b/src/qr.ts @@ -1,6 +1,8 @@ // Copyright 2026 Oddbit (https://oddbit.id) // SPDX-License-Identifier: Apache-2.0 +import { DEFAULT_QR_SIZE, MAX_QR_SIZE, MIN_QR_SIZE } from "./constants"; + /** * Generate a QR code matrix for the given text. * Returns a 2D boolean grid (true = dark module) or null if the text is too long. @@ -197,19 +199,23 @@ function rsEncode(data: number[], numEcc: number): number[] { /** * Render a QR code as an SVG string. - * Returns null if the text cannot be encoded. + * Returns null if the text cannot be encoded or the size is invalid. */ export function renderQrSvg( text: string, options: { size?: number; fg?: string; bg?: string } = {}, ): string | null { + if (options.size !== undefined && (!Number.isInteger(options.size) || options.size < MIN_QR_SIZE || options.size > MAX_QR_SIZE)) { + return null; + } + const matrix = makeQR(text); if (!matrix) return null; const modules = matrix.length; const margin = 4; const total = modules + margin * 2; - const cellSize = (options.size ?? 220) / total; + const cellSize = (options.size ?? DEFAULT_QR_SIZE) / total; const svgSize = total * cellSize; const fg = options.fg ?? "#001110"; const bg = options.bg ?? "#ffffff"; diff --git a/src/services/link-management.ts b/src/services/link-management.ts index cb25159..264710b 100644 --- a/src/services/link-management.ts +++ b/src/services/link-management.ts @@ -129,6 +129,7 @@ export async function updateLink( } catch { return fail(400, "url must be a valid URL"); } + body.url = normalizeUrl(body.url); } const link = await LinkRepository.update(env.DB, id, body); @@ -170,19 +171,21 @@ export async function enableLink(env: Env, id: number, identity: string): Promis const link = await LinkRepository.getById(env.DB, id); if (!link) return fail(404, "Link not found"); if (link.created_by !== identity) return fail(403, "Only the link owner can enable this link"); - const enabled = await LinkRepository.update(env.DB, id, { expires_at: null }); + const enabled = await LinkRepository.enable(env.DB, id); + // null on race: link deleted between getById and the UPDATE. + if (!enabled) return fail(404, "Link not found"); await Promise.all( - enabled!.slugs.map((s) => + enabled.slugs.map((s) => SlugCache.put(env.SLUG_KV, s.slug, { - url: enabled!.url, + url: enabled.url, disabled_at: s.disabled_at, expires_at: null, }), ), ); - return ok(enabled!); + return ok(enabled); } export async function deleteLink(env: Env, id: number, identity: string): Promise> { diff --git a/src/slugs.ts b/src/slugs.ts index 56e25ce..224c0da 100644 --- a/src/slugs.ts +++ b/src/slugs.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { SlugRepository } from "./db"; -import { MIN_SLUG_LENGTH } from "./constants"; +import { MIN_SLUG_LENGTH, MAX_SLUG_LENGTH } from "./constants"; // Unambiguous lowercase characters: removed l, o, 0, 1 to avoid confusion export const RANDOM_CHARSET = "abcdefghijkmnpqrstuvwxyz23456789"; @@ -43,6 +43,6 @@ export function validateCustomSlug(slug: string): string | null { export function validateSlugLength(length: number): string | null { if (!Number.isInteger(length) || length < MIN_SLUG_LENGTH) return `Slug length must be an integer >= ${MIN_SLUG_LENGTH}`; - if (length > 128) return "Slug length must be an integer <= 128"; + if (length > MAX_SLUG_LENGTH) return `Slug length must be an integer <= ${MAX_SLUG_LENGTH}`; return null; }