From 6767f7010c1f1e0f8da41c46586c0f1eb8b5f75b Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 12 Mar 2024 10:40:08 +0000 Subject: [PATCH] Polishing contribution Closes gh-29958 --- .../client/reactive/ClientHttpRequest.java | 13 +++++++------ .../reactive/ClientHttpRequestDecorator.java | 12 ++++++------ .../HttpComponentsClientHttpRequest.java | 2 +- .../reactive/JettyClientHttpRequest.java | 17 +++++++++-------- .../reactive/ReactorClientHttpConnector.java | 11 ++++++++++- .../reactive/ReactorClientHttpRequest.java | 18 ++++++++---------- .../ReactorNetty2ClientHttpConnector.java | 11 ++++++++++- .../ReactorNetty2ClientHttpRequest.java | 16 +++++++--------- .../client/DefaultClientRequestBuilder.java | 5 +---- .../client/WebClientIntegrationTests.java | 7 +++---- 10 files changed, 62 insertions(+), 50 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpRequest.java index 770a5eaeb0..9f8c7aa17f 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -48,6 +48,12 @@ public interface ClientHttpRequest extends ReactiveHttpOutputMessage { */ MultiValueMap getCookies(); + /** + * Return a mutable map of the request attributes. + * @since 6.2 + */ + Map getAttributes(); + /** * Return the request from the underlying HTTP library. * @param the expected type of the request to cast to @@ -55,9 +61,4 @@ public interface ClientHttpRequest extends ReactiveHttpOutputMessage { */ T getNativeRequest(); - /** - * Return a mutable map of the request attributes. - */ - Map getAttributes(); - } diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpRequestDecorator.java b/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpRequestDecorator.java index ade5faa44a..388c628271 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpRequestDecorator.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpRequestDecorator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -76,6 +76,11 @@ public class ClientHttpRequestDecorator implements ClientHttpRequest { return this.delegate.getCookies(); } + @Override + public Map getAttributes() { + return this.delegate.getAttributes(); + } + @Override public DataBufferFactory bufferFactory() { return this.delegate.bufferFactory(); @@ -86,11 +91,6 @@ public class ClientHttpRequestDecorator implements ClientHttpRequest { return this.delegate.getNativeRequest(); } - @Override - public Map getAttributes() { - return this.delegate.getAttributes(); - } - @Override public void beforeCommit(Supplier> action) { this.delegate.beforeCommit(action); diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpRequest.java index cd95213ca0..6e7f4ac3dc 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpRequest.java @@ -163,7 +163,7 @@ class HttpComponentsClientHttpRequest extends AbstractClientHttpRequest { @Override protected void applyAttributes() { getAttributes().forEach((key, value) -> { - if(this.context.getAttribute(key) == null) { + if (this.context.getAttribute(key) == null) { this.context.setAttribute(key, value); } }); diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpRequest.java index 3003651546..573e34e4d6 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpRequest.java @@ -138,14 +138,6 @@ class JettyClientHttpRequest extends AbstractClientHttpRequest { .forEach(this.jettyRequest::cookie); } - /** - * Applies the attributes to {@link Request#getAttributes()}. - */ - @Override - protected void applyAttributes() { - getAttributes().forEach(this.jettyRequest::attribute); - } - @Override protected void applyHeaders() { HttpHeaders headers = getHeaders(); @@ -162,6 +154,15 @@ class JettyClientHttpRequest extends AbstractClientHttpRequest { return HttpHeaders.readOnlyHttpHeaders(new JettyHeadersAdapter(this.jettyRequest.getHeaders())); } + @Override + protected void applyAttributes() { + getAttributes().forEach((key, value) -> { + if (this.jettyRequest.getAttributes().get(key) == null) { + this.jettyRequest.attribute(key, value); + } + }); + } + public ReactiveRequest toReactiveRequest() { return this.builder.build(); } diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpConnector.java b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpConnector.java index 5f1def633a..9647211faf 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpConnector.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpConnector.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,9 +17,11 @@ package org.springframework.http.client.reactive; import java.net.URI; +import java.util.Map; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; +import io.netty.util.AttributeKey; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Mono; @@ -49,6 +51,13 @@ import org.springframework.util.Assert; */ public class ReactorClientHttpConnector implements ClientHttpConnector, SmartLifecycle { + /** + * Channel attribute key under which {@code WebClient} request attributes are stored as a Map. + * @since 6.2 + */ + public static final AttributeKey> ATTRIBUTES_KEY = + AttributeKey.valueOf(ReactorClientHttpRequest.class.getName() + ".ATTRIBUTES"); + private static final Log logger = LogFactory.getLog(ReactorClientHttpConnector.class); private static final Function defaultInitializer = client -> client.compress(true); diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpRequest.java index 5717637d3c..83e081c3c9 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,11 +18,9 @@ package org.springframework.http.client.reactive; import java.net.URI; import java.nio.file.Path; -import java.util.Map; import io.netty.buffer.ByteBuf; import io.netty.handler.codec.http.cookie.DefaultCookie; -import io.netty.util.AttributeKey; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -48,8 +46,6 @@ import org.springframework.http.support.Netty4HeadersAdapter; */ class ReactorClientHttpRequest extends AbstractClientHttpRequest implements ZeroCopyHttpOutputMessage { - public static final String ATTRIBUTES_CHANNEL_KEY = "attributes"; - private final HttpMethod httpMethod; private final URI uri; @@ -141,14 +137,16 @@ class ReactorClientHttpRequest extends AbstractClientHttpRequest implements Zero } /** - * Applies the request attributes to the {@link reactor.netty.http.client.HttpClientRequest} by setting - * a single {@link Map} into the {@link reactor.netty.channel.ChannelOperations#channel()}, - * with {@link io.netty5.util.AttributeKey#name()} equal to {@link #ATTRIBUTES_CHANNEL_KEY}. + * Saves the {@link #getAttributes() request attributes} to the + * {@link reactor.netty.channel.ChannelOperations#channel() channel} as a single map + * attribute under the key {@link ReactorClientHttpConnector#ATTRIBUTES_KEY}. */ @Override protected void applyAttributes() { - ((ChannelOperations) this.request) - .channel().attr(AttributeKey.valueOf(ATTRIBUTES_CHANNEL_KEY)).set(getAttributes()); + if (!getAttributes().isEmpty()) { + ((ChannelOperations) this.request).channel() + .attr(ReactorClientHttpConnector.ATTRIBUTES_KEY).set(getAttributes()); + } } @Override diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorNetty2ClientHttpConnector.java b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorNetty2ClientHttpConnector.java index afe14e4f92..6a910ba055 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorNetty2ClientHttpConnector.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorNetty2ClientHttpConnector.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,9 +17,11 @@ package org.springframework.http.client.reactive; import java.net.URI; +import java.util.Map; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; +import io.netty5.util.AttributeKey; import reactor.core.publisher.Mono; import reactor.netty5.NettyOutbound; import reactor.netty5.http.client.HttpClient; @@ -41,6 +43,13 @@ import org.springframework.util.Assert; */ public class ReactorNetty2ClientHttpConnector implements ClientHttpConnector { + /** + * Channel attribute key under which {@code WebClient} request attributes are stored as a Map. + * @since 6.2 + */ + public static final AttributeKey> ATTRIBUTES_KEY = + AttributeKey.valueOf(ReactorNetty2ClientHttpRequest.class.getName() + ".ATTRIBUTES"); + private static final Function defaultInitializer = client -> client.compress(true); diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorNetty2ClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorNetty2ClientHttpRequest.java index 8c0a8e685d..c24ec75ece 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorNetty2ClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorNetty2ClientHttpRequest.java @@ -18,11 +18,9 @@ package org.springframework.http.client.reactive; import java.net.URI; import java.nio.file.Path; -import java.util.Map; import io.netty5.buffer.Buffer; import io.netty5.handler.codec.http.headers.DefaultHttpCookiePair; -import io.netty5.util.AttributeKey; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -49,8 +47,6 @@ import org.springframework.http.support.Netty5HeadersAdapter; */ class ReactorNetty2ClientHttpRequest extends AbstractClientHttpRequest implements ZeroCopyHttpOutputMessage { - public static final String ATTRIBUTES_CHANNEL_KEY = "attributes"; - private final HttpMethod httpMethod; private final URI uri; @@ -144,14 +140,16 @@ class ReactorNetty2ClientHttpRequest extends AbstractClientHttpRequest implement } /** - * Applies the request attributes to the {@link reactor.netty.http.client.HttpClientRequest} by setting - * a single {@link Map} into the {@link reactor.netty.channel.ChannelOperations#channel()}, - * with {@link AttributeKey#name()} equal to {@link #ATTRIBUTES_CHANNEL_KEY}. + * Saves the {@link #getAttributes() request attributes} to the + * {@link reactor.netty.channel.ChannelOperations#channel() channel} as a single map + * attribute under the key {@link ReactorNetty2ClientHttpConnector#ATTRIBUTES_KEY}. */ @Override protected void applyAttributes() { - ((ChannelOperations) this.request) - .channel().attr(AttributeKey.valueOf(ATTRIBUTES_CHANNEL_KEY)).set(getAttributes()); + if (!getAttributes().isEmpty()) { + ((ChannelOperations) this.request).channel() + .attr(ReactorNetty2ClientHttpConnector.ATTRIBUTES_KEY).set(getAttributes()); + } } @Override diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestBuilder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestBuilder.java index 9324f972c1..05fdfeb915 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestBuilder.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestBuilder.java @@ -266,10 +266,7 @@ final class DefaultClientRequestBuilder implements ClientRequest.Builder { })); } - Map requestAttributes = request.getAttributes(); - if (!this.attributes.isEmpty()) { - this.attributes.forEach((key, value) -> requestAttributes.put(key, value)); - } + request.getAttributes().putAll(this.attributes); if (this.httpRequestConsumer != null) { this.httpRequestConsumer.accept(request); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientIntegrationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientIntegrationTests.java index 62b1c629fe..3bbaea8809 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientIntegrationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientIntegrationTests.java @@ -41,7 +41,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import io.netty.util.Attribute; -import io.netty.util.AttributeKey; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; @@ -207,13 +206,13 @@ class WebClientIntegrationTests { StepVerifier.create(result).expectComplete().verify(); if (nativeRequest.get() instanceof ChannelOperations nativeReq) { - Attribute> attributes = nativeReq.channel().attr(AttributeKey.valueOf("attributes")); + Attribute> attributes = nativeReq.channel().attr(ReactorClientHttpConnector.ATTRIBUTES_KEY); assertThat(attributes.get()).isNotNull(); assertThat(attributes.get()).containsEntry("foo", "bar"); } else if (nativeRequest.get() instanceof reactor.netty5.channel.ChannelOperations nativeReq) { io.netty5.util.Attribute> attributes = - nativeReq.channel().attr(io.netty5.util.AttributeKey.valueOf("attributes")); + nativeReq.channel().attr(ReactorNetty2ClientHttpConnector.ATTRIBUTES_KEY); assertThat(attributes.get()).isNotNull(); assertThat(attributes.get()).containsEntry("foo", "bar"); } @@ -221,7 +220,7 @@ class WebClientIntegrationTests { assertThat(nativeReq.getAttributes()).containsEntry("foo", "bar"); } else if (nativeRequest.get() instanceof org.apache.hc.core5.http.HttpRequest nativeReq) { - // TODO get attributes from HttpClientContext + // Attributes are not in the request, but in separate HttpClientContext } }