diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index b44059e78..8de18274e 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -17,7 +17,7 @@ given the ability to merge pull requests. ## Code of Conduct This project adheres to the Contributor Covenant [code of -conduct](https://github.com/spring-cloud/spring-cloud-build/blob/main/docs/src/main/asciidoc/code-of-conduct.adoc). By participating, you are expected to uphold this code. Please report +conduct](https://github.com/spring-cloud/spring-cloud-build/blob/main/docs/modules/ROOT/partials/code-of-conduct.adoc). By participating, you are expected to uphold this code. Please report unacceptable behavior to spring-code-of-conduct@pivotal.io. ## Code Conventions and Housekeeping diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 5b829a0c7..3e82a383e 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -2,17 +2,12 @@ version: 2 updates: - package-ecosystem: "github-actions" directory: "/" - target-branch: "3.1.x" # oldest OSS supported branch + target-branch: "4.1.x" # oldest OSS supported branch schedule: interval: "weekly" - package-ecosystem: "github-actions" directory: "/" - target-branch: "4.0.x" # oldest OSS supported branch - schedule: - interval: "weekly" - - package-ecosystem: "github-actions" - directory: "/" - target-branch: "4.1.x" + target-branch: "4.2.x" schedule: interval: "weekly" - package-ecosystem: "github-actions" @@ -24,18 +19,7 @@ updates: directory: / schedule: interval: daily - target-branch: 3.1.x - ignore: - # only upgrade patch versions for maintenance branch - - dependency-name: "*" - update-types: - - version-update:semver-major - - version-update:semver-minor - - package-ecosystem: maven - directory: / - schedule: - interval: daily - target-branch: 4.0.x + target-branch: 4.1.x ignore: # only upgrade patch versions for maintenance branch - dependency-name: "*" @@ -46,7 +30,7 @@ updates: directory: / schedule: interval: daily - target-branch: 4.1.x + target-branch: 4.2.x ignore: # only upgrade patch versions for maintenance branch - dependency-name: "*" @@ -78,3 +62,8 @@ updates: directory: /docs schedule: interval: weekly + - package-ecosystem: npm + target-branch: 4.2.x + directory: /docs + schedule: + interval: weekly diff --git a/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc b/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc index 52ee890be..42b97dea8 100644 --- a/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc +++ b/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc @@ -146,7 +146,25 @@ When it comes to the Apache HttpClient 5-backed Feign clients, it's enough to en You can customize the HTTP client used by providing a bean of either `org.apache.hc.client5.http.impl.classic.CloseableHttpClient` when using Apache HC5. You can further customise http clients by setting values in the `spring.cloud.openfeign.httpclient.xxx` properties. The ones prefixed just with `httpclient` will work for all the clients, the ones prefixed with `httpclient.hc5` to Apache HttpClient 5, the ones prefixed with `httpclient.okhttp` to OkHttpClient and the ones prefixed with `httpclient.http2` to Http2Client. You can find a full list of properties you can customise in the appendix. -If you can not configure Apache HttpClient 5 by using properties, there is an `HttpClientBuilderCustomizer` interface for programmatic configuration. +If you can not configure Apache HttpClient 5 by using properties, there is an `HttpClient5FeignConfiguration.HttpClientBuilderCustomizer` interface for programmatic configuration. + +TIP: Apache HTTP Components `5.4` have changed defaults in the HttpClient relating to HTTP/1.1 TLS upgrades. Most proxy servers handle upgrades without issue, however, you may encounter issues with Envoy or Istio. If you need to restore previous behaviour, you can use `HttpClient5FeignConfiguration.HttpClientBuilderCustomizer` to do it, as shown in the example below. + +[source,java,indent=0] +---- +@Configuration +public class FooConfiguration { + + @Bean + public HttpClient5FeignConfiguration.HttpClientBuilderCustomizer httpClientBuilder() { + return (httpClientBuilder) -> { + RequestConfig.Builder requestConfigBuilder = RequestConfig.custom(); + requestConfigBuilder.setProtocolUpgradeEnabled(false); + httpClientBuilder.setDefaultRequestConfig(requestConfigBuilder.build()); + }; + } +} +---- TIP: Starting with Spring Cloud OpenFeign 4, the Feign Apache HttpClient 4 is no longer supported. We suggest using Apache HttpClient 5 instead. @@ -288,7 +306,12 @@ public class CustomConfiguration { } ---- -TIP: By default, Feign clients do not encode slash `/` characters. You can change this behaviour, by setting the value of `spring.cloud.openfeign.client.decodeSlash` to `false`. +TIP: By default, Feign clients do not encode slash `/` characters. You can change this behaviour, by setting the value of `spring.cloud.openfeign.client.decode-slash` to `false`. + + +TIP: By default, Feign clients do not remove trailing slash `/` characters from the request path. +You can change this behaviour, by setting the value of `spring.cloud.openfeign.client.remove-trailing-slash` to `true`. +Trailing slash removal from the request path is going to be made the default behaviour in the next major release. [[springencoder-configuration]] ==== `SpringEncoder` configuration @@ -737,7 +760,38 @@ The params attribute also supports the use of multiple `key=value` or only one ` - When `params = { "key1=v1", "key2=v2" }`, the request url is parsed as `/stores/storeId?key1=v1&key2=v2`. - When `params = "key"`, the request url is parsed as `/stores/storeId?key`. +[[negated-params-support]] +=== Negated Parameters Support + +Spring Cloud OpenFeign provides optional support for negated parameters in `@RequestMapping` annotations. By default, negated parameters (such as `params = {"!expiration"}` or `params = {"param!=value"}`) are not supported in Feign clients, resulting in an `IllegalArgumentException`. + +When sharing interfaces between server and client, negated parameters can be useful for disambiguation of request mappings on the server side. To allow the use of negated parameters in your Feign clients, set the following property: +[source,yaml] +---- +spring: + cloud: + openfeign: + client: + config: + feignName: + allowNegatedParams: true +---- + +When enabled, negated parameters will be silently ignored when creating the client request. This is the expected behavior since negated parameters are constraints used only on the server side to avoid ambiguous mappings. + +Global configuration is also supported: + +[source,yaml] +---- +spring: + cloud: + openfeign: + client: + config: + default: + allowNegatedParams: true +---- [[feign-querymap-support]] === Feign @QueryMap support diff --git a/docs/modules/ROOT/partials/_configprops.adoc b/docs/modules/ROOT/partials/_configprops.adoc index 81cc1df18..b8b6781be 100644 --- a/docs/modules/ROOT/partials/_configprops.adoc +++ b/docs/modules/ROOT/partials/_configprops.adoc @@ -1,7 +1,7 @@ |=== |Name | Default | Description -|spring.cloud.compatibility-verifier.compatible-boot-versions | | Default accepted versions for the Spring Boot dependency. You can set {@code x} for the patch version if you don't want to specify a concrete value. Example: {@code 3.4.x} +|spring.cloud.compatibility-verifier.compatible-boot-versions | `+++3.5.x+++` | Default accepted versions for the Spring Boot dependency. You can set {@code x} for the patch version if you don't want to specify a concrete value. Example: {@code 3.5.x} |spring.cloud.compatibility-verifier.enabled | `+++false+++` | Enables creation of Spring Cloud compatibility verification. |spring.cloud.config.allow-override | `+++true+++` | Flag to indicate that {@link #isOverrideSystemProperties() systemPropertiesOverride} can be used. Set to false to prevent users from changing the default accidentally. Default true. |spring.cloud.config.initialize-on-context-refresh | `+++false+++` | Flag to initialize bootstrap configuration on context refresh event. Default false. @@ -58,6 +58,7 @@ |spring.cloud.loadbalancer.retry.retry-on-all-operations | `+++false+++` | Indicates retries should be attempted on operations other than `HttpMethod.GET`. |spring.cloud.loadbalancer.retry.retryable-exceptions | `+++{}+++` | A `Set` of `Throwable` classes that should trigger a retry. |spring.cloud.loadbalancer.retry.retryable-status-codes | `+++{}+++` | A `Set` of status codes that should trigger a retry. +|spring.cloud.loadbalancer.stats.include-path | `+++true+++` | Indicates whether the {@code path} should be added to {@code uri} tag in metrics. When {@link RestTemplate} is used to execute load-balanced requests with high cardinality paths, setting it to {@code false} is recommended. |spring.cloud.loadbalancer.stats.micrometer.enabled | `+++false+++` | Enables micrometer metrics for load-balanced requests. |spring.cloud.loadbalancer.sticky-session.add-service-instance-cookie | `+++false+++` | Indicates whether a cookie with the newly selected instance should be added by LoadBalancer. |spring.cloud.loadbalancer.sticky-session.instance-id-cookie-name | `+++sc-lb-instance-id+++` | The name of the cookie holding the preferred instance id. @@ -73,6 +74,7 @@ |spring.cloud.openfeign.client.default-config | `+++default+++` | |spring.cloud.openfeign.client.default-to-properties | `+++true+++` | |spring.cloud.openfeign.client.refresh-enabled | `+++false+++` | Enables options value refresh capability for Feign. +|spring.cloud.openfeign.client.remove-trailing-slash | `+++false+++` | If {@code true}, trailing slashes at the end of request urls will be removed. |spring.cloud.openfeign.compression.request.content-encoding-types | | The list of content encodings (applicable encodings depend on the used client). |spring.cloud.openfeign.compression.request.enabled | `+++false+++` | Enables the request sent by Feign to be compressed. |spring.cloud.openfeign.compression.request.mime-types | `+++[text/xml, application/xml, application/json]+++` | The list of supported mime types. @@ -85,19 +87,19 @@ |spring.cloud.openfeign.httpclient.disable-ssl-validation | `+++false+++` | |spring.cloud.openfeign.httpclient.follow-redirects | `+++true+++` | |spring.cloud.openfeign.httpclient.hc5.connection-request-timeout | `+++3+++` | Default value for connection request timeout. -|spring.cloud.openfeign.httpclient.hc5.connection-request-timeout-unit | | Default value for connection request timeout unit. +|spring.cloud.openfeign.httpclient.hc5.connection-request-timeout-unit | `+++minutes+++` | Default value for connection request timeout unit. |spring.cloud.openfeign.httpclient.hc5.enabled | `+++true+++` | Enables the use of the Apache HTTP Client 5 by Feign. -|spring.cloud.openfeign.httpclient.hc5.pool-concurrency-policy | | Pool concurrency policies. -|spring.cloud.openfeign.httpclient.hc5.pool-reuse-policy | | Pool connection re-use policies. +|spring.cloud.openfeign.httpclient.hc5.pool-concurrency-policy | `+++strict+++` | Pool concurrency policies. +|spring.cloud.openfeign.httpclient.hc5.pool-reuse-policy | `+++fifo+++` | Pool connection re-use policies. |spring.cloud.openfeign.httpclient.hc5.socket-timeout | `+++5+++` | Default value for socket timeout. -|spring.cloud.openfeign.httpclient.hc5.socket-timeout-unit | | Default value for socket timeout unit. +|spring.cloud.openfeign.httpclient.hc5.socket-timeout-unit | `+++seconds+++` | Default value for socket timeout unit. |spring.cloud.openfeign.httpclient.http2.version | `+++HTTP_2+++` | Configure the protocols used by this client to communicate with remote servers. Uses {@link String} value of {@link HttpClient.Version}. |spring.cloud.openfeign.httpclient.max-connections | `+++200+++` | |spring.cloud.openfeign.httpclient.max-connections-per-route | `+++50+++` | |spring.cloud.openfeign.httpclient.ok-http.protocols | | Configure the protocols used by this client to communicate with remote servers. Uses {@link String} values of {@link Protocol}. |spring.cloud.openfeign.httpclient.ok-http.read-timeout | `+++60s+++` | {@link OkHttpClient} read timeout; defaults to 60 seconds. |spring.cloud.openfeign.httpclient.time-to-live | `+++900+++` | -|spring.cloud.openfeign.httpclient.time-to-live-unit | | +|spring.cloud.openfeign.httpclient.time-to-live-unit | `+++seconds+++` | |spring.cloud.openfeign.lazy-attributes-resolution | `+++false+++` | Switches @FeignClient attributes resolution mode to lazy. |spring.cloud.openfeign.micrometer.enabled | `+++true+++` | Enables Micrometer capabilities for Feign. |spring.cloud.openfeign.oauth2.clientRegistrationId | | Provides a clientId to be used with OAuth2. @@ -105,8 +107,8 @@ |spring.cloud.openfeign.okhttp.enabled | `+++false+++` | Enables the use of the OK HTTP Client by Feign. |spring.cloud.refresh.additional-property-sources-to-retain | | Additional property sources to retain during a refresh. Typically only system property sources are retained. This property allows property sources, such as property sources created by EnvironmentPostProcessors to be retained as well. |spring.cloud.refresh.enabled | `+++true+++` | Enables autoconfiguration for the refresh scope and associated features. -|spring.cloud.refresh.extra-refreshable | `+++true+++` | Additional class names for beans to post process into refresh scope. -|spring.cloud.refresh.never-refreshable | `+++true+++` | Comma separated list of class names for beans to never be refreshed or rebound. +|spring.cloud.refresh.extra-refreshable | `+++true+++` | Additional bean names or class names for beans to post process into refresh scope. +|spring.cloud.refresh.never-refreshable | `+++true+++` | Comma separated list of bean names or class names for beans to never be refreshed or rebound. |spring.cloud.refresh.on-restart.enabled | `+++true+++` | Enable refreshing context on start. |spring.cloud.service-registry.auto-registration.enabled | `+++true+++` | Whether service auto-registration is enabled. Defaults to true. |spring.cloud.service-registry.auto-registration.fail-fast | `+++false+++` | Whether startup fails if there is no AutoServiceRegistration. Defaults to false. diff --git a/docs/pom.xml b/docs/pom.xml index f3068e824..6b9d3c1be 100644 --- a/docs/pom.xml +++ b/docs/pom.xml @@ -8,7 +8,7 @@ org.springframework.cloud spring-cloud-openfeign - 4.1.5-SNAPSHOT + 4.3.0-SNAPSHOT .. diff --git a/pom.xml b/pom.xml index df793dfd5..c8d2c046c 100644 --- a/pom.xml +++ b/pom.xml @@ -4,14 +4,14 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> 4.0.0 spring-cloud-openfeign - 4.1.5-SNAPSHOT + 4.3.0-SNAPSHOT pom Spring Cloud OpenFeign Spring Cloud OpenFeign org.springframework.cloud spring-cloud-build - 4.1.6-SNAPSHOT + 4.3.0-SNAPSHOT @@ -25,8 +25,8 @@ ${basedir} - 2.17.3 - 4.1.6-SNAPSHOT + 2.18.2 + 4.3.0-SNAPSHOT 2.10 @@ -78,6 +78,7 @@ mozilla/public-suffix-list.txt + mockito-extensions/org.mockito.plugins.MockResolver diff --git a/spring-cloud-openfeign-core/pom.xml b/spring-cloud-openfeign-core/pom.xml index cd726f90d..e30886c63 100644 --- a/spring-cloud-openfeign-core/pom.xml +++ b/spring-cloud-openfeign-core/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-openfeign - 4.1.5-SNAPSHOT + 4.3.0-SNAPSHOT .. @@ -191,7 +191,7 @@ commons-io commons-io - 2.16.1 + 2.18.0 test diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientProperties.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientProperties.java index bec1b653a..29dcc6d04 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientProperties.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientProperties.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2023 the original author or authors. + * Copyright 2013-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. @@ -45,6 +45,7 @@ * @author Hyeonmin Park * @author Jasbir Singh * @author Dominique Villard + * @author kssumin */ @ConfigurationProperties("spring.cloud.openfeign.client") public class FeignClientProperties { @@ -61,6 +62,16 @@ public class FeignClientProperties { */ private boolean decodeSlash = true; + /** + * If {@code true}, trailing slashes at the end of request urls will be removed. + */ + private boolean removeTrailingSlash; + + /** + * If {@code true}, negated parameters (those starting with '!') will be allowed. + */ + private boolean allowNegatedParams = false; + public boolean isDefaultToProperties() { return defaultToProperties; } @@ -93,6 +104,22 @@ public void setDecodeSlash(boolean decodeSlash) { this.decodeSlash = decodeSlash; } + public boolean isRemoveTrailingSlash() { + return removeTrailingSlash; + } + + public void setRemoveTrailingSlash(boolean removeTrailingSlash) { + this.removeTrailingSlash = removeTrailingSlash; + } + + public void setAllowNegatedParams(Boolean allowNegatedParams) { + this.allowNegatedParams = allowNegatedParams; + } + + public boolean getAllowNegatedParams() { + return allowNegatedParams; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -103,12 +130,13 @@ public boolean equals(Object o) { } FeignClientProperties that = (FeignClientProperties) o; return defaultToProperties == that.defaultToProperties && Objects.equals(defaultConfig, that.defaultConfig) - && Objects.equals(config, that.config) && Objects.equals(decodeSlash, that.decodeSlash); + && Objects.equals(config, that.config) && Objects.equals(decodeSlash, that.decodeSlash) + && Objects.equals(removeTrailingSlash, that.removeTrailingSlash); } @Override public int hashCode() { - return Objects.hash(defaultToProperties, defaultConfig, config, decodeSlash); + return Objects.hash(defaultToProperties, defaultConfig, config, decodeSlash, removeTrailingSlash); } /** diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java index 3372a0c55..23dfcdfa0 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-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. @@ -145,8 +145,7 @@ public QueryMapEncoder feignQueryMapEncoderPageable() { @Bean @ConditionalOnMissingBean public Contract feignContract(ConversionService feignConversionService) { - boolean decodeSlash = feignClientProperties == null || feignClientProperties.isDecodeSlash(); - return new SpringMvcContract(parameterProcessors, feignConversionService, decodeSlash); + return new SpringMvcContract(parameterProcessors, feignConversionService, feignClientProperties); } @Bean diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsRegistrar.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsRegistrar.java index 262c2c5e7..00fe8f195 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsRegistrar.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsRegistrar.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-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. @@ -67,6 +67,7 @@ * @author Marcin Grzejszczak * @author Olga Maciaszek-Sharma * @author Jasbir Singh + * @author Jinho Lee */ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, ResourceLoaderAware, EnvironmentAware { @@ -117,6 +118,9 @@ static String getUrl(String url) { if (!url.contains("://")) { url = "http://" + url; } + if (url.endsWith("/")) { + url = url.substring(0, url.length() - 1); + } try { new URL(url); } @@ -321,11 +325,7 @@ private void validate(Map attributes) { validateFallbackFactory(annotation.getClass("fallbackFactory")); } - /* for testing */ String getName(Map attributes) { - return getName(null, attributes); - } - - String getName(ConfigurableBeanFactory beanFactory, Map attributes) { + String getName(Map attributes) { String name = (String) attributes.get("serviceId"); if (!StringUtils.hasText(name)) { name = (String) attributes.get("name"); @@ -333,7 +333,7 @@ String getName(ConfigurableBeanFactory beanFactory, Map attribut if (!StringUtils.hasText(name)) { name = (String) attributes.get("value"); } - name = resolve(beanFactory, name); + name = resolve(null, name); return getName(name); } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClient.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClient.java index 7744bf5bd..0728d90e0 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClient.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-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. @@ -71,7 +71,7 @@ public class FeignBlockingLoadBalancerClient implements Client { * @deprecated in favour of * {@link FeignBlockingLoadBalancerClient#FeignBlockingLoadBalancerClient(Client, LoadBalancerClient, LoadBalancerClientFactory, List)} */ - @Deprecated + @Deprecated(forRemoval = true) public FeignBlockingLoadBalancerClient(Client delegate, LoadBalancerClient loadBalancerClient, LoadBalancerProperties properties, LoadBalancerClientFactory loadBalancerClientFactory) { this.delegate = delegate; @@ -84,7 +84,7 @@ public FeignBlockingLoadBalancerClient(Client delegate, LoadBalancerClient loadB * @deprecated in favour of * {@link FeignBlockingLoadBalancerClient#FeignBlockingLoadBalancerClient(Client, LoadBalancerClient, LoadBalancerClientFactory, List)} */ - @Deprecated + @Deprecated(forRemoval = true) public FeignBlockingLoadBalancerClient(Client delegate, LoadBalancerClient loadBalancerClient, LoadBalancerClientFactory loadBalancerClientFactory) { this.delegate = delegate; diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java index 77414bdf4..0b17f7e46 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java @@ -90,7 +90,7 @@ public class RetryableFeignBlockingLoadBalancerClient implements Client { * @deprecated in favour of * {@link RetryableFeignBlockingLoadBalancerClient#RetryableFeignBlockingLoadBalancerClient(Client, LoadBalancerClient, LoadBalancedRetryFactory, LoadBalancerClientFactory, List)} */ - @Deprecated + @Deprecated(forRemoval = true) public RetryableFeignBlockingLoadBalancerClient(Client delegate, LoadBalancerClient loadBalancerClient, LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerProperties properties, LoadBalancerClientFactory loadBalancerClientFactory) { @@ -105,7 +105,7 @@ public RetryableFeignBlockingLoadBalancerClient(Client delegate, LoadBalancerCli * @deprecated in favour of * {@link RetryableFeignBlockingLoadBalancerClient#RetryableFeignBlockingLoadBalancerClient(Client, LoadBalancerClient, LoadBalancedRetryFactory, LoadBalancerClientFactory, List)} */ - @Deprecated + @Deprecated(forRemoval = true) public RetryableFeignBlockingLoadBalancerClient(Client delegate, LoadBalancerClient loadBalancerClient, LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerClientFactory loadBalancerClientFactory) { this.delegate = delegate; @@ -257,6 +257,12 @@ public URI getURI() { return URI.create(request.url()); } + @Override + public Map getAttributes() { + Map attributes = new HashMap<>(request.requestTemplate().queries()); + return attributes; + } + @Override public HttpHeaders getHeaders() { Map> headers = new HashMap<>(); diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/PageableSpringEncoder.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/PageableSpringEncoder.java index 49f69ec69..dce848824 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/PageableSpringEncoder.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/PageableSpringEncoder.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-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. @@ -33,6 +33,7 @@ * * @author Pascal Büttiker * @author Yanming Zhou + * @author Gokalp Kuscu */ public class PageableSpringEncoder implements Encoder { @@ -53,6 +54,11 @@ public class PageableSpringEncoder implements Encoder { */ private String sortParameter = "sort"; + /** + * Sort ignoreCase parameter name. + */ + private final String ignoreCase = "ignorecase"; + /** * Creates a new PageableSpringEncoder with the given delegate for fallback. If no * delegate is provided and this encoder cant handle the request, an EncodeException @@ -115,7 +121,11 @@ private void applySort(RequestTemplate template, Sort sort) { } } for (Sort.Order order : sort) { - sortQueries.add(order.getProperty() + "%2C" + order.getDirection()); + String sortQuery = order.getProperty() + "%2C" + order.getDirection(); + if (order.isIgnoreCase()) { + sortQuery += "%2C" + ignoreCase; + } + sortQueries.add(sortQuery); } if (!sortQueries.isEmpty()) { template.query(sortParameter, sortQueries); diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/PageableSpringQueryMapEncoder.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/PageableSpringQueryMapEncoder.java index a7c853a12..d44d23499 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/PageableSpringQueryMapEncoder.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/PageableSpringQueryMapEncoder.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-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. @@ -32,6 +32,7 @@ * * @author Hyeonmin Park * @author Yanming Zhou + * @author Gokalp Kuscu * @since 2.2.8 */ public class PageableSpringQueryMapEncoder extends BeanQueryMapEncoder { @@ -51,6 +52,11 @@ public class PageableSpringQueryMapEncoder extends BeanQueryMapEncoder { */ private String sortParameter = "sort"; + /** + * Sort ignoreCase parameter name. + */ + private final String ignoreCase = "ignorecase"; + public void setPageParameter(String pageParameter) { this.pageParameter = pageParameter; } @@ -92,7 +98,11 @@ else if (object instanceof Sort sort) { private void applySort(Map queryMap, Sort sort) { List sortQueries = new ArrayList<>(); for (Sort.Order order : sort) { - sortQueries.add(order.getProperty() + "%2C" + order.getDirection()); + String sortQuery = order.getProperty() + "%2C" + order.getDirection(); + if (order.isIgnoreCase()) { + sortQuery += "%2C" + ignoreCase; + } + sortQueries.add(sortQuery); } if (!sortQueries.isEmpty()) { queryMap.put(sortParameter, sortQueries); diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SortJsonComponent.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SortJsonComponent.java index cf22a0dbf..7c454be3c 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SortJsonComponent.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SortJsonComponent.java @@ -39,6 +39,7 @@ * * @author Can Bezmen * @author Olga Maciaszek-Sharma + * @author Gokalp Kuscu */ public class SortJsonComponent { @@ -90,8 +91,23 @@ public Class handledType() { private static Sort toSort(ArrayNode arrayNode) { List orders = new ArrayList<>(); for (JsonNode jsonNode : arrayNode) { - Sort.Order order = new Sort.Order(Sort.Direction.valueOf(jsonNode.get("direction").textValue()), - jsonNode.get("property").textValue()); + Sort.Order order; + // there is no way to construct without null handling + if ((jsonNode.has("ignoreCase") && jsonNode.get("ignoreCase").isBoolean()) + && jsonNode.has("nullHandling") && jsonNode.get("nullHandling").isTextual()) { + + boolean ignoreCase = jsonNode.get("ignoreCase").asBoolean(); + String nullHandlingValue = jsonNode.get("nullHandling").textValue(); + + order = new Sort.Order(Sort.Direction.valueOf(jsonNode.get("direction").textValue()), + jsonNode.get("property").textValue(), ignoreCase, + Sort.NullHandling.valueOf(nullHandlingValue)); + } + else { + // backward compatibility + order = new Sort.Order(Sort.Direction.valueOf(jsonNode.get("direction").textValue()), + jsonNode.get("property").textValue()); + } orders.add(order); } return Sort.by(orders); diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringDecoder.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringDecoder.java index b1219ec01..1067f2ab6 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringDecoder.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringDecoder.java @@ -87,7 +87,13 @@ public HttpStatusCode getStatusCode() { return HttpStatusCode.valueOf(response.status()); } - @Override + /** + * This method used to override a method from ClientHttpResponse interface but was + * removed in Spring Framework 6.2, so we should remove it as well. + * @deprecated in favour of + * {@link SpringDecoder.FeignResponseAdapter#getStatusCode()} + */ + @Deprecated(forRemoval = true) public int getRawStatusCode() { return response.status(); } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java index 9d9d7a8c1..fe589f0bc 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java @@ -41,6 +41,7 @@ import org.springframework.cloud.openfeign.AnnotatedParameterProcessor; import org.springframework.cloud.openfeign.CollectionFormat; +import org.springframework.cloud.openfeign.FeignClientProperties; import org.springframework.cloud.openfeign.SpringQueryMap; import org.springframework.cloud.openfeign.annotation.CookieValueParameterProcessor; import org.springframework.cloud.openfeign.annotation.MatrixVariableParameterProcessor; @@ -88,6 +89,7 @@ * @author Ram Anaswara * @author Sam Kruglov * @author Tang Xiong + * @author kssumin */ public class SpringMvcContract extends Contract.BaseContract implements ResourceLoaderAware { @@ -115,6 +117,10 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource private final boolean decodeSlash; + private final boolean removeTrailingSlash; + + private final boolean allowNegatedParams; + public SpringMvcContract() { this(Collections.emptyList()); } @@ -128,8 +134,36 @@ public SpringMvcContract(List annotatedParameterPro this(annotatedParameterProcessors, conversionService, true); } + /** + * Creates a {@link SpringMvcContract} based on annotatedParameterProcessors, + * conversionService and decodeSlash value. + * @param annotatedParameterProcessors list of {@link AnnotatedParameterProcessor} + * objects used to resolve parameters + * @param conversionService {@link ConversionService} used for type conversion + * @param decodeSlash indicates whether slashes should be decoded + * @deprecated in favour of + * {@link SpringMvcContract#SpringMvcContract(List, ConversionService, FeignClientProperties)} + */ + @Deprecated(forRemoval = true) public SpringMvcContract(List annotatedParameterProcessors, ConversionService conversionService, boolean decodeSlash) { + this(annotatedParameterProcessors, conversionService, decodeSlash, false); + } + + /** + * Creates a {@link SpringMvcContract} based on annotatedParameterProcessors, + * conversionService and decodeSlash value. + * @param annotatedParameterProcessors list of {@link AnnotatedParameterProcessor} + * objects used to resolve parameters + * @param conversionService {@link ConversionService} used for type conversion + * @param decodeSlash indicates whether slashes should be decoded + * @param removeTrailingSlash indicates whether trailing slashes should be removed + * @deprecated in favour of + * {@link SpringMvcContract#SpringMvcContract(List, ConversionService, FeignClientProperties)} + */ + @Deprecated(forRemoval = true) + public SpringMvcContract(List annotatedParameterProcessors, + ConversionService conversionService, boolean decodeSlash, boolean removeTrailingSlash) { Assert.notNull(annotatedParameterProcessors, "Parameter processors can not be null."); Assert.notNull(conversionService, "ConversionService can not be null."); @@ -140,6 +174,33 @@ public SpringMvcContract(List annotatedParameterPro this.conversionService = conversionService; convertingExpanderFactory = new ConvertingExpanderFactory(conversionService); this.decodeSlash = decodeSlash; + this.removeTrailingSlash = removeTrailingSlash; + this.allowNegatedParams = false; + } + + @Deprecated(forRemoval = true) + public SpringMvcContract(List annotatedParameterProcessors, + ConversionService conversionService, boolean decodeSlash, boolean removeTrailingSlash, + boolean allowNegatedParams) { + Assert.notNull(annotatedParameterProcessors, "Parameter processors can not be null."); + Assert.notNull(conversionService, "ConversionService can not be null."); + + List processors = getDefaultAnnotatedArgumentsProcessors(); + processors.addAll(annotatedParameterProcessors); + + annotatedArgumentProcessors = toAnnotatedArgumentProcessorMap(processors); + this.conversionService = conversionService; + convertingExpanderFactory = new ConvertingExpanderFactory(conversionService); + this.decodeSlash = decodeSlash; + this.removeTrailingSlash = removeTrailingSlash; + this.allowNegatedParams = allowNegatedParams; + } + + public SpringMvcContract(List annotatedParameterProcessors, + ConversionService conversionService, FeignClientProperties feignClientProperties) { + this(annotatedParameterProcessors, conversionService, + feignClientProperties == null || feignClientProperties.isDecodeSlash(), + feignClientProperties != null && feignClientProperties.isRemoveTrailingSlash()); } private static TypeDescriptor createTypeDescriptor(Method method, int paramIndex) { @@ -229,6 +290,9 @@ protected void processAnnotationOnMethod(MethodMetadata data, Annotation methodA if (!pathValue.startsWith("/") && !data.template().path().endsWith("/")) { pathValue = "/" + pathValue; } + if (removeTrailingSlash && pathValue.endsWith("/")) { + pathValue = pathValue.substring(0, pathValue.length() - 1); + } data.template().uri(pathValue, true); if (data.template().decodeSlash() != decodeSlash) { data.template().decodeSlash(decodeSlash); @@ -369,7 +433,11 @@ private void parseParams(MethodMetadata data, Method method, RequestMapping meth data.template().query(resolve(nameValueResolver.getName()), resolve(nameValueResolver.getValue())); } else { - throw new IllegalArgumentException("Negated params are not supported: " + param); + // Negated parameter case + if (!this.allowNegatedParams) { + throw new IllegalArgumentException("Negated params are not supported: " + param); + } + // When allowed, negated params are skipped } } } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientBuilderTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientBuilderTests.java index 4f876c4da..67a9e85bd 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientBuilderTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientBuilderTests.java @@ -131,7 +131,7 @@ void forType_allFieldsSetOnBuilder() { assertFactoryBeanField(builder, "contextId", "TestContext"); // and: - assertFactoryBeanField(builder, "url", "http://Url/"); + assertFactoryBeanField(builder, "url", "http://Url"); assertFactoryBeanField(builder, "path", "/Path"); assertFactoryBeanField(builder, "dismiss404", true); @@ -155,7 +155,7 @@ void forType_clientFactoryBeanProvided() { assertFactoryBeanField(builder, "contextId", "TestContext"); // and: - assertFactoryBeanField(builder, "url", "http://Url/"); + assertFactoryBeanField(builder, "url", "http://Url"); assertFactoryBeanField(builder, "path", "/Path"); assertFactoryBeanField(builder, "dismiss404", true); List additionalCustomizers = getFactoryBeanField(builder, "additionalCustomizers"); diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarTests.java index 07f5a25b0..5459dd662 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientsRegistrarTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-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. @@ -89,6 +89,12 @@ private String testGetName(String name) { return registrar.getName(Collections.singletonMap("name", name)); } + @Test + void testRemoveTrailingSlashFromUrl() { + String url = FeignClientsRegistrar.getUrl("http://localhost/"); + assertThat(url).isEqualTo("http://localhost"); + } + @Test void testFallback() { assertThatExceptionOfType(IllegalArgumentException.class) diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/FeignPageableEncodingTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/FeignPageableEncodingTests.java index 30fe65990..54b4697d0 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/FeignPageableEncodingTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/FeignPageableEncodingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-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. @@ -53,6 +53,7 @@ * * @author Charlie Mordant. * @author Hyeonmin Park + * @author Gokalp Kuscu */ @SpringBootTest(classes = FeignPageableEncodingTests.Application.class, webEnvironment = RANDOM_PORT, value = { "spring.cloud.openfeign.compression.request.enabled=true" }) @@ -264,6 +265,110 @@ void testSortWithBody() { } } + @Test + void testPageableWithIgnoreCase() { + // given + Sort.Order anySorting = Sort.Order.asc("anySorting").ignoreCase(); + Pageable pageable = PageRequest.of(0, 10, Sort.by(anySorting)); + + // when + final ResponseEntity> response = this.invoiceClient.getInvoicesPaged(pageable); + + // then + assertThat(response).isNotNull(); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getBody()).isNotNull(); + assertThat(pageable.getPageSize()).isEqualTo(response.getBody().getSize()); + assertThat(response.getBody().getPageable().getSort()).hasSize(1); + Optional optionalOrder = response.getBody().getPageable().getSort().get().findFirst(); + assertThat(optionalOrder.isPresent()).isEqualTo(true); + if (optionalOrder.isPresent()) { + Sort.Order order = optionalOrder.get(); + assertThat(order.getDirection()).isEqualTo(Sort.Direction.ASC); + assertThat(order.getProperty()).isEqualTo("anySorting"); + assertThat(order.isIgnoreCase()).as("isIgnoreCase does not have expected value").isEqualTo(true); + assertThat(order.getNullHandling()).isEqualTo(Sort.NullHandling.NATIVE); + } + } + + @Test + void testSortWithIgnoreCaseAndBody() { + // given + Sort.Order anySorting = Sort.Order.desc("amount").ignoreCase(); + Sort sort = Sort.by(anySorting); + + // when + final ResponseEntity> response = this.invoiceClient.getInvoicesSortedWithBody(sort, + "InvoiceTitleFromBody"); + + // then + assertThat(response).isNotNull(); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getBody()).isNotNull(); + assertThat(sort).isEqualTo(response.getBody().getSort()); + + Optional optionalOrder = response.getBody().getPageable().getSort().get().findFirst(); + assertThat(optionalOrder.isPresent()).isEqualTo(true); + if (optionalOrder.isPresent()) { + Sort.Order order = optionalOrder.get(); + assertThat(order.isIgnoreCase()).as("isIgnoreCase does not have expected value").isEqualTo(true); + assertThat(order.getNullHandling()).isEqualTo(Sort.NullHandling.NATIVE); + } + + List invoiceList = response.getBody().getContent(); + assertThat(invoiceList).hasSizeGreaterThanOrEqualTo(1); + + Invoice firstInvoice = invoiceList.get(0); + assertThat(firstInvoice.getTitle()).startsWith("InvoiceTitleFromBody"); + + for (int ind = 0; ind < invoiceList.size() - 1; ind++) { + assertThat(invoiceList.get(ind).getAmount()).isGreaterThanOrEqualTo(invoiceList.get(ind + 1).getAmount()); + } + + } + + @Test + void testPageableMultipleSortPropertiesWithBodyAndIgnoreCase() { + // given + Sort.Order anySorting1 = Sort.Order.desc("anySorting1").ignoreCase(); + Sort.Order anySorting2 = Sort.Order.asc("anySorting2"); + Pageable pageable = PageRequest.of(0, 10, Sort.by(anySorting1, anySorting2)); + + // when + final ResponseEntity> response = this.invoiceClient.getInvoicesPagedWithBody(pageable, + "InvoiceTitleFromBody"); + + // then + assertThat(response).isNotNull(); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getBody()).isNotNull(); + assertThat(pageable.getPageSize()).isEqualTo(response.getBody().getSize()); + + List invoiceList = response.getBody().getContent(); + assertThat(invoiceList).hasSizeGreaterThanOrEqualTo(1); + + Invoice firstInvoice = invoiceList.get(0); + assertThat(firstInvoice.getTitle()).startsWith("InvoiceTitleFromBody"); + + Sort sort = response.getBody().getPageable().getSort(); + assertThat(sort).hasSize(2); + + List orderList = sort.toList(); + assertThat(orderList).hasSize(2); + + Sort.Order firstOrder = orderList.get(0); + assertThat(firstOrder.getDirection()).isEqualTo(Sort.Direction.DESC); + assertThat(firstOrder.getProperty()).isEqualTo("anySorting1"); + assertThat(firstOrder.isIgnoreCase()).as("isIgnoreCase does not have expected value").isEqualTo(true); + assertThat(firstOrder.getNullHandling()).isEqualTo(Sort.NullHandling.NATIVE); + + Sort.Order secondOrder = orderList.get(1); + assertThat(secondOrder.getDirection()).isEqualTo(Sort.Direction.ASC); + assertThat(secondOrder.getProperty()).isEqualTo("anySorting2"); + assertThat(secondOrder.isIgnoreCase()).as("isIgnoreCase does not have expected value").isEqualTo(false); + assertThat(secondOrder.getNullHandling()).isEqualTo(Sort.NullHandling.NATIVE); + } + @EnableFeignClients(clients = InvoiceClient.class) @LoadBalancerClient(name = "local", configuration = LocalClientConfiguration.class) @SpringBootApplication(scanBasePackages = "org.springframework.cloud.openfeign.encoding.app", diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SortJacksonModuleTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SortJacksonModuleTests.java index 303eec4ab..ae71d328d 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SortJacksonModuleTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SortJacksonModuleTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-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. @@ -39,6 +39,7 @@ /** * @author Can Bezmen + * @author Gokalp Kuscu */ @ExtendWith(MockitoExtension.class) class SortJacksonModuleTests { @@ -53,7 +54,7 @@ public void setup() { } @Test - public void deserializePage() throws JsonProcessingException { + public void testDeserializePage() throws JsonProcessingException { // Given String pageJson = "{\"content\":[\"A name\"],\"number\":1,\"size\":2,\"totalElements\":3,\"sort\":[{\"direction\":\"ASC\",\"property\":\"field\",\"ignoreCase\":false,\"nullHandling\":\"NATIVE\",\"descending\":false,\"ascending\":true}]}"; // When @@ -72,11 +73,85 @@ public void deserializePage() throws JsonProcessingException { Sort.Order order = optionalOrder.get(); assertThat(order, hasProperty("property", is("field"))); assertThat(order, hasProperty("direction", is(Sort.Direction.ASC))); + assertThat(order, hasProperty("ignoreCase", is(false))); + assertThat(order, hasProperty("nullHandling", is(Sort.NullHandling.NATIVE))); } } @Test - public void serializePage() throws IOException { + public void testDeserializePageWithoutIgnoreCaseAndNullHandling() throws JsonProcessingException { + // Given + String pageJson = "{\"content\":[\"A name\"],\"number\":1,\"size\":2,\"totalElements\":3,\"sort\":[{\"direction\":\"ASC\",\"property\":\"field\",\"descending\":false,\"ascending\":true}]}"; + // When + Page result = objectMapper.readValue(pageJson, Page.class); + // Then + assertThat(result, notNullValue()); + assertThat(result, hasProperty("totalElements", is(3L))); + assertThat(result.getContent(), hasSize(1)); + assertThat(result.getPageable(), notNullValue()); + assertThat(result.getPageable().getPageNumber(), is(1)); + assertThat(result.getPageable().getPageSize(), is(2)); + assertThat(result.getPageable().getSort(), notNullValue()); + result.getPageable().getSort(); + Optional optionalOrder = result.getPageable().getSort().get().findFirst(); + if (optionalOrder.isPresent()) { + Sort.Order order = optionalOrder.get(); + assertThat(order, hasProperty("property", is("field"))); + assertThat(order, hasProperty("direction", is(Sort.Direction.ASC))); + } + } + + @Test + public void testDeserializePageWithoutNullHandling() throws JsonProcessingException { + // Given + String pageJson = "{\"content\":[\"A name\"],\"number\":1,\"size\":2,\"totalElements\":3,\"sort\":[{\"direction\":\"ASC\",\"property\":\"field\",\"ignoreCase\":true,\"descending\":false,\"ascending\":true}]}"; + // When + Page result = objectMapper.readValue(pageJson, Page.class); + // Then + assertThat(result, notNullValue()); + assertThat(result, hasProperty("totalElements", is(3L))); + assertThat(result.getContent(), hasSize(1)); + assertThat(result.getPageable(), notNullValue()); + assertThat(result.getPageable().getPageNumber(), is(1)); + assertThat(result.getPageable().getPageSize(), is(2)); + assertThat(result.getPageable().getSort(), notNullValue()); + result.getPageable().getSort(); + Optional optionalOrder = result.getPageable().getSort().get().findFirst(); + if (optionalOrder.isPresent()) { + Sort.Order order = optionalOrder.get(); + assertThat(order, hasProperty("property", is("field"))); + assertThat(order, hasProperty("direction", is(Sort.Direction.ASC))); + assertThat(order, hasProperty("ignoreCase", is(false))); + } + } + + @Test + public void testDeserializePageWithTrueMarkedIgnoreCaseAndNullHandling() throws JsonProcessingException { + // Given + String pageJson = "{\"content\":[\"A name\"],\"number\":1,\"size\":2,\"totalElements\":3,\"sort\":[{\"direction\":\"ASC\",\"property\":\"field\",\"ignoreCase\":true,\"nullHandling\":\"NATIVE\",\"descending\":false,\"ascending\":true}]}"; + // When + Page result = objectMapper.readValue(pageJson, Page.class); + // Then + assertThat(result, notNullValue()); + assertThat(result, hasProperty("totalElements", is(3L))); + assertThat(result.getContent(), hasSize(1)); + assertThat(result.getPageable(), notNullValue()); + assertThat(result.getPageable().getPageNumber(), is(1)); + assertThat(result.getPageable().getPageSize(), is(2)); + assertThat(result.getPageable().getSort(), notNullValue()); + result.getPageable().getSort(); + Optional optionalOrder = result.getPageable().getSort().get().findFirst(); + if (optionalOrder.isPresent()) { + Sort.Order order = optionalOrder.get(); + assertThat(order, hasProperty("property", is("field"))); + assertThat(order, hasProperty("direction", is(Sort.Direction.ASC))); + assertThat(order, hasProperty("ignoreCase", is(true))); + assertThat(order, hasProperty("nullHandling", is(Sort.NullHandling.NATIVE))); + } + } + + @Test + public void testSerializePage() throws IOException { // Given Sort sort = Sort.by(Sort.Order.by("fieldName")); // When @@ -86,4 +161,16 @@ public void serializePage() throws IOException { assertThat(result, containsString("\"property\":\"fieldName\"")); } + @Test + public void testSerializePageWithGivenIgnoreCase() throws IOException { + // Given + Sort sort = Sort.by(Sort.Order.by("fieldName"), Sort.Order.by("fieldName2").ignoreCase()); + // When + String result = objectMapper.writeValueAsString(sort); + // Then + assertThat(result, containsString("\"direction\":\"ASC\"")); + assertThat(result, containsString("\"property\":\"fieldName\"")); + assertThat(result, containsString("\"property\":\"fieldName2\",\"ignoreCase\":true")); + } + } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java index 02219e2ce..cb316ae39 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java @@ -36,6 +36,7 @@ import org.junit.jupiter.api.Test; import org.springframework.cloud.openfeign.CollectionFormat; +import org.springframework.cloud.openfeign.FeignClientProperties; import org.springframework.cloud.openfeign.SpringQueryMap; import org.springframework.core.convert.ConversionService; import org.springframework.data.domain.Page; @@ -83,6 +84,7 @@ * @author Sam Kruglov * @author Bhavya Agrawal * @author Tang Xiong + * @author kssumin **/ class SpringMvcContractTests { @@ -189,8 +191,23 @@ void testProcessAnnotations_SimpleNoPath() throws Exception { } @Test - void testProcessAnnotations_SimplePathIsOnlyASlash() throws Exception { - Method method = TestTemplate_Simple.class.getDeclaredMethod("getSlashPath", String.class); + void testProcessAnnotations_SimplePathIsOnlyASlashWithParam() throws Exception { + Method method = TestTemplate_Simple.class.getDeclaredMethod("getSlashPathWithParam", String.class); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/?id=" + "{id}"); + assertThat(data.template().method()).isEqualTo("GET"); + assertThat(data.template().headers().get("Accept").iterator().next()) + .isEqualTo(MediaType.APPLICATION_JSON_VALUE); + } + + @Test + void testProcessAnnotations_SimplePathIsOnlyASlashWithParamWithTrailingSlashRemoval() throws Exception { + FeignClientProperties properties = new FeignClientProperties(); + properties.setRemoveTrailingSlash(true); + contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), properties); + Method method = TestTemplate_Simple.class.getDeclaredMethod("getSlashPathWithParam", String.class); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); assertThat(data.template().url()).isEqualTo("/?id=" + "{id}"); @@ -284,6 +301,48 @@ void testProcessAnnotations_SimplePostMapping() throws Exception { } + @Test + void testProcessAnnotations_SimplePathIsOnlyASlashWithTrailingSlashRemoval() throws Exception { + FeignClientProperties properties = new FeignClientProperties(); + properties.setRemoveTrailingSlash(true); + contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), properties); + Method method = TestTemplate_Simple.class.getDeclaredMethod("getSlashPath"); + + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/"); + assertThat(data.template().method()).isEqualTo("GET"); + assertThat(data.template().headers().get("Accept").iterator().next()) + .isEqualTo(MediaType.APPLICATION_JSON_VALUE); + } + + @Test + void testProcessAnnotations_SimplePathHasTrailingSlash() throws Exception { + Method method = TestTemplate_Simple.class.getDeclaredMethod("getTrailingSlash"); + + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/test1/test2/"); + assertThat(data.template().method()).isEqualTo("GET"); + assertThat(data.template().headers().get("Accept").iterator().next()) + .isEqualTo(MediaType.APPLICATION_JSON_VALUE); + } + + @Test + void testProcessAnnotations_SimplePathHasTrailingSlashWithTrailingSlashRemoval() throws Exception { + FeignClientProperties properties = new FeignClientProperties(); + properties.setRemoveTrailingSlash(true); + contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), properties); + Method method = TestTemplate_Simple.class.getDeclaredMethod("getTrailingSlash"); + + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/test1/test2"); + assertThat(data.template().method()).isEqualTo("GET"); + assertThat(data.template().headers().get("Accept").iterator().next()) + .isEqualTo(MediaType.APPLICATION_JSON_VALUE); + } + @Test void testProcessAnnotationsOnMethod_Advanced() throws Exception { Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest", String.class, String.class, @@ -711,6 +770,28 @@ void shouldSetPageableAsBodyWhenQueryMapParamPresent() { assertThat(data.get(1).bodyIndex()).isEqualTo(0); } + @Test + void testAllowNegatedParams() throws Exception { + contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), true, false, true); + + Method method = TestTemplate_ParseParams.class.getDeclaredMethod("notEqualParams"); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/test"); + assertThat(data.template().method()).isEqualTo("GET"); + } + + @Test + void testDisallowNegatedParams() throws Exception { + contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), true, false, false); + + Method method = TestTemplate_ParseParams.class.getDeclaredMethod("notEqualParams"); + + assertThatIllegalArgumentException().isThrownBy(() -> { + contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + }).withMessageContaining("Negated params are not supported"); + } + private ConversionService getConversionService() { FormattingConversionServiceFactoryBean conversionServiceFactoryBean = new FormattingConversionServiceFactoryBean(); conversionServiceFactoryBean.afterPropertiesSet(); @@ -738,7 +819,13 @@ public interface TestTemplate_Simple { TestObject postMappingTest(@RequestBody TestObject object); @GetMapping(value = "/", produces = MediaType.APPLICATION_JSON_VALUE) - ResponseEntity getSlashPath(@RequestParam("id") String id); + ResponseEntity getSlashPathWithParam(@RequestParam("id") String id); + + @GetMapping(value = "/", produces = MediaType.APPLICATION_JSON_VALUE) + ResponseEntity getSlashPath(); + + @GetMapping(value = "test1/test2/", produces = MediaType.APPLICATION_JSON_VALUE) + ResponseEntity getTrailingSlash(); @GetMapping(path = "test", produces = MediaType.APPLICATION_JSON_VALUE) ResponseEntity getTestNoLeadingSlash(@RequestParam("name") String name); diff --git a/spring-cloud-openfeign-dependencies/pom.xml b/spring-cloud-openfeign-dependencies/pom.xml index c10f36953..99e80bf80 100644 --- a/spring-cloud-openfeign-dependencies/pom.xml +++ b/spring-cloud-openfeign-dependencies/pom.xml @@ -6,11 +6,11 @@ spring-cloud-dependencies-parent org.springframework.cloud - 4.1.6-SNAPSHOT + 4.3.0-SNAPSHOT spring-cloud-openfeign-dependencies - 4.1.5-SNAPSHOT + 4.3.0-SNAPSHOT pom spring-cloud-openfeign-dependencies Spring Cloud OpenFeign Dependencies diff --git a/spring-cloud-starter-openfeign/pom.xml b/spring-cloud-starter-openfeign/pom.xml index abcbadaa0..b997aac42 100644 --- a/spring-cloud-starter-openfeign/pom.xml +++ b/spring-cloud-starter-openfeign/pom.xml @@ -5,7 +5,7 @@ org.springframework.cloud spring-cloud-openfeign - 4.1.5-SNAPSHOT + 4.3.0-SNAPSHOT ..