diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/ast/AstUtilsTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/ast/AstUtilsTests.java new file mode 100644 index 00000000000..174fdff9041 --- /dev/null +++ b/spring-expression/src/test/java/org/springframework/expression/spel/ast/AstUtilsTests.java @@ -0,0 +1,170 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://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.springframework.expression.spel.ast; + +import java.util.List; + +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; + +import org.springframework.expression.EvaluationContext; +import org.springframework.expression.PropertyAccessor; +import org.springframework.expression.TypedValue; +import org.springframework.lang.Nullable; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link AstUtils}. + * + * @author Sam Brannen + * @since 6.1.15 + */ +class AstUtilsTests { + + private final PropertyAccessor animal1Accessor = createAccessor("Animal1", Animal.class); + + private final PropertyAccessor animal2Accessor = createAccessor("Animal2", Animal.class); + + private final PropertyAccessor cat1Accessor = createAccessor("Cat1", Cat.class); + + private final PropertyAccessor cat2Accessor = createAccessor("Cat2", Cat.class); + + private final PropertyAccessor generic1Accessor = createAccessor("Generic1", null); + + private final PropertyAccessor generic2Accessor = createAccessor("Generic2", null); + + private final List accessors = List.of( + generic1Accessor, + cat1Accessor, + animal1Accessor, + animal2Accessor, + cat2Accessor, + generic2Accessor + ); + + + @Test + void emptyAccessorsList() { + List accessorsToTry = getPropertyAccessorsToTry(new Cat(), List.of()); + assertThat(accessorsToTry).isEmpty(); + } + + @Test + void noMatch() { + List accessorsToTry = getPropertyAccessorsToTry(new Dog(), List.of(cat1Accessor)); + assertThat(accessorsToTry).isEmpty(); + } + + @Test + void singleExactTypeMatch() { + List accessorsToTry = getPropertyAccessorsToTry(new Cat(), List.of(cat1Accessor)); + assertThat(accessorsToTry).containsExactly(cat1Accessor); + } + + @Test + void exactTypeMatches() { + List accessorsToTry = getPropertyAccessorsToTry(new Cat(), accessors); + // We would actually expect the following. + // assertThat(accessorsToTry).containsExactly( + // cat1Accessor, cat2Accessor, animal1Accessor, animal2Accessor, generic1Accessor, generic2Accessor); + // However, prior to Spring Framework 6.2, the supertype and generic accessors are not + // ordered properly. So we test that the exact matches come first and in the expected order. + assertThat(accessorsToTry) + .hasSize(accessors.size()) + .startsWith(cat1Accessor, cat2Accessor); + } + + @Disabled("PropertyAccessor ordering for supertype and generic matches is broken prior to Spring Framework 6.2") + @Test + void supertypeMatches() { + List accessorsToTry = getPropertyAccessorsToTry(new Dog(), accessors); + assertThat(accessorsToTry).containsExactly( + animal1Accessor, animal2Accessor, generic1Accessor, generic2Accessor); + } + + @Test + void genericMatches() { + List accessorsToTry = getPropertyAccessorsToTry("not an Animal", accessors); + assertThat(accessorsToTry).containsExactly(generic1Accessor, generic2Accessor); + } + + + private static PropertyAccessor createAccessor(String name, Class type) { + return new DemoAccessor(name, type); + } + + private static List getPropertyAccessorsToTry(Object target, List propertyAccessors) { + return AstUtils.getPropertyAccessorsToTry(target.getClass(), propertyAccessors); + } + + + private static class DemoAccessor implements PropertyAccessor { + + private final String name; + private final Class[] types; + + DemoAccessor(String name, Class type) { + this.name = name; + this.types = (type != null ? new Class[] {type} : null); + } + + @Override + @Nullable + public Class[] getSpecificTargetClasses() { + return this.types; + } + + @Override + public String toString() { + return this.name; + } + + @Override + public boolean canRead(EvaluationContext context, Object target, String name) { + return true; + } + + @Override + public TypedValue read(EvaluationContext context, Object target, String name) { + throw new UnsupportedOperationException("Auto-generated method stub"); + } + + @Override + public boolean canWrite(EvaluationContext context, Object target, String name) { + return false; + } + + @Override + public void write(EvaluationContext context, Object target, String name, Object newValue) { + /* no-op */ + } + } + + sealed interface Animal permits Bat, Cat, Dog { + } + + static final class Bat implements Animal { + } + + static final class Cat implements Animal { + } + + static final class Dog implements Animal { + } + +} diff --git a/spring-web/src/main/java/org/springframework/http/client/observation/DefaultClientRequestObservationConvention.java b/spring-web/src/main/java/org/springframework/http/client/observation/DefaultClientRequestObservationConvention.java index 713ef7c4caf..fef77d242b0 100644 --- a/spring-web/src/main/java/org/springframework/http/client/observation/DefaultClientRequestObservationConvention.java +++ b/spring-web/src/main/java/org/springframework/http/client/observation/DefaultClientRequestObservationConvention.java @@ -43,7 +43,7 @@ public class DefaultClientRequestObservationConvention implements ClientRequestO private static final String DEFAULT_NAME = "http.client.requests"; - private static final Pattern PATTERN_BEFORE_PATH = Pattern.compile("^https?://[^/]+/"); + private static final Pattern PATTERN_BEFORE_PATH = Pattern.compile("^https?://[^/]+"); private static final KeyValue URI_NONE = KeyValue.of(LowCardinalityKeyNames.URI, KeyValue.NONE_VALUE); diff --git a/spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientRequestObservationConventionTests.java b/spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientRequestObservationConventionTests.java index b237ef0d78a..dea5002df03 100644 --- a/spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientRequestObservationConventionTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientRequestObservationConventionTests.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. @@ -88,6 +88,16 @@ class DefaultClientRequestObservationConventionTests { assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("http.url", "https://example.org/resource/42")); } + @Test + void addsKeyValuesForRequestWithUriTemplateWithoutPath() { + ClientRequestObservationContext context = createContext( + new MockClientHttpRequest(HttpMethod.GET, "https://example.org"), response); + context.setUriTemplate("https://example.org"); + assertThat(this.observationConvention.getLowCardinalityKeyValues(context)) + .contains(KeyValue.of("exception", "none"), KeyValue.of("method", "GET"), KeyValue.of("uri", "/"), + KeyValue.of("status", "200"), KeyValue.of("client.name", "example.org"), KeyValue.of("outcome", "SUCCESS")); + assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("http.url", "https://example.org")); + } @Test void addsKeyValuesForRequestWithoutUriTemplate() { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConvention.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConvention.java index 674540ef113..62f5d1287db 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConvention.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConvention.java @@ -43,7 +43,7 @@ public class DefaultClientRequestObservationConvention implements ClientRequestO private static final String ROOT_PATH = "/"; - private static final Pattern PATTERN_BEFORE_PATH = Pattern.compile("^https?://[^/]+/"); + private static final Pattern PATTERN_BEFORE_PATH = Pattern.compile("^https?://[^/]+"); private static final KeyValue URI_NONE = KeyValue.of(LowCardinalityKeyNames.URI, KeyValue.NONE_VALUE); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.java index d1becc26e51..86f620ca78e 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.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. @@ -91,6 +91,19 @@ class DefaultClientRequestObservationConventionTests { .contains(KeyValue.of("http.url", "/resource/42")); } + @Test + void shouldAddKeyValuesForRequestWithUriTemplateNoPath() { + ClientRequest.Builder request = ClientRequest.create(HttpMethod.GET, URI.create("https://example.org")) + .attribute(WebClient.class.getName() + ".uriTemplate", "https://example.org"); + ClientRequestObservationContext context = createContext(request); + context.setUriTemplate("https://example.org"); + assertThat(this.observationConvention.getLowCardinalityKeyValues(context)) + .contains(KeyValue.of("exception", "none"), KeyValue.of("method", "GET"), KeyValue.of("uri", "/"), + KeyValue.of("status", "200"), KeyValue.of("client.name", "example.org"), KeyValue.of("outcome", "SUCCESS")); + assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(1) + .contains(KeyValue.of("http.url", "https://example.org")); + } + @Test void shouldAddKeyValuesForRequestWithoutUriTemplate() { ClientRequestObservationContext context = createContext(ClientRequest.create(HttpMethod.GET, URI.create("/resource/42")));