Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot create URI with missing optional parameters #213

Open
JornWildt opened this issue Nov 24, 2020 · 4 comments
Open

Cannot create URI with missing optional parameters #213

JornWildt opened this issue Nov 24, 2020 · 4 comments

Comments

@JornWildt
Copy link

With this path: "distributed-requisitions/feature-set?version={version}"

and this GET handler: public object Get([Optional]int? version) { ... }

and this configuration:

  ResourceSpace.Has.ResourcesOfType<FeatureSetResource>()
      .AtUri(DistributedRequisitionsConstants.FeatureSetPath)
      .HandledBy<FeatureSetHandler>()
      .TranscodedBy<AutomaticXmlWithHtmlStylesheetCodec<FeatureSetResource,FeatureSet>>();

I can GET that URL with and without "?version=XXX" in the URL. Handling of optional parameters works fine in this case.

But when I try to create a URI without the version parameter:

Uri url = typeof(FeatureSetResource).CreateUri(new { });

it fails:

System.InvalidOperationException: No suitable Uri could be found for resource with key CLR Type: FeatureSetResource with values .
   at OpenRasta.Web.TemplatedUriResolver.CreateUriFor(Uri baseAddress, Object resourceKey, String uriName, NameValueCollection keyValues)
   at OpenRasta.Web.IUriResolverExtensions.CreateUri(Object target, Uri baseUri, String uriName, Object additionalProperties)
   at OpenRasta.Web.IUriResolverExtensions.CreateUri(Object target, Object additionalProperties)

If I add the "version" parameter it works:

Uri contentUri = typeof(FeatureSetResource).CreateUri(new { version = 9});

Problem 1: I would expect typeof(FeatureSetResource).CreateUri(new { }); to work since the "version" parameter is marked as optional.

Problem 2: I could not find any documentation of the use of [Optional] - only some ten years old Google group stories popped up. Is it the right way to handle optional parameters at all?

@serialseb
Copy link
Member

I believe that we switched around 2.1 to making querystrings optional by default, but i'm unsure as to how we treat nullables together with Optional.

Optional is how VB.net implemented / implements(?) default parameters way back when.

I believe that doing (int? version = null) would have the same effect today in c#.

I'll write up some tests to try and figure out, I do agree that creating the URI from the type should just work, although of course the CreateUri method is a bit evil in the sense that it uses a service locator to locate IUriResolver, so you need to be within the context of a request to get the base uri injeected correctly.

@JornWildt
Copy link
Author

I believe that doing (int? version = null) would have the same effect today in c#.

That is very likely. But we tried that first with CreateUri() - and that failed too.

@serialseb
Copy link
Member

Oooooh, i like bugs like that, russian dolls. I'll get to it tomorrow.

If you want to try and have a go at failign tests, dont hesitate to look at IUriResolver_Specificaiton.cs (i think its named something like that) that already has a whole lot of scenarios. If you could add a failing test to those, that'd be enormously helpful!

@JornWildt
Copy link
Author

Seems like IUriResolver_Specificaiton.cs illustrates the problem quite well by itself ... I cannot see a single test that looks at the code-definition of the actual resource handler. The tests only relates URL templates to CreateUri() ... and without knowing enough about the internals of OpenRasta, it indicates that CreateUri() doesn't look at the handler code at all - it only knows about resources types and URL templates - nothing about handlers.

I wouldn't know how to add a test with a given handler :-)

So maybe the solution is in the format of the URL templates? Should my template have been something like "distributed-requisitions/feature-set?version={?version}" instead?

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

No branches or pull requests

2 participants