Browse Source

Merge branch '6.2.x'

pull/34511/head
rstoyanchev 10 months ago
parent
commit
33fef8df84
  1. 2
      spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java
  2. 24
      spring-web/src/test/java/org/springframework/web/service/invoker/HttpRequestValuesTests.java
  3. 22
      spring-web/src/test/java/org/springframework/web/service/invoker/PathVariableArgumentResolverTests.java
  4. 92
      spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitter.java

2
spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java

@ -465,7 +465,7 @@ public class HttpRequestValues { @@ -465,7 +465,7 @@ public class HttpRequestValues {
UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUriString(uriTemplate);
for (Map.Entry<String, List<String>> entry : requestParams.entrySet()) {
String nameVar = entry.getKey().replace(":", "%3A"); // suppress treatment as regex
String nameVar = "queryParam-" + entry.getKey().replace(":", "%3A"); // suppress treatment as regex
uriVars.put(nameVar, entry.getKey());
for (int j = 0; j < entry.getValue().size(); j++) {
String valueVar = nameVar + "[" + j + "]";

24
spring-web/src/test/java/org/springframework/web/service/invoker/HttpRequestValuesTests.java

@ -79,17 +79,19 @@ class HttpRequestValuesTests { @@ -79,17 +79,19 @@ class HttpRequestValuesTests {
assertThat(uriTemplate)
.isEqualTo("/path?" +
"{param1}={param1[0]}&" +
"{param2}={param2[0]}&" +
"{param2}={param2[1]}");
"{queryParam-param1}={queryParam-param1[0]}&" +
"{queryParam-param2}={queryParam-param2[0]}&" +
"{queryParam-param2}={queryParam-param2[1]}");
assertThat(requestValues.getUriVariables())
.containsOnlyKeys("param1", "param2", "param1[0]", "param2[0]", "param2[1]")
.containsEntry("param1", "param1")
.containsEntry("param2", "param2")
.containsEntry("param1[0]", "1st value")
.containsEntry("param2[0]", "2nd value A")
.containsEntry("param2[1]", "2nd value B");
.containsOnlyKeys(
"queryParam-param1", "queryParam-param2", "queryParam-param1[0]",
"queryParam-param2[0]", "queryParam-param2[1]")
.containsEntry("queryParam-param1", "param1")
.containsEntry("queryParam-param2", "param2")
.containsEntry("queryParam-param1[0]", "1st value")
.containsEntry("queryParam-param2[0]", "2nd value A")
.containsEntry("queryParam-param2[1]", "2nd value B");
URI uri = UriComponentsBuilder.fromUriString(uriTemplate)
.encode()
@ -107,7 +109,7 @@ class HttpRequestValuesTests { @@ -107,7 +109,7 @@ class HttpRequestValuesTests {
.build();
String uriTemplate = requestValues.getUriTemplate();
assertThat(uriTemplate).isEqualTo("/path?{userId%3Aeq}={userId%3Aeq[0]}");
assertThat(uriTemplate).isEqualTo("/path?{queryParam-userId%3Aeq}={queryParam-userId%3Aeq[0]}");
URI uri = UriComponentsBuilder.fromUriString(uriTemplate)
.encode()
@ -162,7 +164,7 @@ class HttpRequestValuesTests { @@ -162,7 +164,7 @@ class HttpRequestValuesTests {
String uriTemplate = requestValues.getUriTemplate();
assertThat(uriTemplate).isNotNull();
assertThat(uriTemplate).isEqualTo("/path?{query param}={query param[0]}");
assertThat(uriTemplate).isEqualTo("/path?{queryParam-query param}={queryParam-query param[0]}");
URI uri = UriComponentsBuilder.fromUriString(uriTemplate)
.encode()

22
spring-web/src/test/java/org/springframework/web/service/invoker/PathVariableArgumentResolverTests.java

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2025 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.
@ -16,11 +16,15 @@ @@ -16,11 +16,15 @@
package org.springframework.web.service.invoker;
import java.net.URI;
import org.jspecify.annotations.Nullable;
import org.junit.jupiter.api.Test;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.service.annotation.GetExchange;
import org.springframework.web.util.DefaultUriBuilderFactory;
import static org.assertj.core.api.Assertions.assertThat;
@ -46,6 +50,17 @@ class PathVariableArgumentResolverTests { @@ -46,6 +50,17 @@ class PathVariableArgumentResolverTests {
assertPathVariable("id", "test");
}
@Test // gh-34499
void pathVariableAndRequestParamWithSameName() {
this.service.executeWithPathVarAndRequestParam("{transfer-id}", "aValue");
assertPathVariable("transfer-id", "{transfer-id}");
HttpRequestValues values = this.client.getRequestValues();
URI uri = (new DefaultUriBuilderFactory()).expand(values.getUriTemplate(), values.getUriVariables());
assertThat(uri.toString()).isEqualTo("/transfers/%7Btransfer-id%7D?transfer-id=aValue");
}
@SuppressWarnings("SameParameterValue")
private void assertPathVariable(String name, @Nullable String expectedValue) {
assertThat(this.client.getRequestValues().getUriVariables().get(name)).isEqualTo(expectedValue);
@ -57,6 +72,11 @@ class PathVariableArgumentResolverTests { @@ -57,6 +72,11 @@ class PathVariableArgumentResolverTests {
@GetExchange
void execute(@PathVariable String id);
@GetExchange("/transfers/{transfer-id}")
void executeWithPathVarAndRequestParam(
@PathVariable("transfer-id") String transferId,
@RequestParam("transfer-id") String transferIdParam);
}
}

92
spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitter.java

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 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.
@ -21,7 +21,7 @@ import java.util.ArrayList; @@ -21,7 +21,7 @@ import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
import org.jspecify.annotations.Nullable;
@ -72,20 +72,19 @@ public class ResponseBodyEmitter { @@ -72,20 +72,19 @@ public class ResponseBodyEmitter {
private @Nullable Handler handler;
private final AtomicReference<State> state = new AtomicReference<>(State.START);
/** Store send data before handler is initialized. */
private final Set<DataWithMediaType> earlySendAttempts = new LinkedHashSet<>(8);
/** Store successful completion before the handler is initialized. */
private final AtomicBoolean complete = new AtomicBoolean();
/** Store an error before the handler is initialized. */
private @Nullable Throwable failure;
private final DefaultCallback timeoutCallback = new DefaultCallback();
private final TimeoutCallback timeoutCallback = new TimeoutCallback();
private final ErrorCallback errorCallback = new ErrorCallback();
private final DefaultCallback completionCallback = new DefaultCallback();
private final CompletionCallback completionCallback = new CompletionCallback();
/**
@ -125,7 +124,7 @@ public class ResponseBodyEmitter { @@ -125,7 +124,7 @@ public class ResponseBodyEmitter {
this.earlySendAttempts.clear();
}
if (this.complete.get()) {
if (this.state.get() == State.COMPLETE) {
if (this.failure != null) {
this.handler.completeWithError(this.failure);
}
@ -141,7 +140,7 @@ public class ResponseBodyEmitter { @@ -141,7 +140,7 @@ public class ResponseBodyEmitter {
}
void initializeWithError(Throwable ex) {
if (this.complete.compareAndSet(false, true)) {
if (this.state.compareAndSet(State.START, State.COMPLETE)) {
this.failure = ex;
this.earlySendAttempts.clear();
this.errorCallback.accept(ex);
@ -183,8 +182,7 @@ public class ResponseBodyEmitter { @@ -183,8 +182,7 @@ public class ResponseBodyEmitter {
* @throws java.lang.IllegalStateException wraps any other errors
*/
public synchronized void send(Object object, @Nullable MediaType mediaType) throws IOException {
Assert.state(!this.complete.get(), () -> "ResponseBodyEmitter has already completed" +
(this.failure != null ? " with error: " + this.failure : ""));
assertNotComplete();
if (this.handler != null) {
try {
this.handler.send(object, mediaType);
@ -211,11 +209,15 @@ public class ResponseBodyEmitter { @@ -211,11 +209,15 @@ public class ResponseBodyEmitter {
* @since 6.0.12
*/
public synchronized void send(Set<DataWithMediaType> items) throws IOException {
Assert.state(!this.complete.get(), () -> "ResponseBodyEmitter has already completed" +
(this.failure != null ? " with error: " + this.failure : ""));
assertNotComplete();
sendInternal(items);
}
private void assertNotComplete() {
Assert.state(this.state.get() == State.START, () -> "ResponseBodyEmitter has already completed" +
(this.failure != null ? " with error: " + this.failure : ""));
}
private void sendInternal(Set<DataWithMediaType> items) throws IOException {
if (items.isEmpty()) {
return;
@ -245,7 +247,7 @@ public class ResponseBodyEmitter { @@ -245,7 +247,7 @@ public class ResponseBodyEmitter {
* related events such as an error while {@link #send(Object) sending}.
*/
public void complete() {
if (this.complete.compareAndSet(false, true) && this.handler != null) {
if (trySetComplete() && this.handler != null) {
this.handler.complete();
}
}
@ -262,7 +264,7 @@ public class ResponseBodyEmitter { @@ -262,7 +264,7 @@ public class ResponseBodyEmitter {
* {@link #send(Object) sending}.
*/
public void completeWithError(Throwable ex) {
if (this.complete.compareAndSet(false, true)) {
if (trySetComplete()) {
this.failure = ex;
if (this.handler != null) {
this.handler.completeWithError(ex);
@ -270,6 +272,11 @@ public class ResponseBodyEmitter { @@ -270,6 +272,11 @@ public class ResponseBodyEmitter {
}
}
private boolean trySetComplete() {
return (this.state.compareAndSet(State.START, State.COMPLETE) ||
(this.state.compareAndSet(State.TIMEOUT, State.COMPLETE)));
}
/**
* Register code to invoke when the async request times out. This method is
* called from a container thread when an async request times out.
@ -364,7 +371,7 @@ public class ResponseBodyEmitter { @@ -364,7 +371,7 @@ public class ResponseBodyEmitter {
}
private class DefaultCallback implements Runnable {
private class TimeoutCallback implements Runnable {
private final List<Runnable> delegates = new ArrayList<>(1);
@ -374,9 +381,10 @@ public class ResponseBodyEmitter { @@ -374,9 +381,10 @@ public class ResponseBodyEmitter {
@Override
public void run() {
ResponseBodyEmitter.this.complete.compareAndSet(false, true);
for (Runnable delegate : this.delegates) {
delegate.run();
if (ResponseBodyEmitter.this.state.compareAndSet(State.START, State.TIMEOUT)) {
for (Runnable delegate : this.delegates) {
delegate.run();
}
}
}
}
@ -392,11 +400,51 @@ public class ResponseBodyEmitter { @@ -392,11 +400,51 @@ public class ResponseBodyEmitter {
@Override
public void accept(Throwable t) {
ResponseBodyEmitter.this.complete.compareAndSet(false, true);
for(Consumer<Throwable> delegate : this.delegates) {
delegate.accept(t);
if (ResponseBodyEmitter.this.state.compareAndSet(State.START, State.COMPLETE)) {
for (Consumer<Throwable> delegate : this.delegates) {
delegate.accept(t);
}
}
}
}
private class CompletionCallback implements Runnable {
private final List<Runnable> delegates = new ArrayList<>(1);
public synchronized void addDelegate(Runnable delegate) {
this.delegates.add(delegate);
}
@Override
public void run() {
if (ResponseBodyEmitter.this.state.compareAndSet(State.START, State.COMPLETE)) {
for (Runnable delegate : this.delegates) {
delegate.run();
}
}
}
}
/**
* Represents a state for {@link ResponseBodyEmitter}.
* <p><pre>
* START ----+
* | |
* v |
* TIMEOUT |
* | |
* v |
* COMPLETE <--+
* </pre>
* @since 6.2.4
*/
private enum State {
START,
TIMEOUT, // handling a timeout
COMPLETE
}
}

Loading…
Cancel
Save