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

Default Overview of a JAX-RS Bean [Jersey] #3938

Closed
amihaiemil opened this issue Sep 19, 2018 · 25 comments
Closed

Default Overview of a JAX-RS Bean [Jersey] #3938

amihaiemil opened this issue Sep 19, 2018 · 25 comments

Comments

@amihaiemil
Copy link

amihaiemil commented Sep 19, 2018

I submitted this feature request to the JAX-RS API repository and Mr. Karg suggested that it should first be implemented by one of the vendors before it can be taken into consideration as a Specification standard.

What do you think of it? Would you see potential in such a feature?

@jansupol
Copy link
Contributor

Do you know about any product that does this? I am little worried about security impact when exposing the endpoints like this. Surely this should not be a default behaviour, but possibly it could be a configurable feature

@amihaiemil
Copy link
Author

amihaiemil commented Sep 20, 2018

@jansupol Github does it, for instance: https://api.github.com/

You see, that is the base API endpoint and it reveals further links. Also if you go on https://api.github.com/users/amihaiemil, it reveals further links/methods for fetching followers and so on.

I think this should be basic functionality of a REST api. As I said in the proposal, it's about navigability. I wouldn't say it's a security issue (but then again, I'm not much of an expert): the actual endpoints have to have some kind of security measures anyway, and a possible attacker will find them either way, if they really want to (just open the page, press F12 and see what calls are made in Network tab).

Then again, if you are not comfortable with this being the default behaviour, a configuration option/annotation would also be nice :D

@mkarg
Copy link
Member

mkarg commented Sep 25, 2018

My personal opinion is that this is something an API gateway (like Github has one) should provide by explicit configuration, but not necessarily an automatism provided by each microservice. So it should be an option with default being "off". Maybe @amihaiemil could provide a PR with an optional Jersey module to play so we better understand the usefulness or we could decide that it is an external third-party module one could add to Jersey if wanted, but not an internal module of Jersey?

@amihaiemil
Copy link
Author

@mkarg I like the optional module alternative, but how could an external gateway know these overviews without hardcoded configuration? :D

To my understanding, such a gateway should have more of a security role: authentication, API calls limitation, load balancing etc rather than docs and navigability. Same navigability should be provided if any gateway is put in front of the API.

@amihaiemil
Copy link
Author

@jansupol @mkarg I am willing to invest the time in providing a PR (first have to learn how Jersey is actually implemented), but only after we agree that it is wanted and exactly in what form :)

@mkarg
Copy link
Member

mkarg commented Sep 25, 2018

@amihaiemil An API gateway is a well-known concept in the microservice paradigm and I will not discuss the pattern here. There are good books how such products work, and there are plenty of such off-the-shelf-products. I doubt that someone will agree on your idea in the current state, as it is too unclear what the outcome shall be.

@amihaiemil
Copy link
Author

@mkarg Nothing seems unclear about the outcome to me, though, I think it is pretty straight forward :-??

I don't see any problems, especially if the behaviour can be configured, not necessarily be the default one. Let's see what others are saying, or if you have time I'd appreciate an e-mail with concerns that I am missing :D

@mkarg
Copy link
Member

mkarg commented Sep 25, 2018

Please note that I won't switch to email as the EF wants all discussions to be public.

The sole concern is that what you are designing is not an explicit JAX-RS or Jersey topic, but that you want to create a standard document format for REST. It makes no sense to design a document format that only JAX-RS server will implement, but which is not covered by other platforms like node.js based services.

@jansupol
Copy link
Contributor

Note that making this a default would violate the JAX-RS Spec, Section 3.7, that mandates 404.

@mkarg
Copy link
Member

mkarg commented Sep 25, 2018

@jansupol Very good point! 👍

@spericas
Copy link
Contributor

I generally like the idea of providing easy access to service meta-data. Jersey has done this in the past with its WADL support. If this feature is not enabled by default, I support the idea of trying this out in Jersey.

As for the API gateway discussion, I certainly hope developers can use Jersey to implement those too, so I don't understand why this feature could not be part of Jersey.

@amihaiemil
Copy link
Author

amihaiemil commented Sep 25, 2018

@spericas Thanks. It's not only about service meta-data, it's more about navigability, a step closer to HATEOAS.

@mkarg I don't really understand your concern:

what you are designing is not an explicit JAX-RS or Jersey topic, but that you want to create a standard document format for REST. It makes no sense to design a document format that only JAX-RS server will implement, but which is not covered by other platforms like node.js based services.

