OpenFeign / feign

Feign makes writing java http clients easier
Apache License 2.0
9.49k stars 1.93k forks source link

version 10.2.0 does not support resolve parameters in request path in RequestInterceptor #915

Closed LareinaH closed 5 years ago

LareinaH commented 5 years ago

in version 10.0.1, I could invoke template.resolve in RequestInterceptor to replace parameters in request path

RequestTemplate resolve(Map unencoded, Map alreadyEncoded) {
    replaceQueryValues(unencoded, alreadyEncoded);
    Map encoded = new LinkedHashMap();
    for (Entry entry : unencoded.entrySet()) {
      final String key = entry.getKey();
      final Object objectValue = entry.getValue();
      String encodedValue = encodeValueIfNotEncoded(key, objectValue, alreadyEncoded);
      encoded.put(key, encodedValue);
    }
    String resolvedUrl = expand(url.toString(), encoded).replace("+", "%20");
    if (decodeSlash) {
      resolvedUrl = resolvedUrl.replace("%2F", "/");
    }
    url = new StringBuilder(resolvedUrl);

    Map> resolvedHeaders =
        new LinkedHashMap>();
    for (String field : headers.keySet()) {
      Collection resolvedValues = new ArrayList();
      for (String value : valuesOrEmpty(headers, field)) {
        String resolved = expand(value, unencoded);
        resolvedValues.add(resolved);
      }
      resolvedHeaders.put(field, resolvedValues);
    }
    headers.clear();
    headers.putAll(resolvedHeaders);
    if (bodyTemplate != null) {
      body(urlDecode(expand(bodyTemplate, encoded)));
    }
    return this;
  }

while in version 10.2.0, I find it unavailable, for UriTemplate.resolve is execute before RequestInterceptor, and the expresion is ignored,

public String expand(Map variables) {
    if (variables == null) {
      throw new IllegalArgumentException("variable map is required.");
    }

    /* resolve all expressions within the template */
    StringBuilder resolved = new StringBuilder();
    for (TemplateChunk chunk : this.templateChunks) {
      if (chunk instanceof Expression) {
        Expression expression = (Expression) chunk;
        Object value = variables.get(expression.getName());
        if (value != null) {
          String expanded = expression.expand(value, this.encode.isEncodingRequired());
          if (this.encodeSlash) {
            logger.fine("Explicit slash decoding specified, decoding all slashes in uri");
            expanded = expanded.replaceAll("/", "%2F");
          }
          resolved.append(expanded);
        } else {
          if (this.allowUnresolved) {
            /* unresolved variables are treated as literals */
            resolved.append(encode(expression.toString()));
          }
          // here, expresion is ignored, in UriTemplate docs, "unresolved variables are preserved as literals", but ignored with init param ExpansionOptions.REQUIRED
          may i have solutions to stay the same behavior with old versions ?
        }
      } else {
        /* chunk is a literal value */
        resolved.append(chunk.getValue());
      }
    }
    return resolved.toString();
  }

maybe uritemplate shoud init with ALLOW_UNRESOLVED below:

private UriTemplate(String template, boolean encodeSlash, Charset charset) {
    super(template, ExpansionOptions.REQUIREDALLOW_UNRESOLVED, EncodingOptions.REQUIRED, encodeSlash, charset);
  }
kdavisk6 commented 5 years ago

Request Interceptors have always been applied after the template is resolved. That behavior is not new. Can you please provide a sample of your Request Interceptor to help us understand what you are trying to accomplish? We may be able to provide a workaround.

LareinaH commented 5 years ago

for example, several server apis are defined below:

@RequestLine("GET /openapi/api1/query1/{_accessId}/{param}")
Object query1(
        @Param("param") String param
);
@RequestLine("GET /openapi/api2/query2/{_accessId}/{param}")
Object query2(
        @Param("param") String param
);
@RequestLine("GET /openapi/api3/query3/{_accessId}/{param}")
Object query3(
        @Param("param") String param
);

@kdavisk6 {_accessId} param is common and same for all apis, before openfeign v10.2.0, RequestInterceptor could replace the common param in URI, but after v10.2.0, UriTemplate is init with ExpansionOptions.REQUIRED which makes /{_accessId}/ to // in RequestInterceptor process

kdavisk6 commented 5 years ago

@LareinaH

I'm surprised your example worked in previous versions. I suspect it was due to the many different, inconsistent template handling techniques we removed as part of 10.x. In order to remain compliant with RFC 6570, unresolved expressions that appear in the uri path are not allowed and must be removed, otherwise the result would be an invalid uri. Simply put, you cannot have an expression in the uri path that does not resolve and remain in the path.

Getting back to your issue, with the template being resolved before it is passed to the RequestInterceptor, your current technique is not going to work any longer. You are going to need to revisit that design. Your best approach would be to change your method signatures to accept the accessId as a method parameter, as that is the only option that will guarantee that accessId is resolved and remains on the uri.

kdavisk6 commented 5 years ago

I'm going to close this issue and recommend that questions like this be made on Stack Overflow, allowing more users to help you.