Skip to content

fix(resources): URL attribute-suffix routing for programmatic static-properties Resources - #1933

Merged
kriszyp merged 2 commits into
mainfrom
fix/parsepath-static-properties-1922
Jul 28, 2026
Merged

fix(resources): URL attribute-suffix routing for programmatic static-properties Resources#1933
kriszyp merged 2 commits into
mainfrom
fix/parsepath-static-properties-1922

Conversation

@kylebernhardy

Copy link
Copy Markdown
Member

Summary

Closes #1922 (follow-up from #1921's review). Resource.parsePath resolves a URL attribute suffix (/Widget/id.label) by scanning this.attributes for a matching name. A programmatic Resource that declares only static properties (a Record, no attributes Array — the shape #1921 makes first-class for schemas) has attributes === undefined, so that routing silently never matched.

Fix: fall back to an own-key lookup on the static properties Record, which is keyed by attribute name. It's O(1), so there's no per-request projection on this hot path (the issue floated a memoized projection; a direct key check is simpler and needs no cache or invalidation).

Where to look

  • resources/Resource.ts (parsePath) — one added clause: ... || (properties != null && Object.hasOwn(properties, property)). Object.hasOwn (not properties[property]) is deliberate — a plain index would match inherited Object.prototype members (constructor, toString, …) and mistake /id.constructor for an attribute suffix. Table-backed routing is unchanged (the attributes.find check still runs first).

Tests

unitTests/resources/parsePath.test.js — attribute-suffix routing on a programmatic static properties Resource (with/without a query object), an unknown suffix left untouched, and the inherited-prototype guard (constructor/toString/hasOwnProperty/__proto__ are not treated as attributes).

Self-review caught the inherited-prototype issue; given the change is a single additive clause I did not run the full cross-model orchestration.

PR generated by kAIle (Claude Opus 4.8).

… static-properties Resources (#1922)

`Resource.parsePath` resolved a `/id.attr` suffix by scanning `this.attributes`,
so a programmatic Resource declaring only `static properties` (no `attributes`
Array) never matched — `/Widget/id.label` silently didn't route to the attribute.

Fall back to an OWN-key lookup on the `static properties` Record (keyed by
attribute name): O(1), so no per-request projection. The own-key check
(Object.hasOwn) avoids matching inherited Object.prototype members like
`constructor`/`toString` that a plain `properties[property]` would.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy
@kylebernhardy
kylebernhardy requested a review from kriszyp July 24, 2026 03:43
@kylebernhardy kylebernhardy added this to the v5.2 milestone Jul 24, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for URL attribute-suffix routing on programmatic resources that declare static properties instead of an attributes array, utilizing Object.hasOwn to prevent matching inherited Object.prototype members. It also adds comprehensive unit tests to verify this routing behavior. The feedback suggests removing a redundant any cast when accessing this.properties in Resource.ts to preserve TypeScript's type safety, as properties is already declared as a static property on the class.

Comment thread resources/Resource.ts Outdated
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

…review)

`static properties` is declared on Resource, so `this.properties` is typed in the
static method — no cast needed. Raised by gemini-code-assist.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy
@kylebernhardy

Copy link
Copy Markdown
Member Author

Docs companion opened: HarperFast/documentation#605 — docs(resources): document static properties as a first-class MCP/OpenAPI schema source, which updates the REST overview's GET /MyTable/123.propertyName note to cover programmatic static properties Resources.

Comment generated by kAIle (Claude Opus 4.8).

@kylebernhardy
kylebernhardy marked this pull request as ready for review July 24, 2026 21:58
@kriszyp
kriszyp merged commit b508039 into main Jul 28, 2026
46 checks passed
@kriszyp
kriszyp deleted the fix/parsepath-static-properties-1922 branch July 28, 2026 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[MCP] parsePath attribute-suffix routing doesn't resolve for programmatic static-properties Resources

4 participants