Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flux type response should be corresponding to List #2199

Merged
merged 7 commits into from
Oct 14, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 36 additions & 0 deletions core/src/main/java/feign/Types.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.util.Arrays;
import java.util.NoSuchElementException;

import static feign.Util.checkState;

/**
* Static methods for working with types.
*
Expand Down Expand Up @@ -325,6 +327,40 @@ public static Type resolveReturnType(Type baseType, Type overridingType) {
return baseType;
}

/**
* Resolves the last type parameter of the parameterized {@code supertype}, based on the {@code
* genericContext}, into its upper bounds.
* <p/>
* Implementation copied from {@code
* retrofit.RestMethodInfo}.
*
* @param genericContext Ex. {@link java.lang.reflect.Field#getGenericType()}
* @param supertype Ex. {@code Decoder.class}
* @return in the example above, the type parameter of {@code Decoder}.
* @throws IllegalStateException if {@code supertype} cannot be resolved into a parameterized type
* using {@code context}.
*/
public static Type resolveLastTypeParameter(Type genericContext, Class<?> supertype)
throws IllegalStateException {
Type resolvedSuperType =
Types.getSupertype(genericContext, Types.getRawType(genericContext), supertype);
checkState(resolvedSuperType instanceof ParameterizedType,
"could not resolve %s into a parameterized type %s",
genericContext, supertype);
Type[] types = ParameterizedType.class.cast(resolvedSuperType).getActualTypeArguments();
for (int i = 0; i < types.length; i++) {
Type type = types[i];
if (type instanceof WildcardType) {
types[i] = ((WildcardType) type).getUpperBounds()[0];
}
}
return types[types.length - 1];
}

public static ParameterizedType parameterize(Class<?> rawClass, Type... typeArguments) {
return new ParameterizedTypeImpl(rawClass.getEnclosingClass(), rawClass, typeArguments);
}

