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.

Sign up for GitHub

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

Jump to bottom

Flatten the public API of apollo_router::plugin #1267

Merged
merged 1 commit into from
Jun 16, 2022
Merged

Flatten the public API of apollo_router::plugin #1267

merged 1 commit into from
Jun 16, 2022

Conversation

SimonSapin
Copy link
Contributor

@SimonSapin SimonSapin commented Jun 16, 2022 •
edited
Loading

Fixes #1265

  • apollo_router::plugin::util is gone, its content inlined into apollo_router::plugin instead
  • apollo_router::plugin::test::mock and apollo_router::plugin::test::service are now private, replaced with a new apollo_router::plugin::test::MockSubgraph reexport and its existing sibling reexpports

An example involving both:

-use apollo_router::plugin::utils::test::mock::subgraph::MockSubgraph;
+use apollo_router::plugin::test::MockSubgraph;

@SimonSapin SimonSapin requested review from garypen and BrynCooke June 16, 2022 09:01
garypen
garypen requested changes Jun 16, 2022
View reviewed changes
Copy link
Contributor

@garypen garypen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cargo doc issues quite a few warnings now. No need to address private or not a hyperlink, but should address warnings like:

warning: unresolved link to `http_compat::Request`
  --> apollo-router/src/context.rs:16:21
   |
16 | /// Context for a [`http_compat::Request`]
   |                     ^^^^^^^^^^^^^^^^^^^^ no item named `http_compat` in scope
   |
   = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

@SimonSapin SimonSapin mentioned this pull request Jun 16, 2022
Closed
@SimonSapin
Copy link
Contributor Author

I see the same 10 warnings on the api-1.0 branch. I suspect the recent ones come from #1242. I’ll make a separate PR

@SimonSapin
Copy link
Contributor Author

@garypen Those are unrelated to this PR. I’m fixing some of them in #1271. Let’s discuss the remaining ones in #1150 (comment)