If we agree that this overview behaviour is a step closer to the REST paradigm then why not do it? JAX-RS is supposed to be an API for REST after all, not just an API for HTTP endpoints. And if other platforms like node.js don't offer this behaviour they definetely should. If they don't have it, it's their loss, they'd be the ones with a step behind JAX-RS, right?

At least we cleared that we are definetely left with the configuration alternative, since default would contradict the Spec (maybe in future versions the Spec can be edited so it allows this behaviour as a default? But that's a different discussion, I guess...).

@mkarg
Copy link
Member

mkarg commented Sep 25, 2018

Again, it makes no sense to define a JAX-RS only document format. I'm +1 for an API allowing to plug-in external formats, but I am -1 for defining a format that will not be shared by non-Java RESTservers, as the microservice universe is far bigger than JAX-RS.

@amihaiemil
Copy link
Author

@mkarg I'm not sure what you mean by "document format"? Are your refering to the example JSON overview I gave in the original ticket? That was just an example, it doesn't have to look exactly like that. What does navigation or "easy access to service meta-data" have to do with "document format"? :D

@mkarg
Copy link
Member

mkarg commented Sep 25, 2018

So what you propose is not a default document format (like WADL) but an API between Jersey and WADL, so we can use a different document format for the self-description of the service? Agreed, that would be nice.

And certainly it would be cool to have Jersey be the kernel of an API gateway. 8-)

@amihaiemil
Copy link
Author

@mkarg These overview endpoints would be offered by Jersey to be consumed by whoever, for navigability reasons. They would be part of the actual RESTful API, that simple. (exactly how Github does it with their API, see my comment above). Maybe we're talking about the same thing in different terms haha.

Another example of navigability, which I would like to see JAX-RS do somehow is the following (but this is probably to be discussed in another ticket):

Any listing endpoint, for instance a search endpoint which returns a page of results, should automatically return a few links and meta-data among the effective list of results:

  1. Number of total results.
  2. Number of the current page.
  3. Link to the previous page of results
  4. Link to the next page of results
  5. Maybe a list of all the results pages.

The page links would be differenciated by the query params index and number (index of the first result and number of results per page).

@mkarg
Copy link
Member

mkarg commented Sep 26, 2018

For me these are separate endpoints and we should first concentrate on an API which allows to externally define the actual format of documents (entities), headers, and URLs returned when performing the introspection.

@amihaiemil
Copy link
Author

@mkarg Maybe I should make a PR with what I have in mind and then we can discuss further.

@jansupol Is there a "Getting Started" guide? I suppose I have to pass through some formalities like signing some kind of "Intelectual Property" document, but aside from that is there a technical guide? I looked in the code a little and I must say I have absolutely no idea where to start from :D

@mkarg
Copy link
Member

mkarg commented Sep 27, 2018

@amihaiemil You need to do this:

  1. Register an Account with the Eclipse Foundation.
  2. Sign the Eclipse Contributor Agreement (ECA).
  3. Fork the Jersey repository on Github to be able to send PRs.
  4. Send a PR including a feasible explanation what your want to reach and why you want Jersey to adopt it.
  5. Wait for reviews. :-)

@mkarg
Copy link
Member

mkarg commented Oct 21, 2018

@amihaiemil Can you please share some insights about the progress of this issue? :-)

@amihaiemil
Copy link
Author

@mkarg Nope, didn't have time for it yet, unfortunately. Again, some kind of kick-starter Technical Documentation would be nice to have, it would spare me a lot of investigation time -- I have no idea how Jersey is actually implemented.

In the meantime, maybe we can close the original Issue from the jax-rs repo, since we have this one and the one from RedHat's Jira. WDYT?

@jansupol
Copy link
Contributor

We have a starting guide at jersey.github.io, if that helps

@amihaiemil
Copy link
Author

@jansupol thank you, looks like a good starting point indeed

@mkarg
Copy link
Member

mkarg commented Oct 22, 2018

@amihaiemil I do not see how this issue and the one from Red Hat would weigh-out the one on jax-rs. The formers are product-specific extensions, the latter aims on a general standard.

@amihaiemil
Copy link
Author

amihaiemil commented Nov 25, 2018

@mkarg @jansupol I had a look over this and now, indeed, I can't see my idea as a signifiant addition either -- it would be, as you said, just another layer on top of WADL which would be simple to implement by the users anyway (it would basically be just parsing of the wadl XML to JSON).

This said, I'm closing this Issue. Thanks all for your time and input.

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