[castle][JIRA] Created: (MR-394) PatternRoute should support url part with dot in it

[Gauthier Segay (JIRA)]

PatternRoute should support url part with dot in it
---------------------------------------------------

Key: MR-394
URL: http://support.castleproject.org//browse/MR-394
Project: MonoRail
Issue Type: New Feature
Environment: trunk: Monorail + new RoutingModuleEx
Reporter: Gauthier Segay
Priority: Minor
Attachments: patternroute.patch

by default a pattern such as PatternRoute route = new PatternRoute("/<controller>/<id>/<action>").Restrict("action").AnyOf("index");

will match "/some/123/index" but not "/some/a.b.c/index" after investigation this is due to two things:
- the url is splitted with both '/' and '.' characters, each split is evaluated against a DefaultNode in a foreach loop
- the regular expression used in DefaultNode doesn't comply to match strings with dot

I propose to add WithDot and WithoutDot methods to the RestrictionConfigurer inner class to allow specifying if a given node can match strings and rewrite the first loop of PatternRoute.Matches method

attached patch does:
- add WithDot and WithoutDot methods to PatternRoute.RestrictionConfigurer
- add acceptsDot member + AcceptsDot property in DefaultNode
- rewrite firstloop of PatternRoute.Matches method (code quality is not top notch BTW)
- added test case for new methods of RestrictionConfigurer are ok

Does the patch/feature could be accepted? let me know if something is not ok.

Thanks.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://support.castleproject.org//secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

[Gauthier Segay (JIRA)]