static final class ParameterizedTypeImpl implements ParameterizedType {

private final Type ownerType;
Expand Down
27 changes: 3 additions & 24 deletions core/src/main/java/feign/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -215,33 +215,12 @@ public static void ensureClosed(Closeable closeable) {
}

/**
* Resolves the last type parameter of the parameterized {@code supertype}, based on the {@code
* genericContext}, into its upper bounds.
* <p/>
* Implementation copied from {@code
* retrofit.RestMethodInfo}.
*
* @param genericContext Ex. {@link java.lang.reflect.Field#getGenericType()}
* @param supertype Ex. {@code Decoder.class}
* @return in the example above, the type parameter of {@code Decoder}.
* @throws IllegalStateException if {@code supertype} cannot be resolved into a parameterized type
* using {@code context}.
* Moved to {@code feign.Types.resolveLastTypeParameter}
*/
@Deprecated
public static Type resolveLastTypeParameter(Type genericContext, Class<?> supertype)
throws IllegalStateException {
Type resolvedSuperType =
Types.getSupertype(genericContext, Types.getRawType(genericContext), supertype);
checkState(resolvedSuperType instanceof ParameterizedType,
"could not resolve %s into a parameterized type %s",
genericContext, supertype);
Type[] types = ParameterizedType.class.cast(resolvedSuperType).getActualTypeArguments();
for (int i = 0; i < types.length; i++) {
Type type = types[i];
if (type instanceof WildcardType) {
types[i] = ((WildcardType) type).getUpperBounds()[0];
}
}
return types[types.length - 1];
return Types.resolveLastTypeParameter(genericContext, supertype);
}

/**
Expand Down
58 changes: 20 additions & 38 deletions reactive/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,34 @@ implementation to your classpath. Then configure Feign to use the reactive stre
public interface GitHubReactor {

@RequestLine("GET /repos/{owner}/{repo}/contributors")
Flux<Contributor> contributors(@Param("owner") String owner, @Param("repo") String repo);
Flux<Contributor> contributorsFlux(@Param("owner") String owner, @Param("repo") String repo);

@RequestLine("GET /repos/{owner}/{repo}/contributors")
Mono<List<Contributor>> contributorsMono(@Param("owner") String owner, @Param("repo") String repo);
Comment on lines 16 to +20
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The contributorsFlux and contributorsMono methods have the same @RequestLine annotation. This could lead to confusion and potential errors. Consider renaming these methods to better reflect their return types or the data they are fetching.


class Contributor {
String login;

public Contributor(String login) {
this.login = login;
}
String login;

public String getLogin() {
return login;
}

public void setLogin(String login) {
this.login = login;
}
}
}

public class ExampleReactor {
public static void main(String args[]) {
GitHubReactor gitHub = ReactorFeign.builder()
GitHubReactor gitHub = ReactorFeign.builder()
.decoder(new ReactiveDecoder(new JacksonDecoder()))
.target(GitHubReactor.class, "https://api.github.com");

List<Contributor> contributors = gitHub.contributors("OpenFeign", "feign")
.collect(Collectors.toList())
List<GitHubReactor.Contributor> contributorsFromFlux = gitHub.contributorsFlux("OpenFeign", "feign")
.collectList()
.block();
List<GitHubReactor.Contributor> contributorsFromMono = gitHub.contributorsMono("OpenFeign", "feign")
.block();
}
}
Expand Down Expand Up @@ -79,33 +89,5 @@ the wrapped in the appropriate reactive wrappers.
### Iterable and Collections responses

Due to the Synchronous nature of Feign requests, methods that return `Iterable` types must specify the collection
in the `Publisher`. For `Reactor` types, this limits the use of `Flux` as a response type. If you
want to use `Flux`, you will need to manually convert the `Mono` or `Iterable` response types into
`Flux` using the `fromIterable` method.

in the `Publisher`. For `Reactor` types, this limits the use of `Flux` as a response type.
Comment on lines 90 to +93
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of the example code that demonstrates how to manually convert Mono or Iterable response types into Flux using the fromIterable method could potentially confuse users. Consider adding a similar example or providing a link to documentation that explains this process.


```java
public interface GitHub {

@RequestLine("GET /repos/{owner}/{repo}/contributors")
Mono<List<Contributor>> contributors(@Param("owner") String owner, @Param("repo") String repo);

class Contributor {
String login;

public Contributor(String login) {
this.login = login;
}
}
}

public class ExampleApplication {
public static void main(String[] args) {
GitHub gitHub = ReactorFeign.builder()
.target(GitHub.class, "https://api.github.com");

Mono<List<Contributor>> contributors = gitHub.contributors("OpenFeign", "feign");
Flux<Contributor> contributorFlux = Flux.fromIterable(contributors.block());
}
}
```
50 changes: 50 additions & 0 deletions reactive/src/main/java/feign/reactive/ReactiveDecoder.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright 2012-2023 The Feign 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.reactive;

import feign.FeignException;
import feign.Response;
import feign.Types;
import feign.codec.Decoder;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;

import java.io.IOException;
import java.lang.reflect.Type;
import java.util.List;

public class ReactiveDecoder implements Decoder {

private final Decoder delegate;

public ReactiveDecoder(Decoder decoder) {
this.delegate = decoder;
}

@Override
public Object decode(Response response, Type type) throws IOException, FeignException {
Class<?> rawType = Types.getRawType(type);
if (rawType.isAssignableFrom(Mono.class)) {
Type lastType = Types.resolveLastTypeParameter(type, Mono.class);
return Mono.just(delegate.decode(response, lastType));
}
if (rawType.isAssignableFrom(Flux.class)) {
Type lastType = Types.resolveLastTypeParameter(type, Flux.class);
Type listType = Types.parameterize(List.class, lastType);
return Flux.fromIterable((Iterable)delegate.decode(response, listType));
}

return delegate.decode(response, type);
}
}
gromspys marked this conversation as resolved.
Show resolved Hide resolved