From 16d20304a6341b62e24f22d90f462694178b7c10 Mon Sep 17 00:00:00 2001 From: SB Software Agent Date: Fri, 15 May 2026 21:45:47 +0200 Subject: [PATCH 1/9] CR-133 Mirror page path matching for resources --- spec/resource/path_matching_spec.cr | 71 +++++++++++++++++++++++++++++ src/path_matching.cr | 22 +++++++-- src/resource/resource.cr | 63 +++++++++++++++++++------ 3 files changed, 136 insertions(+), 20 deletions(-) diff --git a/spec/resource/path_matching_spec.cr b/spec/resource/path_matching_spec.cr index 2ea791d..d7ecbda 100644 --- a/spec/resource/path_matching_spec.cr +++ b/spec/resource/path_matching_spec.cr @@ -7,6 +7,42 @@ module Crumble::Resource::PathMatchingSpec end end + class PageStyleResource < Resource + root_path "/accounts" + path_param account_id + path_param slug, /[a-z0-9-]+/ + nested_path "posts" + nested_path "details" + + def index + render "account_id=#{account_id} slug=#{slug}" + end + end + + class NestedMemberResource < Resource + root_path "/nested-members" + path_param id + nested_path "details" + + def index + render "nested id=#{id}" + end + end + + class LegacyNestedResource < Resource + def self.root_path + "/legacy-nested" + end + + def self.nested_path + "/details" + end + + def index + render "legacy id=#{id}" + end + end + describe "RootResource.match" do it "should match on /" do RootResource.match("/").should be_truthy @@ -16,4 +52,39 @@ module Crumble::Resource::PathMatchingSpec RootResource.match("/test/").should be_falsey end end + + describe "PageStyleResource.match" do + it "supports the same path matching declarations as pages" do + PageStyleResource.uri_path(account_id: 123, slug: "hello-world").should eq("/accounts/123/hello-world/posts/details") + PageStyleResource.match("/accounts/123/hello-world/posts/details").should be_truthy + PageStyleResource.match("/accounts/123/hello_world/posts/details").should be_falsey + end + + it "exposes declared path params while handling requests" do + response = String.build do |io| + ctx = Crumble::Server::TestRequestContext.new(response_io: io, resource: PageStyleResource.uri_path(account_id: 123, slug: "hello-world")) + PageStyleResource.handle(ctx).should eq(true) + ctx.response.flush + end + + response.should contain("account_id=123 slug=hello-world") + end + end + + describe "NestedMemberResource.match" do + it "does not match the parent member path without the nested component" do + NestedMemberResource.match(NestedMemberResource.uri_path(id: 7)).should be_truthy + NestedMemberResource.match("/nested-members/7").should be_falsey + NestedMemberResource.match("/nested-members").should be_falsey + end + end + + describe "LegacyNestedResource.match" do + it "does not match the parent collection or member path without the nested component" do + LegacyNestedResource.uri_path(7).should eq("/legacy-nested/7/details") + LegacyNestedResource.match(LegacyNestedResource.uri_path(7)).should be_truthy + LegacyNestedResource.match("/legacy-nested/7").should be_falsey + LegacyNestedResource.match("/legacy-nested").should be_falsey + end + end end diff --git a/src/path_matching.cr b/src/path_matching.cr index b1a3cb4..3c102c2 100644 --- a/src/path_matching.cr +++ b/src/path_matching.cr @@ -46,13 +46,21 @@ module Crumble::PathMatching uri_path_matcher.match(path) end - def self._root_path + def self._path_matching_derived_root_path suffix = _path_matching_root_suffix base_name = suffix.empty? ? self.name : self.name.chomp(suffix) "/" + base_name.gsub("::", "/").underscore end + def self._root_path + _path_matching_derived_root_path + end + def self.uri_path(**params) + _path_matching_uri_path(**params) + end + + def self._path_matching_uri_path(**params) param_values = {} of Symbol => String params.each do |key, value| param_values[key] = value.to_s @@ -79,10 +87,7 @@ module Crumble::PathMatching end def self.uri_path_matcher - root_segments = _root_path.split('/').reject(&.empty?) - segment_patterns = root_segments.map { |seg| Regex.escape(seg) } - - segment_patterns.concat(_path_parts.map(&.segment_pattern)) + segment_patterns = _path_matching_segment_patterns if segment_patterns.empty? /^\/$/ @@ -91,6 +96,13 @@ module Crumble::PathMatching end end + def self._path_matching_segment_patterns + root_segments = _root_path.split('/').reject(&.empty?) + segment_patterns = root_segments.map { |seg| Regex.escape(seg) } + + segment_patterns.concat(_path_parts.map(&.segment_pattern)) + end + protected def path_params : Hash(String, String) @path_params ||= begin match = self.class.match(ctx.request.path) diff --git a/src/resource/resource.cr b/src/resource/resource.cr index c3cfd99..3819adf 100644 --- a/src/resource/resource.cr +++ b/src/resource/resource.cr @@ -1,7 +1,17 @@ require "../server/view_handler" +require "../path_matching" abstract class Crumble::Resource include Crumble::Server::ViewHandler + include Crumble::PathMatching + + def self._path_matching_root_suffix : String + "Resource" + end + + def self._root_path + root_path + end macro before(&blk) before(:index, :create, :show, :update, :destroy) {{blk}} @@ -66,36 +76,55 @@ abstract class Crumble::Resource return true end - def self.match(path) - uri_path_matcher.match(path) - end - def self.root_path - "/" + self.name.chomp("Resource").gsub("::", "/").underscore + _path_matching_derived_root_path end def self.root_path(id) "#{root_path}/#{id}" end + macro root_path(path) + def self._root_path + {{path}} + end + + def self.root_path + {{path}} + end + end + def self.nested_path "" end + macro nested_path(path) + PATH_PARTS << Crumble::PathMatching::NestedPathPart.new({{path}}) + end + def self.uri_path(id = nil) - path = root_path - if id - path += "/#{id}" - path += nested_path - end - path + return root_path unless id + return _path_matching_uri_path(id: id) unless _path_parts.empty? + "#{root_path}/#{id}#{nested_path}" + end + + def self.uri_path(**params) + _path_matching_uri_path(**params) end def self.uri_path_matcher - if nested_path.empty? - /^#{root_path}(\/|\/(\d+))?$/ + if _path_parts.empty? + root = root_path.chomp("/") + escaped_root = Regex.escape(root) + + if nested_path.empty? + root.empty? ? /^\/(?:(?\d+)\/?)?$/ : Regex.new("^#{escaped_root}(?:/(?\\d+))?/?$") + else + # A nested resource path belongs to the nested endpoint only; the parent collection/member path must stay available to its own resource. + root.empty? ? Regex.new("^/(?\\d+)#{Regex.escape(nested_path)}/?$") : Regex.new("^#{escaped_root}/(?\\d+)#{Regex.escape(nested_path)}/?$") + end else - /^#{root_path}(\/|\/(\d+)(#{nested_path})?)?$/ + previous_def end end @@ -186,7 +215,7 @@ abstract class Crumble::Resource end def id? - self.class.match(ctx.request.path).try { |m| m[2]?.try(&.to_i64) } + path_params["id"]?.try(&.to_i64?) end def id @@ -194,6 +223,10 @@ abstract class Crumble::Resource end def nested? + unless self.class._path_parts.empty? + return self.class._path_parts.any? { |part| part.is_a?(Crumble::PathMatching::NestedPathPart) } + end + self.class.nested_path.size > 0 end From eb695c2f58af1e5289aa0d6e1f0bbff984e3c788 Mon Sep 17 00:00:00 2001 From: SB Software Agent Date: Fri, 15 May 2026 22:25:25 +0200 Subject: [PATCH 2/9] CR-133 address PR feedback --- src/resource/resource.cr | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/resource/resource.cr b/src/resource/resource.cr index 3819adf..cfa529b 100644 --- a/src/resource/resource.cr +++ b/src/resource/resource.cr @@ -94,10 +94,19 @@ abstract class Crumble::Resource end end + @[Deprecated("Use nested_path \"...\" declarations instead.")] def self.nested_path "" end + def self._legacy_nested_path + {% if @type.class.methods.map(&.name.stringify).includes?("nested_path") %} + nested_path + {% else %} + "" + {% end %} + end + macro nested_path(path) PATH_PARTS << Crumble::PathMatching::NestedPathPart.new({{path}}) end @@ -105,7 +114,7 @@ abstract class Crumble::Resource def self.uri_path(id = nil) return root_path unless id return _path_matching_uri_path(id: id) unless _path_parts.empty? - "#{root_path}/#{id}#{nested_path}" + "#{root_path}/#{id}#{_legacy_nested_path}" end def self.uri_path(**params) @@ -117,11 +126,14 @@ abstract class Crumble::Resource root = root_path.chomp("/") escaped_root = Regex.escape(root) - if nested_path.empty? + legacy_nested_path = _legacy_nested_path + + if legacy_nested_path.empty? root.empty? ? /^\/(?:(?\d+)\/?)?$/ : Regex.new("^#{escaped_root}(?:/(?\\d+))?/?$") else # A nested resource path belongs to the nested endpoint only; the parent collection/member path must stay available to its own resource. - root.empty? ? Regex.new("^/(?\\d+)#{Regex.escape(nested_path)}/?$") : Regex.new("^#{escaped_root}/(?\\d+)#{Regex.escape(nested_path)}/?$") + escaped_nested_path = Regex.escape(legacy_nested_path) + root.empty? ? Regex.new("^/(?\\d+)#{escaped_nested_path}/?$") : Regex.new("^#{escaped_root}/(?\\d+)#{escaped_nested_path}/?$") end else previous_def @@ -227,7 +239,7 @@ abstract class Crumble::Resource return self.class._path_parts.any? { |part| part.is_a?(Crumble::PathMatching::NestedPathPart) } end - self.class.nested_path.size > 0 + self.class._legacy_nested_path.size > 0 end def top_level? From 727a2d896b562bfc1464665444c9a45401f58d90 Mon Sep 17 00:00:00 2001 From: SB Software Agent Date: Sun, 24 May 2026 09:11:38 +0200 Subject: [PATCH 3/9] CR-133 address PR feedback --- README.md | 2 +- spec/resource/path_matching_spec.cr | 23 ----------------------- src/resource/resource.cr | 28 +++------------------------- 3 files changed, 4 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index b870a6a..b742ea2 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,7 @@ form.values # => {name: "...", bio: nil, slug: "..."} ### Resources -`Crumble::Resource` gives you RESTful handlers with sensible defaults for `index`, `show`, `create`, `update`, and `destroy`. Routing follows the class name (`CommentsResource` → `/comments`); nested paths are supported one level deep via `self.nested_path`. +`Crumble::Resource` gives you RESTful handlers with sensible defaults for `index`, `show`, `create`, `update`, and `destroy`. Routing follows the class name (`CommentsResource` → `/comments`); use the same `root_path`, `path_param`, and `nested_path` declarations as pages for custom Resource paths. ```crystal require "css" diff --git a/spec/resource/path_matching_spec.cr b/spec/resource/path_matching_spec.cr index d7ecbda..c923350 100644 --- a/spec/resource/path_matching_spec.cr +++ b/spec/resource/path_matching_spec.cr @@ -29,20 +29,6 @@ module Crumble::Resource::PathMatchingSpec end end - class LegacyNestedResource < Resource - def self.root_path - "/legacy-nested" - end - - def self.nested_path - "/details" - end - - def index - render "legacy id=#{id}" - end - end - describe "RootResource.match" do it "should match on /" do RootResource.match("/").should be_truthy @@ -78,13 +64,4 @@ module Crumble::Resource::PathMatchingSpec NestedMemberResource.match("/nested-members").should be_falsey end end - - describe "LegacyNestedResource.match" do - it "does not match the parent collection or member path without the nested component" do - LegacyNestedResource.uri_path(7).should eq("/legacy-nested/7/details") - LegacyNestedResource.match(LegacyNestedResource.uri_path(7)).should be_truthy - LegacyNestedResource.match("/legacy-nested/7").should be_falsey - LegacyNestedResource.match("/legacy-nested").should be_falsey - end - end end diff --git a/src/resource/resource.cr b/src/resource/resource.cr index cfa529b..be8ff5a 100644 --- a/src/resource/resource.cr +++ b/src/resource/resource.cr @@ -94,19 +94,6 @@ abstract class Crumble::Resource end end - @[Deprecated("Use nested_path \"...\" declarations instead.")] - def self.nested_path - "" - end - - def self._legacy_nested_path - {% if @type.class.methods.map(&.name.stringify).includes?("nested_path") %} - nested_path - {% else %} - "" - {% end %} - end - macro nested_path(path) PATH_PARTS << Crumble::PathMatching::NestedPathPart.new({{path}}) end @@ -114,7 +101,7 @@ abstract class Crumble::Resource def self.uri_path(id = nil) return root_path unless id return _path_matching_uri_path(id: id) unless _path_parts.empty? - "#{root_path}/#{id}#{_legacy_nested_path}" + "#{root_path}/#{id}" end def self.uri_path(**params) @@ -125,16 +112,7 @@ abstract class Crumble::Resource if _path_parts.empty? root = root_path.chomp("/") escaped_root = Regex.escape(root) - - legacy_nested_path = _legacy_nested_path - - if legacy_nested_path.empty? - root.empty? ? /^\/(?:(?\d+)\/?)?$/ : Regex.new("^#{escaped_root}(?:/(?\\d+))?/?$") - else - # A nested resource path belongs to the nested endpoint only; the parent collection/member path must stay available to its own resource. - escaped_nested_path = Regex.escape(legacy_nested_path) - root.empty? ? Regex.new("^/(?\\d+)#{escaped_nested_path}/?$") : Regex.new("^#{escaped_root}/(?\\d+)#{escaped_nested_path}/?$") - end + root.empty? ? /^\/(?:(?\d+)\/?)?$/ : Regex.new("^#{escaped_root}(?:/(?\\d+))?/?$") else previous_def end @@ -239,7 +217,7 @@ abstract class Crumble::Resource return self.class._path_parts.any? { |part| part.is_a?(Crumble::PathMatching::NestedPathPart) } end - self.class._legacy_nested_path.size > 0 + false end def top_level? From c3b8d46d1b6e86277e5d2d848688bdad1abd898c Mon Sep 17 00:00:00 2001 From: SB Software Agent Date: Sun, 24 May 2026 09:52:59 +0200 Subject: [PATCH 4/9] CR-133 address PR feedback --- src/path_matching.cr | 18 +++++------------- src/resource/resource.cr | 8 ++------ 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/src/path_matching.cr b/src/path_matching.cr index 3c102c2..68a964d 100644 --- a/src/path_matching.cr +++ b/src/path_matching.cr @@ -46,16 +46,12 @@ module Crumble::PathMatching uri_path_matcher.match(path) end - def self._path_matching_derived_root_path + def self._root_path suffix = _path_matching_root_suffix base_name = suffix.empty? ? self.name : self.name.chomp(suffix) "/" + base_name.gsub("::", "/").underscore end - def self._root_path - _path_matching_derived_root_path - end - def self.uri_path(**params) _path_matching_uri_path(**params) end @@ -87,7 +83,10 @@ module Crumble::PathMatching end def self.uri_path_matcher - segment_patterns = _path_matching_segment_patterns + root_segments = _root_path.split('/').reject(&.empty?) + segment_patterns = root_segments.map { |seg| Regex.escape(seg) } + + segment_patterns.concat(_path_parts.map(&.segment_pattern)) if segment_patterns.empty? /^\/$/ @@ -96,13 +95,6 @@ module Crumble::PathMatching end end - def self._path_matching_segment_patterns - root_segments = _root_path.split('/').reject(&.empty?) - segment_patterns = root_segments.map { |seg| Regex.escape(seg) } - - segment_patterns.concat(_path_parts.map(&.segment_pattern)) - end - protected def path_params : Hash(String, String) @path_params ||= begin match = self.class.match(ctx.request.path) diff --git a/src/resource/resource.cr b/src/resource/resource.cr index be8ff5a..2778aa9 100644 --- a/src/resource/resource.cr +++ b/src/resource/resource.cr @@ -77,7 +77,7 @@ abstract class Crumble::Resource end def self.root_path - _path_matching_derived_root_path + "/" + self.name.chomp("Resource").gsub("::", "/").underscore end def self.root_path(id) @@ -213,11 +213,7 @@ abstract class Crumble::Resource end def nested? - unless self.class._path_parts.empty? - return self.class._path_parts.any? { |part| part.is_a?(Crumble::PathMatching::NestedPathPart) } - end - - false + self.class._path_parts.any? { |part| part.is_a?(Crumble::PathMatching::NestedPathPart) } end def top_level? From 38bb58715454d3ca29944c89db12b9eb14cfc3f0 Mon Sep 17 00:00:00 2001 From: SB Software Agent Date: Sun, 24 May 2026 10:20:25 +0200 Subject: [PATCH 5/9] CR-133 address PR feedback --- spec/resource/path_matching_spec.cr | 4 +--- spec/resource/redirect_back_spec.cr | 4 +--- spec/server/request_dispatcher_spec.cr | 8 ++------ src/resource/resource.cr | 6 +----- 4 files changed, 5 insertions(+), 17 deletions(-) diff --git a/spec/resource/path_matching_spec.cr b/spec/resource/path_matching_spec.cr index c923350..c3cc3e1 100644 --- a/spec/resource/path_matching_spec.cr +++ b/spec/resource/path_matching_spec.cr @@ -2,9 +2,7 @@ require "../spec_helper" module Crumble::Resource::PathMatchingSpec class RootResource < Resource - def self.root_path - "/" - end + root_path "/" end class PageStyleResource < Resource diff --git a/spec/resource/redirect_back_spec.cr b/spec/resource/redirect_back_spec.cr index 7c14188..86ebbb2 100644 --- a/spec/resource/redirect_back_spec.cr +++ b/spec/resource/redirect_back_spec.cr @@ -2,9 +2,7 @@ require "../spec_helper" module Crumble::Resource::RedirectBackSpec class HomeResource < Resource - def self.root_path - "/" - end + root_path "/" end class MyResource < Resource diff --git a/spec/server/request_dispatcher_spec.cr b/spec/server/request_dispatcher_spec.cr index 53dfd22..b123c38 100644 --- a/spec/server/request_dispatcher_spec.cr +++ b/spec/server/request_dispatcher_spec.cr @@ -42,9 +42,7 @@ end module Crumble::Server::RequestDispatcherResourceSpec class WidgetResource < Crumble::Resource - def self.root_path - "/root-handler-resource" - end + root_path "/root-handler-resource" def index ctx.response.print "resource index" @@ -64,9 +62,7 @@ module Crumble::Server::RequestDispatcherResourceSpec end class SessionResource < Crumble::Resource - def self.root_path - "/root-handler-session" - end + root_path "/root-handler-session" def index current = ctx.session.blah || 0 diff --git a/src/resource/resource.cr b/src/resource/resource.cr index 2778aa9..ffec364 100644 --- a/src/resource/resource.cr +++ b/src/resource/resource.cr @@ -9,10 +9,6 @@ abstract class Crumble::Resource "Resource" end - def self._root_path - root_path - end - macro before(&blk) before(:index, :create, :show, :update, :destroy) {{blk}} end @@ -77,7 +73,7 @@ abstract class Crumble::Resource end def self.root_path - "/" + self.name.chomp("Resource").gsub("::", "/").underscore + _root_path end def self.root_path(id) From 4084a96fe5fd1a89d2d8081e17c2a9236bb9b743 Mon Sep 17 00:00:00 2001 From: SB Software Agent Date: Sun, 24 May 2026 22:31:11 +0200 Subject: [PATCH 6/9] CR-133 address PR feedback --- src/resource/resource.cr | 33 ++++----------------------------- 1 file changed, 4 insertions(+), 29 deletions(-) diff --git a/src/resource/resource.cr b/src/resource/resource.cr index ffec364..c74cc4e 100644 --- a/src/resource/resource.cr +++ b/src/resource/resource.cr @@ -72,41 +72,16 @@ abstract class Crumble::Resource return true end - def self.root_path - _root_path - end - - def self.root_path(id) - "#{root_path}/#{id}" - end - - macro root_path(path) - def self._root_path - {{path}} - end - - def self.root_path - {{path}} - end - end - - macro nested_path(path) - PATH_PARTS << Crumble::PathMatching::NestedPathPart.new({{path}}) - end - def self.uri_path(id = nil) - return root_path unless id + return _root_path unless id return _path_matching_uri_path(id: id) unless _path_parts.empty? - "#{root_path}/#{id}" - end - - def self.uri_path(**params) - _path_matching_uri_path(**params) + root = _root_path.chomp("/") + root.empty? ? "/#{id}" : "#{root}/#{id}" end def self.uri_path_matcher if _path_parts.empty? - root = root_path.chomp("/") + root = _root_path.chomp("/") escaped_root = Regex.escape(root) root.empty? ? /^\/(?:(?\d+)\/?)?$/ : Regex.new("^#{escaped_root}(?:/(?\\d+))?/?$") else From 2e15ad069675b965cc471704dd655eb14da1645c Mon Sep 17 00:00:00 2001 From: SB Software Agent Date: Sun, 24 May 2026 22:47:42 +0200 Subject: [PATCH 7/9] CR-133 address PR feedback --- src/resource/resource.cr | 1 + 1 file changed, 1 insertion(+) diff --git a/src/resource/resource.cr b/src/resource/resource.cr index c74cc4e..b453467 100644 --- a/src/resource/resource.cr +++ b/src/resource/resource.cr @@ -83,6 +83,7 @@ abstract class Crumble::Resource if _path_parts.empty? root = _root_path.chomp("/") escaped_root = Regex.escape(root) + # Plain resources keep REST collection/member routes; the named capture feeds #id? through PathMatching#path_params. root.empty? ? /^\/(?:(?\d+)\/?)?$/ : Regex.new("^#{escaped_root}(?:/(?\\d+))?/?$") else previous_def From 3c233782bd00e25e5693d9eb6f6559b58c4b6fa6 Mon Sep 17 00:00:00 2001 From: SB Software Agent Date: Tue, 26 May 2026 12:22:51 +0200 Subject: [PATCH 8/9] CR-133 address PR feedback --- README.md | 1 + spec/resource/before_spec.cr | 4 +++ spec/resource/path_matching_spec.cr | 13 ++++++++-- spec/resource/redirect_spec.cr | 2 ++ spec/resource/routing_spec.cr | 2 ++ spec/server/request_dispatcher_spec.cr | 1 + src/resource/resource.cr | 35 ++++++++++++++++++-------- 7 files changed, 46 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index b742ea2..2f50391 100644 --- a/README.md +++ b/README.md @@ -206,6 +206,7 @@ end ``` - `render SomeView` wraps the view in the configured layout and sets `Content-Type` to HTML. +- Declare path params such as `path_param id` when a Resource should handle member routes; collection paths route to `index`/`create`, and full param paths route to `show`/`update`. - `redirect` and `redirect_back` set a `303 See Other` by default. - Add `before` filters to short-circuit with `false` (400) or an `Int32` status code. diff --git a/spec/resource/before_spec.cr b/spec/resource/before_spec.cr index 0776472..521a98c 100644 --- a/spec/resource/before_spec.cr +++ b/spec/resource/before_spec.cr @@ -12,6 +12,8 @@ module Crumble::Resource::BeforeSpec end class Res1 < Parent + path_param id + before(:show) do true end @@ -30,6 +32,8 @@ module Crumble::Resource::BeforeSpec end class Res2 < Parent + path_param id + def index raise "This should not be reached!" end diff --git a/spec/resource/path_matching_spec.cr b/spec/resource/path_matching_spec.cr index c3cc3e1..86de79c 100644 --- a/spec/resource/path_matching_spec.cr +++ b/spec/resource/path_matching_spec.cr @@ -13,6 +13,10 @@ module Crumble::Resource::PathMatchingSpec nested_path "details" def index + render "accounts index" + end + + def show render "account_id=#{account_id} slug=#{slug}" end end @@ -22,7 +26,7 @@ module Crumble::Resource::PathMatchingSpec path_param id nested_path "details" - def index + def show render "nested id=#{id}" end end @@ -35,10 +39,15 @@ module Crumble::Resource::PathMatchingSpec it "should not match on any other path ending on /" do RootResource.match("/test/").should be_falsey end + + it "does not match member paths without declared params" do + RootResource.match("/1").should be_falsey + end end describe "PageStyleResource.match" do it "supports the same path matching declarations as pages" do + PageStyleResource.match("/accounts").should be_truthy PageStyleResource.uri_path(account_id: 123, slug: "hello-world").should eq("/accounts/123/hello-world/posts/details") PageStyleResource.match("/accounts/123/hello-world/posts/details").should be_truthy PageStyleResource.match("/accounts/123/hello_world/posts/details").should be_falsey @@ -57,9 +66,9 @@ module Crumble::Resource::PathMatchingSpec describe "NestedMemberResource.match" do it "does not match the parent member path without the nested component" do + NestedMemberResource.match("/nested-members").should be_truthy NestedMemberResource.match(NestedMemberResource.uri_path(id: 7)).should be_truthy NestedMemberResource.match("/nested-members/7").should be_falsey - NestedMemberResource.match("/nested-members").should be_falsey end end end diff --git a/spec/resource/redirect_spec.cr b/spec/resource/redirect_spec.cr index f52e18c..f3a6041 100644 --- a/spec/resource/redirect_spec.cr +++ b/spec/resource/redirect_spec.cr @@ -2,6 +2,8 @@ require "../spec_helper" module Crumble::Resource::RedirectSpec class MyResource < Resource + path_param id + def create redirect self.class.uri_path(1) end diff --git a/spec/resource/routing_spec.cr b/spec/resource/routing_spec.cr index fdd7fcb..47ab851 100644 --- a/spec/resource/routing_spec.cr +++ b/spec/resource/routing_spec.cr @@ -2,6 +2,8 @@ require "../spec_helper" module Crumble::Resource::RoutingSpec class MyResource < Resource + path_param id + def index render "Index!" end diff --git a/spec/server/request_dispatcher_spec.cr b/spec/server/request_dispatcher_spec.cr index b123c38..afdff70 100644 --- a/spec/server/request_dispatcher_spec.cr +++ b/spec/server/request_dispatcher_spec.cr @@ -43,6 +43,7 @@ end module Crumble::Server::RequestDispatcherResourceSpec class WidgetResource < Crumble::Resource root_path "/root-handler-resource" + path_param id def index ctx.response.print "resource index" diff --git a/src/resource/resource.cr b/src/resource/resource.cr index b453467..9929a89 100644 --- a/src/resource/resource.cr +++ b/src/resource/resource.cr @@ -50,7 +50,7 @@ abstract class Crumble::Resource case ctx.request.method when "GET" - if instance.id? && instance.top_level? + if instance.member? before_action_handling(instance, :show) instance.show else @@ -58,7 +58,7 @@ abstract class Crumble::Resource instance.index end when "POST" - if instance.id? && instance.top_level? + if instance.member? before_action_handling(instance, :update) instance.update else @@ -74,22 +74,33 @@ abstract class Crumble::Resource def self.uri_path(id = nil) return _root_path unless id - return _path_matching_uri_path(id: id) unless _path_parts.empty? - root = _root_path.chomp("/") - root.empty? ? "/#{id}" : "#{root}/#{id}" + raise ArgumentError.new("Cannot build a member path for #{self} without path params") if _path_parts.empty? + _path_matching_uri_path(id: id) end def self.uri_path_matcher - if _path_parts.empty? - root = _root_path.chomp("/") - escaped_root = Regex.escape(root) - # Plain resources keep REST collection/member routes; the named capture feeds #id? through PathMatching#path_params. - root.empty? ? /^\/(?:(?\d+)\/?)?$/ : Regex.new("^#{escaped_root}(?:/(?\\d+))?/?$") + if path_param? + collection_pattern = _root_path_segment_patterns.empty? ? "/" : "/" + _root_path_segment_patterns.join("/") + member_pattern = "/" + (_root_path_segment_patterns + _path_parts.map(&.segment_pattern)).join("/") + + if collection_pattern == "/" + Regex.new("^(?:/|#{member_pattern}/?)$") + else + Regex.new("^(?:#{collection_pattern}|#{member_pattern})/?$") + end else previous_def end end + def self.path_param? + _path_parts.any? { |part| part.is_a?(Crumble::PathMatching::ParamPathPart) } + end + + def self._root_path_segment_patterns + _root_path.split('/').reject(&.empty?).map { |seg| Regex.escape(seg) } + end + def initialize(@request_ctx); end def resource_layout @@ -184,6 +195,10 @@ abstract class Crumble::Resource id?.not_nil! end + def member? + path_params.size > 0 + end + def nested? self.class._path_parts.any? { |part| part.is_a?(Crumble::PathMatching::NestedPathPart) } end From 830a746f766e786c22e05174effa43f36b101492 Mon Sep 17 00:00:00 2001 From: SB Software Agent Date: Thu, 28 May 2026 22:23:09 +0200 Subject: [PATCH 9/9] CR-133 address PR feedback --- spec/resource/before_spec.cr | 2 +- src/resource/resource.cr | 8 -------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/spec/resource/before_spec.cr b/spec/resource/before_spec.cr index 521a98c..39316f7 100644 --- a/spec/resource/before_spec.cr +++ b/spec/resource/before_spec.cr @@ -3,7 +3,7 @@ require "../spec_helper" module Crumble::Resource::BeforeSpec class Parent < Crumble::Resource before do - if id? + if member? true else false diff --git a/src/resource/resource.cr b/src/resource/resource.cr index 9929a89..c10fe6c 100644 --- a/src/resource/resource.cr +++ b/src/resource/resource.cr @@ -187,14 +187,6 @@ abstract class Crumble::Resource ctx.response.print "Not Found" end - def id? - path_params["id"]?.try(&.to_i64?) - end - - def id - id?.not_nil! - end - def member? path_params.size > 0 end