davsclaus commented on code in PR #27589:
URL: https://github.com/apache/camel/pull/27589#discussion_r4227328798
##########
components/camel-http/src/main/java/org/apache/camel/component/http/HttpComponent.java:
##########
@@ -299,6 +300,22 @@ private HttpClientConfigurer configureOAuth2Authentication(
"oauth2BodyAuthentication",
boolean.class,
configDefaults.isOauth2BodyAuthentication());
+ // not removed from the parameters, so the endpoint gets the option too
+ OAuth2CachedTokensKeyResolver cachedTokensKeyResolver = null;
+ Object resolver = parameters.get("oauth2CachedTokensKeyResolver");
+ if (resolver instanceof OAuth2CachedTokensKeyResolver r) {
+ cachedTokensKeyResolver = r;
+ } else if (resolver != null) {
+ cachedTokensKeyResolver = EndpointHelper.resolveReferenceParameter(
+ getCamelContext(), resolver.toString(),
OAuth2CachedTokensKeyResolver.class);
+ }
Review Comment:
Since `oauth2CachedTokensKeyResolver` stays in the parameters,
`setProperties(endpoint, parameters)` resolves it again for the endpoint field.
For `#myResolver` that's the same bean, but with `#class:...` the endpoint ends
up with a second instance that isn't the one used by the configurer. Putting
the resolved instance back into the parameters would make both use the same
one. Non-blocking.
```suggestion
} else if (resolver != null) {
cachedTokensKeyResolver =
EndpointHelper.resolveReferenceParameter(
getCamelContext(), resolver.toString(),
OAuth2CachedTokensKeyResolver.class);
// bind the same instance on the endpoint rather than resolving
the reference again
parameters.put("oauth2CachedTokensKeyResolver",
cachedTokensKeyResolver);
}
```
##########
components/camel-http/src/main/java/org/apache/camel/component/http/OAuth2CachedTokensKey.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.component.http;
+
+import java.net.URI;
+import java.util.Locale;
+
+/**
+ * The built-in strategies for the cache key of OAuth2 tokens, see {@link
OAuth2CachedTokensKeyResolver}.
+ */
+public enum OAuth2CachedTokensKey implements OAuth2CachedTokensKeyResolver {
+
+ /**
+ * One token per scheme, host and port.
+ */
+ HOST_ONLY {
+ @Override
+ public String resolveKey(URI requestUri) {
+ return authority(requestUri);
+ }
+ },
+ /**
+ * One token per scheme, host, port and path. The query is not taken into
account.
+ */
+ HOST_AND_PATH {
+ @Override
+ public String resolveKey(URI requestUri) {
+ String path = requestUri.getRawPath();
+ return authority(requestUri) + (path != null ? path : "");
+ }
+ },
+ /**
+ * One token per request URI, including the query (the default).
+ */
+ FULL_URI {
+ @Override
+ public String resolveKey(URI requestUri) {
+ return requestUri.toString();
+ }
+ };
+
+ private static String authority(URI uri) {
+ String scheme = uri.getScheme() != null ?
uri.getScheme().toLowerCase(Locale.ROOT) : "";
+ String host = uri.getHost() != null ?
uri.getHost().toLowerCase(Locale.ROOT) : "";
+ return scheme + "://" + host + ":" + effectivePort(uri.getPort(),
scheme);
+ }
+
+ private static int effectivePort(int port, String scheme) {
Review Comment:
`effectivePort` duplicates `OAuth2ClientConfigurer.effectivePort(URI)`.
Could both use one package-private static helper so the default-port rules
can't drift apart? Non-blocking.
##########
components/camel-http/src/main/docs/http-component.adoc:
##########
@@ -672,6 +672,63 @@ YAML::
NOTE: Resource Indicator is the URL to the actual endpoint as defined in the
component URI.
+==== Caching tokens
+
+With `oauth2CacheTokens=true`, Camel caches the token until it expires (see
`oauth2CachedTokensDefaultExpirySeconds`
+and `oauth2CachedTokensExpirationMarginSeconds`). The option
`oauth2CachedTokensKey` decides which requests share a
+cached token:
+
+* `FULL_URI` (default): one token per request URI, including the query.
+* `HOST_AND_PATH`: one token per path. The query is not taken into account.
+* `HOST_ONLY`: one token shared by all requests to the same scheme, host and
port.
+
+The client id, client secret, token endpoint, scope and resource indicator are
always part of the key. The token
+request itself does not depend on the request URI, so with dynamic paths or
queries (for example with `toD`),
+`HOST_AND_PATH` or `HOST_ONLY` avoid requesting a new token for every new URI.
Review Comment:
Small precision point: `HOST_AND_PATH` only helps with dynamic queries; with
dynamic paths (as in the `toD` example below) only `HOST_ONLY` avoids a new
token per URI. Maybe: "...so with dynamic queries `HOST_AND_PATH`, and with
dynamic paths (for example with `toD`) `HOST_ONLY`, avoid requesting a new
token for every new URI." (The catalog copy needs regenerating as well.)
##########
components/camel-http/src/main/docs/http-component.adoc:
##########
@@ -672,6 +672,63 @@ YAML::
NOTE: Resource Indicator is the URL to the actual endpoint as defined in the
component URI.
+==== Caching tokens
Review Comment:
The new `==== Caching tokens` subsection is inserted before the general
OAuth2 `[NOTE]` (client credentials flow only) and `[IMPORTANT]` (no token
validation) blocks, so those two notes now render as part of "Caching tokens".
Could you move this subsection below the `[IMPORTANT]` block, right before `===
Vault-backed credential rotation` (or move the two notes above it)? Please
regenerate the catalog copy
(`catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/http-component.adoc`)
with it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
