openfeign / feignx Goto Github PK

See #22

Scenario: Simple HTTP request that obtains a response.

Given: a request is made to a known endpoint,
When: the request is send to the target,
Then: a response is returned with a 200 - OK Status Code,
And: the response content length is not zero,
And: the response body contains information.

Additional Context
Any additional context you would like to provide.

Feignx replaces a number of core Feign interfaces. This non-backward compatible breaking change will hinder adoption for those that have built their own extensions. However, most of the breaking changes are in name only, not concept:

• Encoder => RequestEncoder
• Decoder => ResponseDecoder
• ErrorDecoder => ErrorHandler

This should allow us to create a compatibility library that bridges the Feign components with the new Feignx components, allowing the library of existing extensions to take advantages of Feignx's improvements.

See #22

Scenario: Simple HTTP request that results for a large file can be streamed from the Response by interacting with the input stream directly.

Given: a request is made to a known endpoint,
When: the request is sent to the target,
And: a response is returned with a 200 response code
And: the response is not empty
Then: the input stream provided by the Response.

Additional Context
This test case should demonstrate that it is possible to handle the response streams directly by users, either in a decoder or bypassing the decoder entirely.

Feignx replicates the same issue that exits in regular Feign, it uses methods for reflection that are no longer valid in JDK 9+. These must be addressed before later JDK version remove the warning and make them an error.

To Reproduce
Steps to reproduce the behavior:

1. Create a new Feign Interface that uses a default method
2. Run the application on JDK 9+
3. Observe the following error: Illegal reflective access by feign.proxy.GuardMethodHandler (file:...) to constructor java.lang.invoke.MethodHandles$Lookup(java.lang.Class)

Expected behavior
That on JDK 9+ this error doesn't occur

Environment (please complete the following information):

• JDK Version [9+]

Additional context
Since Feignx does not support JDK versions prior to 11, we can safely change to the updated method for obtaining this information.

Create a module that contains a number of high-level functional tests that verify that FeignX is working as expected. Tests should include:

Basic API Scenarios:

• 200 - OK Response with data.
• 200 - OK Response without data.
• 204 - OK Response.
• 300 - Redirect.
• 4xx - Client Error
• 5xx - Server Error

Large Response Scenarios

• Streaming a large binary file, similar to a file download.
• GZIP request and responses.
• Binary request body.

Each scenario should have it's own Issue with the following information

• Scenario Description.
• Expected Inputs.
• Expected Results.

See #22

Scenario: Simple HTTP request, that was redirected and results in a 200 OK response.

Given: a request is made to a known endpoint,
When: the request is sent to the target and redirected,
Then: a response is returned with a 200 - NO CONTENT Status Code,
And: the response content length is not zero,
And: the response body is not empty.

Additional Context
Any additional context you would like to provide.

Feign currently supports Hystrix only. This support is widely used and mature and I would like to see it added to FeignX.

When doing so, we should consider creating a general abstraction for these types of extensions so users can choose between any Circuit Breaker implementation. At a minimum, we should consider not only Hystrix, but also Resilience4J, due to Hystrix's current status.

See #22

Scenario: Simple HTTP request that results in a 4xx response code, is sent to the Exception Handler.

Given: a request is made to a known endpoint,
When: the request is sent to the target,
And: a response is returned with a 4xx response code
Then: the exception handler is invoked.

Additional Context
This test case should not attempt to decode the response into the desired response type because an error occurred. That should be up to the Exception Handler to manage.

Feign allows users to supply an Expander as a property for any Template Parameter (@Param) annotated method parameters, that control how that parameter value is "expanded" onto the uri template. The current solution is sufficient, but does have the following limitations:

• Cannot be singletons
• Must have a no-arg constructor
• Are created each expansion

In Feignx, we should consider providing this support but address these limitations. One possible solution is to provide a ExpanderRegistry or ExpanderFactory, that can be shared between Feign Targets. Users can register their own expanders in the registry and specify which expander to use when defining the Template Parameter's (@Param) encoder property, using the class name of the Encoder instance. During expansion, the desired Expander will be retrieved from the registry and applied.

Using this approach, expanders can be singletons and cached/shared to avoid undesired new object creation.

