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

Documentation for MP GraphQL #1515

Closed
andymc12 opened this issue May 6, 2020 · 21 comments
Closed

Documentation for MP GraphQL #1515

andymc12 opened this issue May 6, 2020 · 21 comments
Assignees
Labels
22.0.0.6 peer reviewed technical reviewed An SME reviewed and approved the documentation from a technical perspective.
Milestone

Comments

@andymc12
Copy link
Contributor

andymc12 commented May 6, 2020

The MicroProfile GraphQL 1.0 feature (OpenLiberty/open-liberty#7956) should be documented. Aside from the automatic feature documentation, I was thinking that we should probably have a "concept page" that describes what GraphQL is and some of the basics on how to use it in Open Liberty. We may want it to provide links to the MicroProfile GraphQL specification document and/or javadocs.

In addition, we should document that if the mpGraphQL-1.0 and mpMetrics-2.3 features are used together, then Open Liberty will track usage statistics of application queries and mutations include a "hit count" and total elapsed time. This currently goes beyond the MP GraphQL 1.0 specification.

Lastly, we should document how authorization will work with GraphQL. Like JAX-RS, the authentication will be handled by the web container, but authorization can be controlled by using @RolesAllowed("role1", "role2"), @DenyAll, and @PermitAll annotations on the GraphQL application classes. This is very similar to how JAX-RS authorization works. @chirp1 and I discussed that it might be a good idea to have one document that describes these annotations and how they work in general - and then allow other pages (like GraphQL, JAX-RS, gRPC, etc.) to link to it and provide details of their own component-specific nuances.

Note that there is a guide planned for MP GraphQL at OpenLiberty/guides-common#423

There is also a sample application (with very minimal doc) at: https://github.com/OpenLiberty/sample-mp-graphql

and I used that sample in a webcast that will have more background info at: https://event.on24.com/wcc/r/2288357/E65C1931FEA0A73CF2A5B49D701E5F4F (best if viewed using Chrome).

@chirp1
Copy link
Contributor

chirp1 commented May 12, 2020

Here is the epic with a link to the design doc: OpenLiberty/open-liberty#7956

@andymc12
Copy link
Contributor Author

andymc12 commented May 12, 2020

For the integration with mpMetrics-2.3, we probably only need a paragraph that tells users that metrics for GraphQL applications will be collected if they enable the mpGraphQL-1.0 and mpMetrics-2.3 features. For viewing the collected metrics, they would need to go to https://localhost:<http_or_https_port>/metrics(more info in the enabling vendor metrics section of the MP Metrics guide).

For integration with app security (authorization), @chirp1 mentioned that she has a draft for the general-purpose authorization page. I think that looks good, and so I think that the MP GraphQL doc can reference that for general setup of authorization, and then mention that the @DenyAll, @PermitAll, and @RolesAllowed("role1", "role2", ...) annotations can apply to all application classes annotated with @GraphQLApi and the query/mutation methods (annotated with @Query or @Mutation) in those classes.

The general-purpose authorization page should also include information such as the following:

Annotations on a method override the annotations on a class. For example:

@DenyAll
public class MyClass {
    public SomeResponse method1() { ... }

    @PermitAll
    public SomeResponse method2() { ... }

    @RolesAllowed("Administrators")
    public SomeResponse method3() { ... }
}

In this example, nobody would be allowed to access method1() , but everybody would be allowed to access method2, and only users in the "Administrators" role would be able to access method3.

The GraphQL Authorization section could also include this link as a reference ( https://graphql.org/learn/authorization/ ) - it might be worth noting that this article uses JavaScript, but that the general concept of performing authorization in the business logic layer is considered a best practice.

Wrt to the sample application - the webcast goes into some level of detail about how to create a GraphQL application. The first ~19 minutes of the webcast describes why a user might choose to use GraphQL. At about 19:07 (to get there fast, you'll need to open the "Media Player" widget and then move the slider to almost the halfway point - also, the webcast is finicky about browsers - I had the best luck using Chrome), I talk about the sample application.

Here are some other useful links that might be useful for background/reference materials:
https://graphql.org/learn/ - a great tutorial on GraphQL in general (not specific to Liberty, MicroProfile or even Java)
https://download.eclipse.org/microprofile/microprofile-graphql-1.0.2/microprofile-graphql.html - the MP GraphQL specification document (also available in PDF format)
https://download.eclipse.org/microprofile/microprofile-graphql-1.0.2/apidocs/ - MP GraphQL JavaDocs
https://pages.github.ibm.com/WASL3/site/STE/WAS/mpGraphQL-1.0/ - STE (Skills Transfer Education) for L2/L3 (IBM-only)
https://ibm.box.com/s/eb5e4vreamdbvdrfddt3sge6l3awbry6 - WAD

@chirp1
Copy link
Contributor

chirp1 commented May 19, 2020

Here is David's general authorization topic that I mentioned to Andy and that he references previoualy in this issue: https://draft-openlibertyio.mybluemix.net/docs/ref/general/#authorization.html

@dmuelle dmuelle added this to the 21.0.0.12 milestone Sep 30, 2021
@dmuelle dmuelle removed this from the 21.0.0.12 milestone Oct 8, 2021
@andymc12
Copy link
Contributor Author

@WhiteCat22 - this might be something to work on as part of MP GraphQL 2.0 (#19529) - same doc for 1.0 and 2.0 (only package namespace change)

@dmuelle dmuelle self-assigned this Feb 2, 2022
@dmuelle dmuelle added this to the 22.0.0.3 milestone Feb 2, 2022
@dmuelle
Copy link
Member

dmuelle commented Feb 2, 2022

tentatively targeting for 22.0.0.3 22.0.0.4 release- here is the epic for 2.0: OpenLiberty/open-liberty#19529

@dmuelle
Copy link
Member

dmuelle commented Feb 23, 2022

@dmuelle
Copy link
Member

dmuelle commented Feb 23, 2022

Hi @WhiteCat22 @jim-krueger - I think Andy has provided plenty of resources here for us to write a concise overview on MP GraphQL on Open Liberty- what it is, why/when you'd use it, and what you can do with it. We can also explain what GraphiQL is andd how to use it. For actual examples and implementation details we can point out to the GraphQL guide.

However, the guide doesn't cover the Metrics integration or how to set up authorization so I think we'll want to include an explanation of each of those.

  1. For the Metrics implementation- all the existing info calls out Metrics 2.3. Is everything still the same for MpMetrics 4.0?
  2. For authorization, Andy says in a blog post that "restricting access to certain queries/mutations...is under consideration for a future version of the spec". Has that been implemented yet? If not we have some doc for defining roles in an app that we can point to and update/enhance if needed. Andy mentioned the need to call out that annotations on a method override the annotations on a class, so I can update our doc to make that clear.
  3. Lastly, I found a blog post that mentions the GraphQL client API, which at the time was expected in a future release. Has that been implemented? Do we need to doc anything around that?

Let me know what you think when you have a chance. Since this doc isn't necessarily specific to MPGraphQL 2.0, it could publish sooner than the feature release. If it takes longer to develop or includes info specific to 2.0, it can wait until the feature is GA.

Thanks!

@jim-krueger
Copy link
Member

jim-krueger commented Feb 23, 2022

Hi @dmuelle,
Metrics is not part of GraphQL 2.0. Any information you require on that will need to be directed to @Channyboy and would require its own issue.

@Channyboy
Copy link
Contributor

Components that wish to integrate with metrics perform the integration themselves.
i.e. I saw that Andy made MetricServices

For for graphq-1.0 it seems it it works with Metrics 2.3 and 3.0.

@jim-krueger
Copy link
Member

@dmuelle, Have you seen the information in this issue? OpenLiberty/open-liberty#19636

@jim-krueger
Copy link
Member

I do not believe that much/anything has changed since the beta.

@dmuelle
Copy link
Member

dmuelle commented Feb 24, 2022

thanks @jim-krueger - the beta info says that it is the same functionality as 1.0 so that answers most of my questions. Based on that and @Channyboy's comment, I would assume that mpGraphql-2.0 integrates with mpMetrics 2.3 and 3.0, but not 4.0. David is that correct as far as you can tell? Just want to figure out how to doc the metrics integration since it doesn't appear to be covered elsewhere

@dmuelle
Copy link
Member

dmuelle commented Mar 2, 2022

Based on discussion from MP scrum, GraphQL 2.0 will integrate with Metrics 4.0

@dmuelle dmuelle modified the milestones: 22.0.0.3, 22.0.0.4 Mar 7, 2022
dmuelle added a commit that referenced this issue Mar 9, 2022
This was referenced Mar 9, 2022
@dmuelle dmuelle mentioned this issue Mar 16, 2022
@dmuelle
Copy link
Member

dmuelle commented Mar 16, 2022

Hi @WhiteCat22 @jim-krueger, the draft for the MP GraphQL doc is now available for review:

https://docs-draft-openlibertyio.mybluemix.net/docs/latest/microprofile-graphql.html

note that the link in the Observability and GraphQL section to MP GraphOL 2.0 won't work until we have the 22.0.0.4 generated docs (the week before GA) and that the link to the "Optimizing REST queries for microservices with GraphQL" guide at the end doesn't work from the docs draft site, because the site has only docs content.

Other than that, please let me know what edits are needed. When you're satisfied with the doc, you can sign off by adding the Technical Reviewed label and the doc will publish when the feature goes GA. Thanks!

@dmuelle
Copy link
Member

dmuelle commented Mar 31, 2022

Thanks for reviewing @ramkumar-k-9286, draft is updated:

https://draft-openlibertyio.mybluemix.net/docs/latest/microprofile-graphql.html

All suggestions implemented, except for the following:

It provides a code-first approach to development that is an alternative to, though not necessarily a replacement for, the REST architecture.

I don't think "the" is needed here, since REST architecture is referenced in a general sense, and out of a group of different architectures

GraphQL services requires less data fetching than REST services,

"services" is plural, agrees with "require"

These responses are an example of over-fetching, and the processing cost for filtering the extra data falls to the service that makes the request.

I think "cost of" is more common in this context- though I can't find a hard rule one way or the other.

The Optimizing REST queries for microservices with GraphQL link doesn't work from docs-draft because its a relative link to the blogs site. It works from the main draft site that is linked in this comment though.

@dmuelle
Copy link
Member

dmuelle commented Apr 5, 2022

changes are on vNExt and will publish with 22.0.0.4. Closing this issue

@dmuelle dmuelle closed this as completed Apr 5, 2022
@dmuelle
Copy link
Member

dmuelle commented Apr 6, 2022

Reopening as the epic moved to 22.0.0.5

@dmuelle dmuelle reopened this Apr 6, 2022
@dmuelle dmuelle added 22.0.0.5 and removed 22.0.0.4 labels Apr 6, 2022
@dmuelle dmuelle modified the milestones: 22.0.0.4, 22.0.0.5 Apr 6, 2022
dmuelle added a commit that referenced this issue Apr 6, 2022
@dmuelle
Copy link
Member

dmuelle commented Apr 6, 2022

commented out the two references to version 2.0 so the doc can publish with 22.0.0.4. Will restore when the feature GAs

@dmuelle
Copy link
Member

dmuelle commented May 31, 2022

topic is on vNExt and will publish with 22.0.0.6

@dmuelle dmuelle closed this as completed May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
22.0.0.6 peer reviewed technical reviewed An SME reviewed and approved the documentation from a technical perspective.
Projects
None yet
Development

No branches or pull requests

8 participants