@SimonSapin SimonSapin enabled auto-merge (squash) June 16, 2022 16:00
@SimonSapin SimonSapin merged commit 861239e into api-1.0 Jun 16, 2022
@SimonSapin SimonSapin deleted the simon/flatten branch June 16, 2022 16:02
@SimonSapin SimonSapin mentioned this pull request Jun 17, 2022
Closed
19 tasks
@o0Ignition0o o0Ignition0o mentioned this pull request Jul 5, 2022
Merged
o0Ignition0o added a commit that referenced this pull request Jul 5, 2022
@o0Ignition0o
release: v0.10.0 (#1351)
2aafba8 
# [0.10.0] - 2022-07-05
## ❗ BREAKING ❗

### Change configuration for custom attributes for metrics in telemetry plugin ([PR #1300](#1300)

```diff
telemetry:
  metrics:
    common:
      attributes:
-        static:
-          - name: "version"
-            value: "v1.0.0"
-        from_headers:
-          - named: "content-type"
-            rename: "payload_type"
-            default: "application/json"
-          - named: "x-custom-header-to-add"
+        router:
+          static:
+            - name: "version"
+              value: "v1.0.0"
+          request:
+            header:
+              - named: "content-type"
+                rename: "payload_type"
+                default: "application/json"
+              - named: "x-custom-header-to-add"
```

By [@bnjjj](https://github.com/bnjjj) in #1300

### Rename http_compat to http_ext ([PR #1291](#1291)

The module provides extensions to the `http` crate which are specific to the way we use that crate in the router. This change also cleans up the provided extensions and fixes a few potential sources of error (by removing them)
such as the Request::mock() fn.

By [@garypen](https://github.com/garypen) in #1291

### Rework the entire public API structure ([PR #1216](#1216),  [PR #1242](#1242),  [PR #1267](#1267),  [PR #1277](#1277), [PR #1303](#1303))

* Many items have been removed from the public API and made private.
  If you still need some of them, please file an issue.

* Many reexports have been removed, 
  notably from the crate root and all of the `prelude` module.
  Corresponding items need to be imported from another location instead,
  usually the module that define them.

* Some items have moved and need to be imported from a different location.

For example, here are the changes made to `examples/add-timestamp-header/src/main.rs`:

```diff
-use apollo_router::{plugin::utils, Plugin, RouterRequest, RouterResponse};
+use apollo_router::plugin::test;
+use apollo_router::plugin::Plugin;
+use apollo_router::services::{RouterRequest, RouterResponse};
```
```diff
-let mut mock = utils::test::MockRouterService::new();
+let mut mock = test::MockRouterService::new();
```
```diff
-if let apollo_router::ResponseBody::GraphQL(response) =
+if let apollo_router::services::ResponseBody::GraphQL(response) =
     service_response.next_response().await.unwrap()
 {
```

If you’re unsure where a given item needs to be imported from when porting code,
unfold the listing below and use your browser’s search function (CTRL+F or ⌘+F).

<details>
<summary>
  Output of <code>./scripts/public_items.sh</code> for 0.10.0
</summary>
<pre>
use apollo_router::ApolloRouter;
use apollo_router::Configuration;
use apollo_router::ConfigurationKind;
use apollo_router::Context;
use apollo_router::Error;
use apollo_router::Executable;
use apollo_router::Request;
use apollo_router::Response;
use apollo_router::Schema;
use apollo_router::SchemaKind;
use apollo_router::ShutdownKind;
use apollo_router::error::CacheResolverError;
use apollo_router::error::FetchError;
use apollo_router::error::JsonExtError;
use apollo_router::error::Location;
use apollo_router::error::ParseErrors;
use apollo_router::error::PlannerErrors;
use apollo_router::error::QueryPlannerError;
use apollo_router::error::SchemaError;
use apollo_router::error::ServiceBuildError;
use apollo_router::error::SpecError;
use apollo_router::graphql::Error;
use apollo_router::graphql::NewErrorBuilder;
use apollo_router::graphql::Request;
use apollo_router::graphql::Response;
use apollo_router::json_ext::Object;
use apollo_router::json_ext::Path;
use apollo_router::json_ext::PathElement;
use apollo_router::layers::ServiceBuilderExt;
use apollo_router::layers::ServiceExt;
use apollo_router::layers::async_checkpoint::AsyncCheckpointLayer;
use apollo_router::layers::async_checkpoint::AsyncCheckpointService;
use apollo_router::layers::cache::CachingLayer;
use apollo_router::layers::cache::CachingService;
use apollo_router::layers::instrument::InstrumentLayer;
use apollo_router::layers::instrument::InstrumentService;
use apollo_router::layers::map_future_with_context::MapFutureWithContextLayer;
use apollo_router::layers::map_future_with_context::MapFutureWithContextService;
use apollo_router::layers::sync_checkpoint::CheckpointLayer;
use apollo_router::layers::sync_checkpoint::CheckpointService;
use apollo_router::main;
use apollo_router::mock_service;
use apollo_router::plugin::DynPlugin;
use apollo_router::plugin::Handler;
use apollo_router::plugin::Plugin;
use apollo_router::plugin::PluginFactory;
use apollo_router::plugin::plugins;
use apollo_router::plugin::register_plugin;
use apollo_router::plugin::serde::deserialize_header_name;
use apollo_router::plugin::serde::deserialize_header_value;
use apollo_router::plugin::serde::deserialize_option_header_name;
use apollo_router::plugin::serde::deserialize_option_header_value;
use apollo_router::plugin::serde::deserialize_regex;
use apollo_router::plugin::test::IntoSchema;
use apollo_router::plugin::test::MockExecutionService;
use apollo_router::plugin::test::MockQueryPlanningService;
use apollo_router::plugin::test::MockRouterService;
use apollo_router::plugin::test::MockSubgraph;
use apollo_router::plugin::test::MockSubgraphService;
use apollo_router::plugin::test::NewPluginTestHarnessBuilder;
use apollo_router::plugin::test::PluginTestHarness;
use apollo_router::plugins::csrf::CSRFConfig;
use apollo_router::plugins::csrf::Csrf;
use apollo_router::plugins::rhai::Conf;
use apollo_router::plugins::rhai::Rhai;
use apollo_router::plugins::telemetry::ROUTER_SPAN_NAME;
use apollo_router::plugins::telemetry::Telemetry;
use apollo_router::plugins::telemetry::apollo::Config;
use apollo_router::plugins::telemetry::config::AttributeArray;
use apollo_router::plugins::telemetry::config::AttributeValue;
use apollo_router::plugins::telemetry::config::Conf;
use apollo_router::plugins::telemetry::config::GenericWith;
use apollo_router::plugins::telemetry::config::Metrics;
use apollo_router::plugins::telemetry::config::MetricsCommon;
use apollo_router::plugins::telemetry::config::Propagation;
use apollo_router::plugins::telemetry::config::Sampler;
use apollo_router::plugins::telemetry::config::SamplerOption;
use apollo_router::plugins::telemetry::config::Trace;
use apollo_router::plugins::telemetry::config::Tracing;
use apollo_router::query_planner::OperationKind;
use apollo_router::query_planner::QueryPlan;
use apollo_router::query_planner::QueryPlanOptions;
use apollo_router::register_plugin;
use apollo_router::services::ErrorNewExecutionResponseBuilder;
use apollo_router::services::ErrorNewQueryPlannerResponseBuilder;
use apollo_router::services::ErrorNewRouterResponseBuilder;
use apollo_router::services::ErrorNewSubgraphResponseBuilder;
use apollo_router::services::ExecutionRequest;
use apollo_router::services::ExecutionResponse;
use apollo_router::services::ExecutionService;
use apollo_router::services::FakeNewExecutionRequestBuilder;
use apollo_router::services::FakeNewExecutionResponseBuilder;
use apollo_router::services::FakeNewRouterRequestBuilder;
use apollo_router::services::FakeNewRouterResponseBuilder;
use apollo_router::services::FakeNewSubgraphRequestBuilder;
use apollo_router::services::FakeNewSubgraphResponseBuilder;
use apollo_router::services::NewExecutionRequestBuilder;
use apollo_router::services::NewExecutionResponseBuilder;
use apollo_router::services::NewExecutionServiceBuilder;
use apollo_router::services::NewQueryPlannerRequestBuilder;
use apollo_router::services::NewQueryPlannerResponseBuilder;
use apollo_router::services::NewRouterRequestBuilder;
use apollo_router::services::NewRouterResponseBuilder;
use apollo_router::services::NewRouterServiceBuilder;
use apollo_router::services::NewSubgraphRequestBuilder;
use apollo_router::services::NewSubgraphResponseBuilder;
use apollo_router::services::PluggableRouterServiceBuilder;
use apollo_router::services::QueryPlannerContent;
use apollo_router::services::QueryPlannerRequest;
use apollo_router::services::QueryPlannerResponse;
use apollo_router::services::ResponseBody;
use apollo_router::services::RouterRequest;
use apollo_router::services::RouterResponse;
use apollo_router::services::RouterService;
use apollo_router::services::SubgraphRequest;
use apollo_router::services::SubgraphResponse;
use apollo_router::services::SubgraphService;
use apollo_router::services::http_ext::FakeNewRequestBuilder;
use apollo_router::services::http_ext::IntoHeaderName;
use apollo_router::services::http_ext::IntoHeaderValue;
use apollo_router::services::http_ext::NewRequestBuilder;
use apollo_router::services::http_ext::Request;
use apollo_router::services::http_ext::Response;
use apollo_router::subscriber::RouterSubscriber;
use apollo_router::subscriber::is_global_subscriber_set;
use apollo_router::subscriber::replace_layer;
use apollo_router::subscriber::set_global_subscriber;
</pre>
</details>

By [@SimonSapin](https://github.com/SimonSapin)

### Entry point improvements ([PR #1227](#1227)) ([PR #1234](#1234)) ([PR #1239](#1239)) ([PR #1263](#1263))

The interfaces around the entry point have been improved for naming consistency and to enable reuse when customization is required. 
Most users will continue to use:
```rust
apollo_router::main()  
```

However, if you want to specify extra customization to configuration/schema/shutdown then you may use `Executable::builder()` to override behavior. 

```rust
use apollo_router::Executable;
Executable::builder()
  .router_builder_fn(|configuration, schema| ...) // Optional
  .start().await?
```

Migration tips:
* Calls to `ApolloRouterBuilder::default()` should be migrated to `ApolloRouter::builder`.
* `FederatedServerHandle` has been renamed to `ApolloRouterHandle`.
* The ability to supply your own `RouterServiceFactory` has been removed.
* `StateListener`. This made the internal state machine unnecessarily complex. `listen_address()` remains on `ApolloRouterHandle`.
* `FederatedServerHandle::shutdown()` has been removed. Instead, dropping `ApolloRouterHandle` will cause the router to shutdown.
* `FederatedServerHandle::ready()` has been renamed to `FederatedServerHandle::listen_address()`, it will return the address when the router is ready to serve requests.
* `FederatedServerError` has been renamed to `ApolloRouterError`.
* `main_rt` should be migrated to `Executable::builder()`

By [@BrynCooke](https://github.com/bryncooke) in #1227 #1234 #1239 #1263

### Non-GraphQL response body variants removed from `RouterResponse` ([PR #1307](#1307), [PR #1328](#1328))

The `ResponseBody` enum has been removed.
It had variants for GraphQL and non-GraphQL responses.

It was used:

* In `RouterResponse` which now uses `apollo_router::graphql::Response` instead
* In `Handler` for plugin custom endpoints which now uses `bytes::Bytes` instead

Various type signatures will need changes such as:

```diff
- RouterResponse<BoxStream<'static, ResponseBody>>
+ RouterResponse<BoxStream<'static, graphql::Response>>
```

Necessary code changes might look like:

```diff
- return ResponseBody::GraphQL(response);
+ return response;
```
```diff
- if let ResponseBody::GraphQL(graphql_response) = res {
-     assert_eq!(&graphql_response.errors[0], expected_error);
- } else {
-     panic!("expected a graphql response");
- }
+ assert_eq!(&res.errors[0], expected_error);
```

By [@SimonSapin](https://github.com/SimonSapin)

### Fixed control flow in helm chart for volume mounts & environment variables ([PR #1283](#1283))

You will now be able to actually use the helm chart without being on a managed graph. 

By [@LockedThread](https://github.com/LockedThread) in #1283


### Deny unknown fields on configuration [PR #1278](#1278)
Do not silently skip some bad configuration, now if you add an unknown configuration field at the root of your configuration file it will return an error.

By [@bnjjj](https://github.com/bnjjj) in #1278

## 🚀 Features ( 🚀 )

### Add support to add custom attributes on metrics. [PR #1300](#1300)

Previously, it was only possible to add custom attributes coming from headers from router request. Now you are able to add custom attributes coming from headers and body of router response/request and subgraph response/request. You also have the ability to add an attribute coming from the context. Example:

```yaml
telemetry:
  metrics:
    common:
      attributes:
        router:
          static:
            - name: "version"
              value: "v1.0.0"
          request:
            header:
              - named: "content-type"
                rename: "payload_type"
                default: "application/json"
              - named: "x-custom-header-to-add"
          response:
            body:
              # Take element from the response body of the router located at this path
              - path: .errors[0].extensions.status
                name: error_from_body
          context:
            # Take element from context in plugin chains and add it in attributes
            - named: my_key
        subgraph:
          all:
            static:
              # Always insert on all metrics for all subgraphs
              - name: kind
                value: subgraph_request
          subgraphs:
            my_subgraph_name: # Apply these rules only for the subgraph named `my_subgraph_name`
              request:
                header:
                  - named: "x-custom-header"
                body:
                  # Take element from the request body of the router located at this path (here it's the query)
                  - path: .query
                    name: query
                    default: UNKNOWN
```

By [@bnjjj](https://github.com/bnjjj) in #1300

### Add support for modifying variables from a plugin. [PR #1257](#1257)

Previously, it was not possible to modify variables in a `Request` from a plugin. This is now supported in both Rust and Rhai plugins.

By [@garypen](https://github.com/garypen) in #1257

## 🐛 Fixes


### extend fix for compression support to DIY dockerfiles ([PR #1352](#1352))

Compression support is now shown in the DIY dockerfiles (followup to [PR #1279](#1279)).

By [@garypen](https://github.com/garypen) in #1352

### Improve URL parsing in endpoint configuration ([PR #1341](#1341))

Specifying an endpoint in this form '127.0.0.1:431' resulted in an error: 'relative URL without a base'. The fix enhances the URL parsing logic to check for these errors and re-parses with a default scheme 'http://' so that parsing succeeds.

By [@garypen](https://github.com/garypen) in #1341

### Improve configuration validation and environment expansion ([PR #1331](#1331))

Environment expansion now covers the entire configuration file, and supports non string types.
This means that it is now possible to use environment variable in the server section of the yaml, and also in numeric and boolean fields.

Environment variables will always be shown in their original form in error messages preventing leakage of secrets.

These changes allow more of the configuration file to be validated via json schema, as previously we just skipped errors where fields contained env variables.

By [@BrynCooke](https://github.com/bryncooke) in #1331

### Fix input coercion for a list ([PR #1327](#1327))

The router is now following coercion rules for List regarding [the specs](https://spec.graphql.org/June2018/#sec-Type-System.List). Especially it fixes the case when for an input type `[Int]` only `1` was provided as a value. It's now working and it's coerced to `[1]`.

By [@bnjjj](https://github.com/bnjjj) in #1327

### Returns HTTP 400 bad request instead of 500 when it's a query plan error ([PR #1321](#1321))

By [@bnjjj](https://github.com/bnjjj) in #1321

### Re-enable the subgraph error redaction functionality ([PR #1317](#1317))

In a re-factoring the "include_subgraph_errors" plugin was disabled. This meant that subgraph error handling was not working as intended. This change re-enables it and improves the functionality with additional logging. As part of the fix, the plugin initialisation mechanism was improved to ensure that plugins start in the required sequence.

By [@garypen](https://github.com/garypen) in #1317

### Restrict static introspection to only `__schema` and `__type` ([PR #1299](#1299))
Queries with selected field names starting with `__` are recognized as introspection queries. This includes `__schema`, `__type` and `__typename`. However, `__typename` is introspection at query time which is different from `__schema` and `__type` because two of the later can be answered with queries with empty input variables. This change will restrict introspection to only `__schema` and `__type`.

By [@dingxiangfei2009](https://github.com/dingxiangfei2009) in #1299

### Fix scaffold support ([PR #1293](#1293))

By [@garypen](https://github.com/garypen) in #1293

### Support introspection object types ([PR #1240](#1240))

Introspection queries can use a set of object types defined in the specification. The query parsing code was not recognizing them,
resulting in some introspection queries not working.

By [@Geal](https://github.com/Geal) in #1240

### Update the scaffold template so it works with streams ([#1247](#1247))

Release v0.9.4 changed the way we deal with Response objects, which can now be streams.
This Pull request updates the scaffold template so it generates plugins that are compatible with the new Plugin API.

By [@o0Ignition0o](https://github.com/o0Ignition0o) in #1248


### Fix fragment selection on interfaces ([PR #1295](#1295))

Fragments type conditions were not checked correctly on interfaces, resulting in invalid null fields added to the response
or valid data being nullified.

By [@Geal](https://github.com/Geal) in #1295

### Fix fragment selection on queries ([PR #1296](#1296))

The schema object can specify objects for queries, mutations or subscriptions that are not named `Query`, `Mutation` or
`Subscription`. Response formatting now supports it.

By [@Geal](https://github.com/Geal) in #1296

### Fix fragment selection on unions ([PR #1346](#1346))

Fragments type conditions were not checked correctly on unions, resulting in data being absent.

By [@Geal](https://github.com/Geal) in #1346

### Reduce `poll_ready` calls in query deduplication ([PR #1350](#1350))

The query deduplication service was making assumptions on the underlying service's behaviour, which could result in
subgraph services panicking.

By [@Geal](https://github.com/Geal) in #1350

## 🛠 Maintenance ( 🛠️ )

### chore: Run scaffold tests in CI and xtask only ([PR #1345](#1345))

Run the scaffold tests in CI and through xtask, to keep a steady feedback loop while developping against `cargo test`.

By [@o0Ignition0o](https://github.com/o0Ignition0o) in #1345

### Update rhai to latest release (1.8.0)  ([PR #1337](#1337)

We had been depending on a pinned git version which had a fix we required. This now updates to the latest release.

By [@garypen](https://github.com/garypen) in #1337

### Remove typed-builder ([PR #1218](#1218))
Migrate all typed-builders code to buildstructor
By [@BrynCooke](https://github.com/bryncooke) in #1218
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnjjj bnjjj bnjjj approved these changes

@garypen garypen garypen approved these changes

@BrynCooke BrynCooke Awaiting requested review from BrynCooke

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SimonSapin @garypen @bnjjj