You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "Radu Cotescu (Jira)" <ji...@apache.org> on 2021/07/21 12:18:00 UTC

[jira] [Commented] (SLING-10619) Remove sling.graphql-schema-aggregator capability if not useful

    [ https://issues.apache.org/jira/browse/SLING-10619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17384860#comment-17384860 ] 

Radu Cotescu commented on SLING-10619:
--------------------------------------

This would be the final design IMO for the capability:
 # the GraphQL Schema Aggregator bundle advertises a capability, specifying the syntax supported for the schema files
 # the provider bundles (the ones which provide schema files or partial schema files) require the capability from 1. and they also in turn advertise a capability for each schema / partial they provide
 # since provider bundles can require partials provided by other bundles, these would become required capabilities

With this in place we'd know when building a feature including the aggregator and providers if the requirements are satisfied, rather than figuring this out at runtime, when the GraphQL Schema Aggregator servlet is supposed to serve the final schema files. WDYT?

> Remove sling.graphql-schema-aggregator capability if not useful
> ---------------------------------------------------------------
>
>                 Key: SLING-10619
>                 URL: https://issues.apache.org/jira/browse/SLING-10619
>             Project: Sling
>          Issue Type: Improvement
>          Components: GraphQL
>            Reporter: Bertrand Delacretaz
>            Priority: Minor
>             Fix For: GraphQL Schema Aggregator 0.0.2
>
>
> The {{ProviderBundleTracker}} currently requires provider bundles to define an [OSGi capability|https://github.com/apache/sling-org-apache-sling-graphql-schema-aggregator/blob/04474a2fe16389c0df60471922f38e7dcfc637ef/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/ProviderBundleTracker.java#L48], I think the idea is for that class to efficiently ignore bundles which are not provider bundles.
> However I don't think this is more efficient than just having {{addingBundle(...)}} test for the {{Sling-GraphQL-Schema}} header.
> I would argue that doing the following in that method would be at least as efficient than the current code in terms of ignoring non-provider bundles:
> {code:java}
> @Override
> public Object addingBundle(Bundle bundle, BundleEvent event) {
>   final String providersPath = bundle.getHeaders().get("Sling-GraphQL-Schema");
>   if (providersPath == null) {
>     // not interested in that bundle
>     return;
>   ...
> {code}
> Currently, requiring this capability means provider bundles must declare both the {{Sling-GraphQL-Schema}} header and the {{sling.graphql-schema-aggregator}} capability. 
> Unless there are definite advantages in having both, I'd be in favor of requiring just the {{Sling-GraphQL-Schema}} header to keep things as simple as possible.
> [~radu] I know you were in favor of using the capability, if we agree that the above code has no performance impact, do you see another benefit of using the capability?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)