Skip to content

Mirror page path matching for resources#55

Open
sbsoftware-agent wants to merge 9 commits into
releases/v0.32.0from
feature/CR-133
Open

Mirror page path matching for resources#55
sbsoftware-agent wants to merge 9 commits into
releases/v0.32.0from
feature/CR-133

Conversation

@sbsoftware-agent
Copy link
Copy Markdown
Collaborator

Summary

  • Share Page path matching helpers with Resources
  • Add Resource support for root_path, path_param, nested_path, and named uri_path
  • Prevent nested Resource paths from matching parent-only collection or member paths

Testing

  • CRYSTAL_CACHE_DIR=/tmp/crystal-cache- crystal spec --error-trace

Comment thread spec/resource/path_matching_spec.cr Outdated
Copy link
Copy Markdown
Owner

@sbsoftware sbsoftware left a comment

Choose a reason for hiding this comment

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

Please remove the legacy interface altogether.

@sbsoftware-agent
Copy link
Copy Markdown
Collaborator Author

Removed the legacy Resource.nested_path class-method interface locally, including the fallback matching branch and the legacy spec fixture. Updated README wording to point Resources at the Page-style root_path, path_param, and nested_path declarations.

Verification: CRYSTAL_CACHE_DIR=/tmp/crystal-cache-cr-133-review2 crystal spec --error-trace passes with 101 examples.

I could not create the focused commit from this environment because .git is mounted read-only, and the attempted GitHub connector commit was cancelled.

Copy link
Copy Markdown
Owner

@sbsoftware sbsoftware left a comment

Choose a reason for hiding this comment

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

Please double check whether any of the indirections in the path matching module can be removed now.

@sbsoftware-agent
Copy link
Copy Markdown
Collaborator Author

Double-checked and removed the redundant indirections that are no longer needed after dropping the legacy nested Resource hook:

  • Inlined _path_matching_derived_root_path back into _root_path.
  • Removed _path_matching_segment_patterns and inlined its single use in uri_path_matcher.
  • Kept _path_matching_uri_path because Resource still needs it to support both legacy positional uri_path(id) for normal resources and Page-style named uri_path(**params) for declared path params.
  • Simplified Resource#nested? to check declared path parts directly.

Verification: CRYSTAL_CACHE_DIR=/tmp/crystal-cache-cr-133-review3 crystal spec --error-trace passes with 101 examples.

I could not create the focused commit locally because .git is mounted read-only in this environment.

Comment thread src/resource/resource.cr Outdated
Comment thread src/resource/resource.cr Outdated
Comment thread src/resource/resource.cr Outdated
Comment thread src/resource/resource.cr
Comment thread src/resource/resource.cr Outdated
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.

2 participants