Users should be able to control expander caching, however, so we should consider allowing them to specify how they want the expander used when registering. Here is one possible example:

public class ExpanderRegistry {
   private Map<String, Expander> expanders = new LinkedHashMap<>();
   private Map<String, Class<? extends Expander>> instances = new LinkedHashMap<>();
      
   // register a singleton
   public void register(Expander expander) {
      this.expanders.put(expander.getClass().getSimpleName(), expander);
   }
   
   // register an expander definition
   public void register(Class<? extends Expander> expander) {
      this.instances.put(expander.getSimpleName(), expander);
   }

   public Expander getExpander(String name) {
      /* check the singleton first, the create a new instance */
      return expander;
   }
}

This register should be an optional parameter provided on the FeignConfigurationBuilder

Feign.builder()
   .registry(ExpanderRegistry)
   .expander(Expander)
   .expander(Expander.class)
   .target(Class, "https://uri");

I think all of this together will provide a good balance between sensible defaults and user supplied customization.

See #22

Scenario: Simple HTTP request can be where the request content is binary data provided directly by a user, not through the encoded.

Given: a request is made to a known endpoint,
When: the request is sent to the target,
And: the request contains binary data
And: a response is returned with a 200 response code
Then: the response can be read.

Additional Context
This test case verifies that users can provide byte[] or other binary type values as request content and that the requests should send that data to the target raw.

See #22

Scenario: Simple HTTP request can be made requesting responses be compressed.

Given: a request is made to a known endpoint,
When: the request is sent to the target, accepting gzip responses
And: a response is returned with a 200 response code
And: the response is not empty
Then: the response can be read.

Additional Context
This test case should demonstrate that gzip support is relegated to the Client being used, but should be supported by the default client.

The name is somewhat redundant and we should name the modules similar to how we would name packages.

As with #18, Feign supports non-blocking, reactive execution modes through wrappers andthrough Hystrix's Observable support. Given Feign's originated before non-blocking techniques such as reactive stream existed, attempting to add support to Feign has proven difficult. Non-blocking execution requires a mindset change due to it's use of an event loop and assumption that all events operate asynchronously.

We should take this opportunity to discuss how we can add non-blocking support, through the use of Reactive Streams or native java.util.concurrent.Flow available in JDK 9+.

Types to consider are:

• java.util.concurrent.Flow.Publisher
• org.reactivestreams.Publisher

Methods that specify these return types should be executed on an executor pool, either of the user's choosing or a reasonable default one, with their results returned directly, ideally after decoding the response. Care must be taken to ensure that any step in the request process that may block be wrapped and executed asynchronously to avoid blocking the event loop. The following list of components are capable of blocking:

• RequestEncoder
• RequestInterceptor
• Client
• ResponseDecoder
• ExceptionHandler

I suspect that the TargetMethodHandler implementation for these types will need to determine how to handle these cases.

See #22

Scenario: Simple HTTP request that results in a 5xx response code, is sent to the Exception Handler.

Given: a request is made to a known endpoint,
When: the request is sent to the target,
And: a response is returned with a 5xx response code
Then: the exception handler is invoked.

Additional Context
This test case should not attempt to decode the response into the desired response type because an error occurred. That should be up to the Exception Handler to manage.

Feign provides limited support for template parameters that are not simple values like String or Map. It does allow users to define their own expansion by defining an expander on the @Param annotation, however Expander instances cannot be singletons and must have a default constructor. This limits reuse and requires that a new instance be created for each expansion.

In Feignx, we should consider providing built-in support for Object expansion, using a well known specification or convention, such as the Java Bean Specification. The result of this expansion is a Map representing the properties of the bean, that can be be applied to a uri template.

Example Expander interface

package feign.template;

@FunctionalInterface
public interface Expander {
    Map<String, Object> expand(Serializable object);
}

Feignx should still allow for the use of a user supplied Expander; however, we should pay close attention to the limitations of the current approach.

Feign enables retrying of requests only when the request is an exception or a user throws a RetryableException from an error handler. While sufficient for basic circumstances, like timeouts, handling more complex situations requires users take more control of the process than necessary.

Feignx should allow users to specify when a request should be retried. Here are few examples of some built-in retry policies we could supply to users:

• Status Code: automatically retries the request based on the status code received.
• Exception: automatically retries the request based on a given set of exceptions. This is similar to Feign's current support, but instead of requiring users to extend an internal Exception type, users can supply their own exception types from their own domain.

We may also want to consider other good habits, such as continuing to honor Retry-After headers and support "back off" policies. Combining these two concepts together will produce a simply, yet powerful retry mechanism.

See #22

Scenario: Simple HTTP request that obtains an empty response.

Given: a request is made to a known endpoint,
When: the request is send to the target,
Then: a response is returned with a 200 - OK Status Code,
And: the response content length is zero,
And: the response body is empty.

Additional Context
Any additional context you would like to provide.

Feign's DeclarativeContract uses FunctionalInterface AnnotationProcessors to simply contract parsing and metadata building.

This type of annotation processing should be supported in Feignx for the following reasons:

• Backward Compatibility
• Simplicity
• APT groundwork.

Similar to #19, Server Sent Events are a one-way push style execution mode that FeignX should consider supporting. This can be accomplished easily if we have support reactive streams, but we should also consider provided support that does not rely on reactive streams or Flow.

One possibility is to provide a specific ResponseDecoder that can process responses with the content type of text/event-stream and push all decoded responses to a java.util.Queue. I suspect that there are more interesting and clever ways of accomplishing this, but I think that having something user's can take advantage of out of the box would be useful.

Please create an initial set of user documentation that can, eventually, be published. Ideally we would have something that can be generated each build and published to GitHub pages, or any other hosting provider, when we deploy either a snapshot or release.

Even better would be if we could keep historical version and let user filter on which version they are using.

Feign provides support for the mime-type: multipart/form-data through the feign-form extension. Many APIs and other legacy system support application/x-www-form-urlencoded and multipart/form-data as a means to accept content.

Feignx should provide support for these types without the need for an extension. Ideally this support would be enabled by sniffing the Content-Type header provided by the user on the method signature. Another option is to create a specific annotation for form parameters, such as @FormParam.

Supporting forms this way to bringing parity between Feignx and Feign.

At the moment, there is only one module, but we will be adding many more modules in the very near future. We should enable JaCoCo aggregate coverage reports to keep track of code coverage across the entire project.

Add an additional job to the Circle CI configuration that creates a new GitHub release for each release to Maven Central.

Describe the bug
Circle CI is not storing and, as a result, not keeping track of the test case results.

To Reproduce
Steps to reproduce the behavior:

1. Run a Circle Ci build
2. Observe that the Test Results are not available under the "artifacts" tab.

Expected behavior
That under the "artifacts" tab, the Surefire tests results reports would be available.

Additional context
This appears to be a configuration issue in the test step. It's pointing to the wrong directory. Updating it to the aggregate location will fix it.

One of the shortcomings of Feign is that support asynchronous execution modes is limited to those using Hystrix and their Observable support. Others have looked into supporting this, but have come the same conclusion, adding support for async modes requires a reconsideration of how Feign manages request execution.

Given that this project is "green-field", I would like to propose that we consider supporting, at a minimum, Future and CompletableFuture return types in the core library.

Methods that specify these return types should be executed on an executor pool, either of the user's choosing or a reasonable default one, with their results returned directly, ideally after decoding the response. Here is an example:

public interface Blog {
   @Request(value = "/posts", method = HttpMethod.GET)
   CompletableFuture<Posts> getPosts();
}

public class BlogPostRepository {
   private BlogService blog;

  public Collection<Post> getPosts() {
     CompleteableFuture<Collection<Post> posts = this.blog.getPosts()
         .thenAccept(posts -> posts.stream().collect(Collectors.toList());
     return posts.get();
  }
}

This, in my mind, would be an ideal place to start.

See #22

Scenario: Simple HTTP request that obtains a NO CONTENT response.

Given: a request is made to a known endpoint,
When: the request is send to the target,
Then: a response is returned with a 204 - NO CONTENT Status Code,
And: the response content length is zero,
And: the response body is empty.

Additional Context
Any additional context you would like to provide.

With Circle now set up, we need to update the job workflow to include deployments of snapshots and releases to Maven Central.