[ http://support.castleproject.org//browse/MR-394?page=all ]

Gauthier Segay updated MR-394:
------------------------------

Attachment: patternroute_bis.patch

updated to the trunk

> PatternRoute should support url part with dot in it
> ---------------------------------------------------
>
> Key: MR-394
> URL: http://support.castleproject.org//browse/MR-394
> Project: MonoRail
> Issue Type: New Feature
> Environment: trunk: Monorail + new RoutingModuleEx
> Reporter: Gauthier Segay
> Priority: Minor

> Attachments: patternroute.patch, patternroute_bis.patch

[Gauthier Segay (JIRA)]

[ http://support.castleproject.org//browse/MR-394?page=comments#action_12446 ]

Gauthier Segay commented on MR-394:
-----------------------------------

some disambiguation about a question from James Curran (http://groups.google.com/group/castle-project-devel/browse_frm/thread/55c12e2fda84d999/df0510dd68ba9287#df0510dd68ba9287):

WithoutDot is the default behaviour for pattern nodes, mainly for not inducing breaking change with the previous one, the method is here to allow revert the effect of a WithDot call

sample route:

PatternRoute route = new PatternRoute("/<controller>/<id>/<action>")
.Restrict("action")
.AnyOf("index")

.Restrict("id")
.WithoutDot(); // has no effect compared to current behaviour

this route will match
/some/123/index

but it won't match
/some/1.2.3/index

WithDot is the new behaviour enabled with this patch, it applies to the pattern node restriction, it allows the node to be matched even if it contains dot(s)

sample route:

PatternRoute route = new PatternRoute("/<controller>/<id>/<action>")
.Restrict("action")
.AnyOf("index")

.Restrict("id")
.WithDot();

this route will match both:
/some/123/index
/some/1.2.3/index

hope this clarify the use of the proposed restriction features.

> PatternRoute should support url part with dot in it
> ---------------------------------------------------
>
> Key: MR-394
> URL: http://support.castleproject.org//browse/MR-394
> Project: MonoRail
> Issue Type: New Feature
> Environment: trunk: Monorail + new RoutingModuleEx
> Reporter: Gauthier Segay
> Priority: Minor

> Attachments: patternroute.patch, patternroute_bis.patch

[hamilton verissimo (JIRA)]

[ http://support.castleproject.org//browse/MR-394?page=comments#action_12447 ]

hamilton verissimo commented on MR-394:
---------------------------------------

Can I get more feedback on this one? I'm not sure I like the solution, but also cant think of a better one.

> PatternRoute should support url part with dot in it
> ---------------------------------------------------
>
> Key: MR-394
> URL: http://support.castleproject.org//browse/MR-394
> Project: MonoRail
> Issue Type: New Feature
> Environment: trunk: Monorail + new RoutingModuleEx
> Reporter: Gauthier Segay

> Assigned To: hamilton verissimo

> Priority: Minor
> Attachments: patternroute.patch, patternroute_bis.patch
>
>

[Ken Egozi (JIRA)]

[ http://support.castleproject.org//browse/MR-394?page=comments#action_12448 ]

Ken Egozi commented on MR-394:
------------------------------

imo every legal url character should be matched (except / which separates nodes, and ? that separates to the qstring),

the only backward compatibility or problem I can think of, is which extension on the last node that needs to be ignored. However, I believe that this isn't rule specific but rather site-wide (as you're either on a star-mapping to ASPNET/MR, or on .rails/.castle/.aspx/.whatever) so it could be setup as once on the RoutingEngineEx level, and since we face forward, I'd say that by default it'd think that extensions should not be stripped, and you'd be able to do RoutingEngineEx.ShouldStrip(".castle") or the like

> PatternRoute should support url part with dot in it
> ---------------------------------------------------
>
> Key: MR-394
> URL: http://support.castleproject.org//browse/MR-394
> Project: MonoRail
> Issue Type: New Feature
> Environment: trunk: Monorail + new RoutingModuleEx
> Reporter: Gauthier Segay

> Assigned To: hamilton verissimo
> Priority: Minor
> Attachments: patternroute.patch, patternroute_bis.patch
>
>

[Gauthier Segay (JIRA)]

[ http://support.castleproject.org//browse/MR-394?page=comments#action_12449 ]

Gauthier Segay commented on MR-394:
-----------------------------------

The proposed patch was to allow my requested change to not alter actual behaviour.

A convinient solution (IMHO) would be to remove all the 'extension' split questions but for the very lastnode (after the last slash) and if desired I can work on this solution and remove the WithDot/WithoutDot workarround, it will work like it was WithDot for every route node but the last node.

> PatternRoute should support url part with dot in it
> ---------------------------------------------------
>
> Key: MR-394
> URL: http://support.castleproject.org//browse/MR-394
> Project: MonoRail
> Issue Type: New Feature
> Environment: trunk: Monorail + new RoutingModuleEx
> Reporter: Gauthier Segay

> Assigned To: hamilton verissimo
> Priority: Minor
> Attachments: patternroute.patch, patternroute_bis.patch
>
>

[hamilton verissimo (JIRA)]

[ http://support.castleproject.org//browse/MR-394?page=all ]

hamilton verissimo resolved MR-394.
-----------------------------------

Resolution: Applied

Applied on the routing branch. thanks

> PatternRoute should support url part with dot in it
> ---------------------------------------------------
>
> Key: MR-394
> URL: http://support.castleproject.org//browse/MR-394
> Project: MonoRail
> Issue Type: New Feature
> Environment: trunk: Monorail + new RoutingModuleEx
> Reporter: Gauthier Segay

> Assigned To: hamilton verissimo
> Priority: Minor
> Attachments: patternroute.patch, patternroute_bis.patch
>
>

[Gauthier Segay (JIRA)]

[ http://support.castleproject.org//browse/MR-394?page=comments#action_12580 ]

Gauthier Segay commented on MR-394:
-----------------------------------

I've updated to trunk, I still have problem with the following test case (modified from my initial patch):

[Test]
public void WithDot()
{

PatternRoute route = new PatternRoute("/<controller>/<id>/<action>")
.Restrict("action").AnyOf("index");

RouteMatch match = new RouteMatch();
Assert.IsTrue(route.Matches("/some/123/index", CreateGetContext(), match) > 0);
Assert.AreEqual("some", match.Parameters["controller"]);
Assert.AreEqual("123", match.Parameters["id"]);
Assert.AreEqual("index", match.Parameters["action"]);
route.Matches("/some/id.with.dot.in.it/index", CreateGetContext(), match); // returns 0
Assert.AreEqual("some", match.Parameters["controller"]);
Assert.AreEqual("id.with.dot.in.it", match.Parameters["id"]); // actual = "id", expected = "id.with.dot.in.it"
Assert.AreEqual("index", match.Parameters["action"]);
}

any chance to support this without affecting PatternRoute (adding my then proposed WithDot / WithoutDot) or would an updated patch will be accepted?

Thanks.

> PatternRoute should support url part with dot in it
> ---------------------------------------------------
>
> Key: MR-394
> URL: http://support.castleproject.org//browse/MR-394
> Project: MonoRail
> Issue Type: New Feature
> Environment: trunk: Monorail + new RoutingModuleEx
> Reporter: Gauthier Segay

> Assigned To: hamilton verissimo
> Priority: Minor
> Attachments: patternroute.patch, patternroute_bis.patch
>
>

[hamilton verissimo (JIRA)]

[ http://support.castleproject.org//browse/MR-394?page=comments#action_12581 ]

hamilton verissimo commented on MR-394:
---------------------------------------

I've applied your patch but reverted it. My problem with it is that the routing isn't very simple, and to dealing with dots make the code even more complex.

Anyway, I'd suggest that you implement your own solution with a class that extends PatternRoute.

> PatternRoute should support url part with dot in it
> ---------------------------------------------------
>
> Key: MR-394
> URL: http://support.castleproject.org//browse/MR-394
> Project: MonoRail
> Issue Type: New Feature
> Environment: trunk: Monorail + new RoutingModuleEx
> Reporter: Gauthier Segay

> Assigned To: hamilton verissimo
> Priority: Minor
> Attachments: patternroute.patch, patternroute_bis.patch
>
>

[Gauthier Segay (JIRA)]

[ http://support.castleproject.org//browse/MR-394?page=comments#action_12582 ]

Gauthier Segay commented on MR-394:
-----------------------------------

I'll have a spin for a custom rule that I would contribute, I suppose it require at least to mark Match and CreateUrl as virtual, and some members to be protected.

I'm not sure where the complexity is laying concerning the dot, it should only be a concern in the last node of the pattern and should be processed like any other chars for previous nodes, could you comment what is DefaultNode.afterDot?

Thanks

> PatternRoute should support url part with dot in it
> ---------------------------------------------------
>
> Key: MR-394
> URL: http://support.castleproject.org//browse/MR-394
> Project: MonoRail
> Issue Type: New Feature
> Environment: trunk: Monorail + new RoutingModuleEx
> Reporter: Gauthier Segay

> Assigned To: hamilton verissimo
> Priority: Minor
> Attachments: patternroute.patch, patternroute_bis.patch
>
>

[Gauthier Segay (JIRA)]

[ http://support.castleproject.org//browse/MR-394?page=all ]

Gauthier Segay updated MR-394:
------------------------------

Attachment: patternroute_make_urlparts_creation_virtualmethod.patch

Hamilton, here is a minor patch that separate the url splitting operation in a virtual method.

That all what I need to create custom PatternRoute that would match parts with dot in it.

Do you agree to separate this splitting logic in a virtual method?

If someone ask, I can also provide custom PatternRoute implementation that fit my initial request / testcase.

> PatternRoute should support url part with dot in it
> ---------------------------------------------------
>
> Key: MR-394
> URL: http://support.castleproject.org//browse/MR-394
> Project: MonoRail
> Issue Type: New Feature
> Environment: trunk: Monorail + new RoutingModuleEx
> Reporter: Gauthier Segay

> Assigned To: hamilton verissimo
> Priority: Minor

> Attachments: patternroute.patch, patternroute_bis.patch, patternroute_make_urlparts_creation_virtualmethod.patch

[Gauthier Segay]

I've updated the patch here:
http://support.castleproject.org/projects/MR/issues/view/MR-ISSUE-394

This would allow overriding default url parts resolving used in
PatternRoute.Matches

Could it be applied?

Thanks

[Roelof Blom]

Applied in r5300

[Gauthier Segay]

Thanks,

if useful to someone here is my overrided PatternRoute to handle dots
in url parts:


public class MyPatternRoute : PatternRoute {
public MyPatternRoute(string pattern) : base(pattern) { }


protected override string[] GetUrlParts(string url) {
System.Collections.Generic.List<string> splits = new
System.Collections.Generic.List<string>(url.Split(new char[] { '/' },
System.StringSplitOptions.RemoveEmptyEntries));
string last = splits[splits.Count - 1];
splits.RemoveAt(splits.Count - 1);
splits.AddRange(last.Split(new char[] { '.' },
System.StringSplitOptions.RemoveEmptyEntries));

return splits.ToArray();
}
}



public static class Extensions {
public static PatternRoute WithDot(this
PatternRoute.RestrictionConfigurer configurer) {
return configurer.ValidRegex(@"[a-zA-Z,_,0-9,-,\.]+");

}
}


On Aug 31, 10:26 pm, "Roelof Blom" <roelof.b...@gmail.com> wrote:
> Applied in r5300
>
> On Sat, Aug 30, 2008 at 11:25 PM, Gauthier Segay

> <gauthier.se...@gmail.com>wrote:

[Roelof Blom]

Possibly useful to put on http://using.castleproject.org/display/MR/Home

[Gauthier Segay]

Routing tips page added:

http://using.castleproject.org/display/MR/Routing+tips

everyone is welcome to add their own :)

On Sep 1, 4:23 pm, "Roelof Blom" <roelof.b...@gmail.com> wrote:
> Possibly useful to put onhttp://using.castleproject.org/display/MR/Home

[Roelof Blom]

Great, thanks!