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

ResourceBuilder should treat trailing slash optional #73

Closed
hurelhuyag opened this issue Mar 16, 2021 · 6 comments
Closed

ResourceBuilder should treat trailing slash optional #73

hurelhuyag opened this issue Mar 16, 2021 · 6 comments
Assignees

Comments

@hurelhuyag
Copy link
Contributor

I believe that trailing slash "/" character should be optional in every request URL. For instance below example URL should treated same.

http://localhost:18080/veterinarians/{id}/
http://localhost:18080/veterinarians/{id}

Currently if I use route like below snippet and call this endpoint with http://localhost:18080/veterinarians/abc/. this::veterinarian method receiving variable id=abc/. It must be id=abc (without trailing slash)

  @Override
  public Resource<?> routes() {
     return resource("VeterinarianResource",
        io.vlingo.http.resource.ResourceBuilder.get("/veterinarians/{id}")
            .param(String.class)
            .handle(this::veterinarian)
     );
  }
@VaughnVernon
Copy link
Contributor

VaughnVernon commented Mar 16, 2021

@hurelhuyag We support the Google interpretation of this. The trailing / indicates a directory, and as such, we first attempt to serve the default content for a directory, namely index.html. If the default resource is not there then the response is a 404.

https://developers.google.com/search/blog/2010/04/to-slash-or-not-to-slash

If you don't want this behavior then please don't place a / at the end of a URL. Besides, if the ending / follows a static file resource, or any other logical resource such as /resources/{id}/ rather than /resources/{id}, what would be the purpose?

Another way to think about this is that if your server and client use hypermedia (HATEOS) then you should never have to hardcode any URL except for the initial root, which is minted. From there every next request should be guided by response headers and/or response entity content. This means that the server will provide hypermedia guidance according to its own rules.

@hurelhuyag
Copy link
Contributor Author

hurelhuyag commented Mar 16, 2021

@VaughnVernon Ok. Then which one is correct for our use case, http://localhost:18080/veterinarians or http://localhost:18080/veterinarians/?

@VaughnVernon
Copy link
Contributor

VaughnVernon commented Mar 16, 2021

(1) Assuming that http://localhost:18080/veterinarians is meant to be a resource that is a collection of all individual veterinarian resources, then it is the correct URI.

(2) This URL http://localhost:18080/veterinarians/ would indicate that there is a directory named veterinarians, and we would attempt to read the index.html from that directory. That I am sure would fail with a 404.

To make another point about 1, any veterinarian resources in the response could/should have a URI for each one. In that way, you never have to build your own URI when making requests back to the server. Also, a further step to enable full hypermedia, your responses could even include request lines, such as GET ..., PATCH ..., PUT ..., and DELETE ....

Note also that such request lines would be provided as K-V pairs. For example:

  • tag="QueryVeterinarian" link="http://localhost:18080/veterinarian/82821" type="GET"

After a successful query, other options could be provided:

  • tag="veterinarianSpecialty" link="http://localhost:18080/veterinarian/82821" method="PATCH"
  • tag="veterinarianResigned" link="http://localhost:18080/veterinarian/82821" method="DELETE"

@hurelhuyag
Copy link
Contributor Author

Ok. I remembered something from xoom-starter. According to that rule, This uneditable trailing slash must be eliminated. Right?

Screenshot from 2021-03-16 11-45-40

@VaughnVernon
Copy link
Contributor

VaughnVernon commented Mar 16, 2021

Ok. I remembered something from xoom-starter. According to that rule, This uneditable trailing slash must be eliminated. Right?

Yes, I agree. I have asked numerous times for that change. I thought it had already been made. Perhaps @danilo-ambrosio has already made that change but not built the new binary artifact.

Would you please check and see if the change has been made? If not, please attempt to fix it and request @abdullahcalisir12 to review. Otherwise if it seems too difficult, please ask @abdullahcalisir12 to make the change.

@VaughnVernon
Copy link
Contributor

The URL editing issue was fixed.

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

4 participants