Skip to content

Commit

Permalink
63 delegation leads to unexpected result (#122)
Browse files Browse the repository at this point in the history
* fix test

* fix map argument handling

---------

Co-authored-by: Artem Labazin <artem@labazin.com>
  • Loading branch information
xxlabaza and Artem Labazin committed Jun 25, 2024
1 parent 1577990 commit 63ba1e9
Show file tree
Hide file tree
Showing 6 changed files with 250 additions and 4 deletions.
1 change: 1 addition & 0 deletions feign-form/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ limitations under the License.
<configuration>
<instructions>
<Export-Package>feign.form</Export-Package>
<Export-Package>feign.form.multipart</Export-Package>
</instructions>
</configuration>
</plugin>
Expand Down
4 changes: 2 additions & 2 deletions feign-form/src/main/java/feign/form/FormEncoder.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,13 @@ public FormEncoder (Encoder delegate) {
public void encode (Object object, Type bodyType, RequestTemplate template) throws EncodeException {
String contentTypeValue = getContentTypeValue(template.headers());
val contentType = ContentType.of(contentTypeValue);
if (!processors.containsKey(contentType)) {
if (processors.containsKey(contentType) == false) {
delegate.encode(object, bodyType, template);
return;
}

Map<String, Object> data;
if (MAP_STRING_WILDCARD.equals(bodyType)) {
if (object instanceof Map) {
data = (Map<String, Object>) object;
} else if (isUserPojo(bodyType)) {
data = toMap(object);
Expand Down
3 changes: 1 addition & 2 deletions feign-form/src/test/java/feign/form/Server.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
Expand Down Expand Up @@ -123,7 +122,7 @@ public ResponseEntity<String> json (@RequestBody Dto dto) {
return ResponseEntity.status(status).body("ok");
}

@GetMapping("/query_map")
@PostMapping("/query_map")
public ResponseEntity<Integer> queryMap (
@RequestParam("filter") List<String> filters
) {
Expand Down
76 changes: 76 additions & 0 deletions feign-form/src/test/java/feign/form/issues/Issue63Test.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Copyright 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
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package feign.form.issues;

import static org.assertj.core.api.Assertions.assertThat;

import java.util.HashMap;
import java.util.Map;

import io.undertow.server.HttpServerExchange;
import lombok.val;
import org.junit.jupiter.api.Test;

import feign.Feign;
import feign.Headers;
import feign.RequestLine;
import feign.form.FormEncoder;
import feign.form.utils.UndertowServer;
import feign.jackson.JacksonEncoder;

// https://github.com/OpenFeign/feign-form/issues/63
class Issue63Test {

@Test
void test () {
try (val server = UndertowServer.builder().callback(this::handleRequest).start()) {
val client = Feign.builder()
.encoder(new FormEncoder(new JacksonEncoder()))
.target(Client.class, server.getConnectUrl());

val data = new HashMap<String, String>();
data.put("from", "+987654321");
data.put("to", "+123456789");
data.put("body", "hello world");

assertThat(client.map(data))
.isEqualTo("ok");
}
}

private void handleRequest (HttpServerExchange exchange, byte[] message) {
// assert request
assertThat(exchange.getRequestHeaders().getFirst(io.undertow.util.Headers.CONTENT_TYPE))
.isEqualTo("application/x-www-form-urlencoded; charset=UTF-8");
assertThat(message)
.asString()
.isEqualTo("from=%2B987654321&to=%2B123456789&body=hello+world");

// build response
exchange.getResponseHeaders()
.put(io.undertow.util.Headers.CONTENT_TYPE, "text/plain");
exchange.getResponseSender()
.send("ok");
}

interface Client {

@RequestLine("POST")
@Headers("Content-Type: application/x-www-form-urlencoded; charset=utf-8")
String map (Map<String, String> data);
}
}
101 changes: 101 additions & 0 deletions feign-form/src/test/java/feign/form/utils/UndertowServer.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* Copyright 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
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package feign.form.utils;

import java.net.InetSocketAddress;

import io.appulse.utils.SocketUtils;
import io.undertow.Undertow;
import io.undertow.io.Receiver.FullBytesCallback;
import io.undertow.server.HttpHandler;
import io.undertow.server.HttpServerExchange;
import io.undertow.server.handlers.BlockingHandler;
import io.undertow.util.Headers;
import lombok.Builder;
import lombok.RequiredArgsConstructor;
import lombok.val;

public final class UndertowServer implements AutoCloseable {

private final Undertow undertow;

@Builder(buildMethodName = "start")
private UndertowServer (FullBytesCallback callback) {
val port = SocketUtils.findFreePort()
.orElseThrow(() -> new IllegalStateException("no available port to start server"));

undertow = Undertow.builder()
.addHttpListener(port, "localhost")
.setHandler(
new BlockingHandler(
new ReadAllBytesHandler(callback)
)
)
.build();

undertow.start();
}

/**
* Returns server connect URL.
*
* @return listining server's url.
*/
public String getConnectUrl () {
val listenerInfo = undertow.getListenerInfo()
.iterator()
.next();

val address = (InetSocketAddress) listenerInfo.getAddress();

return String.format(
"%s://%s:%d",
listenerInfo.getProtcol(),
address.getHostString(),
address.getPort()
);
}

@Override
public void close () {
undertow.stop();
}

@RequiredArgsConstructor
private static final class ReadAllBytesHandler implements HttpHandler {

private final FullBytesCallback callback;

@Override
public void handleRequest (HttpServerExchange exchange) throws Exception {
exchange.getRequestReceiver()
.receiveFullBytes(this::handleBytes);
}

private void handleBytes (HttpServerExchange exchange, byte[] message) {
try {
callback.handle(exchange, message);
} catch (Throwable ex) {
exchange.setStatusCode(500);
exchange.getResponseHeaders()
.put(Headers.CONTENT_TYPE, "text/plain");
exchange.getResponseSender()
.send(ex.getMessage());
}
}
}
}
69 changes: 69 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,19 @@ limitations under the License.
<scope>test</scope>
</dependency>

<dependency>
<groupId>io.undertow</groupId>
<artifactId>undertow-core</artifactId>
<version>2.3.14.Final</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.appulse</groupId>
<artifactId>utils-java</artifactId>
<version>1.18.0</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
Expand Down Expand Up @@ -402,6 +415,62 @@ limitations under the License.
</executions>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>3.1.2</version>
<configuration>
<skipTests>${skipUnitTests}</skipTests>
<trimStackTrace>false</trimStackTrace>
<includes>
<include>**/*Test.java</include>
<include>**/*Tests.java</include>
<include>**/Test*.java</include>
</includes>
<excludes>
<exclude>**/it/**</exclude>
<exclude>**/*IntegrationTest.java</exclude>
<exclude>**/*IntegrationTests.java</exclude>
<exclude>**/*IT.java</exclude>
<exclude>**/IT*.java</exclude>
</excludes>
</configuration>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-failsafe-plugin</artifactId>
<version>3.1.2</version>
<configuration>
<skipITs>${skipIntegrationTests}</skipITs>
<trimStackTrace>false</trimStackTrace>
<includes>
<include>**/*IntegrationTest.java</include>
<include>**/*IntegrationTests.java</include>
<include>**/*IT.java</include>
<include>**/IT*.java</include>
<include>**/it/**/*Test.java</include>
<include>**/it/**/*Tests.java</include>
<include>**/it/**/Test*.java</include>
</includes>
<classpathDependencyExcludes>
<classpathDependencyExcludes>${project.groupId}:${project.artifactId}</classpathDependencyExcludes>
</classpathDependencyExcludes>
<additionalClasspathElements>
<additionalClasspathElement>${project.build.outputDirectory}</additionalClasspathElement>
</additionalClasspathElements>
</configuration>
<executions>
<execution>
<id>integration-test</id>
<goals>
<goal>integration-test</goal>
<goal>verify</goal>
</goals>
</execution>
</executions>
</plugin>

<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.13.0</version>
Expand Down

0 comments on commit 63ba1e9

Please sign in to comment.