Merge branch 'dbaje-bug/2511'

fixes #2511

springfox-core/src/main/java/springfox/documentation/common/SpringVersion.java

@@ -0,0 +1,45 @@
+/*
+ *
+ *  Copyright 2018 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 springfox.documentation.common;
+
+/**
+ * Class providing a proxy for @{@link org.springframework.core.SpringVersion}
+ * This enable mocking and testing of specific situations
+ * @author dbaje
+ */
+public class SpringVersion {
+    /**
+     * Return the full version string of the present Spring codebase,
+     * or {@code null} if it cannot be determined.
+     * @see Package#getImplementationVersion()
+     */
+    public Version getVersion() {
+        return Version.parse(getVersionString());
+    }
+
+    /**
+     * Return the full version string of the present Spring codebase,
+     * or {@code null} if it cannot be determined.
+     * @see Package#getImplementationVersion()
+     */
+    private String getVersionString() {
+        return org.springframework.core.SpringVersion.getVersion();
+    }
+}

springfox-swagger-common/src/main/java/springfox/documentation/swagger/common/SpringVersionCapability.java → springfox-core/src/main/java/springfox/documentation/common/SpringVersionCapability.java

@@ -16,23 +16,26 @@
  *
  *
  */
-package springfox.documentation.swagger.common;
+package springfox.documentation.common;
 
 public class SpringVersionCapability {
 
   private static final Version FIVE_ZERO_ZERO = Version.parse("5.0.0.RELEASE");
   private static final Version FIVE_ZERO_FIVE = Version.parse("5.0.5.RELEASE");
+  private static final Version FOUR_THREE_THREE = Version.parse("4.3.3.RELEASE");
   private static final Version FOUR_THREE_FIFTEEN = Version.parse("4.3.15.RELEASE");
 
   public SpringVersionCapability() {
     throw new UnsupportedOperationException();
   }
 
-  public static boolean supportsXForwardPrefixHeader(String version) {
-    Version parsed = Version.parse(version);
+  public static boolean supportsXForwardPrefixHeader(Version version) {
+    return (version.isGreaterThanOrEqualTo(FOUR_THREE_FIFTEEN)
+                && version.isLessThan(FIVE_ZERO_ZERO)) ||
+            version.isGreaterThanOrEqualTo(FIVE_ZERO_FIVE);
+  }
 
-    return (parsed.isGreaterThanOrEqualTo(FOUR_THREE_FIFTEEN)
-                && parsed.isLessThan(FIVE_ZERO_ZERO)) || 
-        parsed.isGreaterThanOrEqualTo(FIVE_ZERO_FIVE);
+  public static boolean supportsExtendedPathVariableAnnotation(Version version) {
+    return version.isGreaterThanOrEqualTo(FOUR_THREE_THREE);
   }
 }

springfox-swagger-common/src/main/java/springfox/documentation/swagger/common/Version.java → springfox-core/src/main/java/springfox/documentation/common/Version.java

@@ -37,7 +37,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package springfox.documentation.swagger.common;
+package springfox.documentation.common;
 
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;

springfox-core/src/test/groovy/springfox/documentation/common/SpringVersionCapabilitySpec.groovy

@@ -0,0 +1,63 @@
+package springfox.documentation.common
+
+import spock.lang.Specification
+import spock.lang.Unroll
+
+import static springfox.documentation.common.SpringVersionCapability.*
+
+class SpringVersionCapabilitySpec extends Specification {
+
+  def "Cannot instantitiate SpringVersionCompatibility"() {
+    when:
+    new SpringVersionCapability()
+
+    then:
+    thrown UnsupportedOperationException
+  }
+
+  @Unroll
+  def "Version #springVersion should preservePath #expected"() {
+    when:
+    def result = !supportsXForwardPrefixHeader(Version.parse(springVersion))
+
+    then:
+    result == expected
+
+    where:
+    springVersion     | expected
+    "3.10.20.RELEASE" | true
+    "4.2.20.RELEASE"  | true
+    "4.3.14.RELEASE"  | true
+    "4.3.15.RELEASE"  | false
+    "4.4.16.RELEASE"  | false
+    "5.0.0.RELEASE"   | true
+    "5.0.5.RELEASE"   | false
+    "5.1.0.RELEASE"   | false
+    "5.1.5.RELEASE"   | false
+    "6.1.6.RELEASE"   | false
+  }
+
+  @Unroll
+  def "Version #springVersion should has extended PathVariable annotation #expected"() {
+    when:
+    def result = supportsExtendedPathVariableAnnotation(Version.parse(springVersion))
+
+    then:
+    result == expected
+
+    where:
+    springVersion     | expected
+    "3.10.20.RELEASE" | false
+    "4.2.20.RELEASE"  | false
+    "4.3.2.RELEASE"   | false
+    "4.3.3.RELEASE"   | true
+    "4.3.14.RELEASE"  | true
+    "4.3.15.RELEASE"  | true
+    "4.4.16.RELEASE"  | true
+    "5.0.0.RELEASE"   | true
+    "5.0.5.RELEASE"   | true
+    "5.1.0.RELEASE"   | true
+    "5.1.5.RELEASE"   | true
+    "6.1.6.RELEASE"   | true
+  }
+}

springfox-spring-web/src/main/java/springfox/documentation/spring/web/readers/parameter/ParameterRequiredReader.java

@@ -29,6 +29,7 @@
 import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.RequestPart;
 import org.springframework.web.bind.annotation.ValueConstants;
+import springfox.documentation.common.SpringVersion;
 import springfox.documentation.service.ResolvedMethodParameter;
 import springfox.documentation.spi.DocumentationType;
 import springfox.documentation.spi.service.ParameterBuilderPlugin;
@@ -46,11 +47,17 @@
 @Component
 @Order(Ordered.HIGHEST_PRECEDENCE)
 public class ParameterRequiredReader implements ParameterBuilderPlugin {
+  private final SpringVersion springVersion;
   private final DescriptionResolver descriptions;
 
   @Autowired
   public ParameterRequiredReader(DescriptionResolver descriptions) {
+    this(descriptions, new SpringVersion());
+  }
+
+  ParameterRequiredReader(DescriptionResolver descriptions, SpringVersion springVersion) {
     this.descriptions = descriptions;
+    this.springVersion = springVersion;
   }
 
   @Override
@@ -83,7 +90,7 @@ private boolean isRequired(
 
     Optional<PathVariable> pathVariable = methodParameter.findAnnotation(PathVariable.class);
     if (pathVariable.isPresent()) {
-      String paramName = ofNullable(pathVariable.get().name()).filter(((Predicate<String>)String::isEmpty).negate())
+      String paramName = ofNullable(pathVariable.get().name()).filter(((Predicate<String>) String::isEmpty).negate())
           .orElse(methodParameter.defaultName().orElse(null));
 
       if (pathVariable.get().required() ||
@@ -106,7 +113,7 @@ private boolean optionalButPresentInThePath(
       String paramName) {
 
     return !pathVariable.required()
-         && operationContext.requestMappingPattern().contains("{" + paramName + "}");
+        && operationContext.requestMappingPattern().contains("{" + paramName + "}");
   }
 
   @SuppressWarnings("squid:S1872")

springfox-spring-web/src/test/groovy/springfox/documentation/spring/web/readers/parameter/ParameterAnnotationSupport.groovy

@@ -20,7 +20,8 @@ trait ParameterAnnotationSupport {
     [
         value   : { -> value },
         name    : { -> value },
-        required: { -> required }
+        required: { -> required },
+        annotationType: { PathVariable.class }
     ] as PathVariable
   }
 

springfox-spring-web/src/test/groovy/springfox/documentation/spring/web/readers/parameter/ParameterRequiredReaderSpec.groovy

@@ -26,6 +26,8 @@ import org.springframework.web.bind.annotation.PathVariable
 import spock.lang.Shared
 import spock.lang.Unroll
 import springfox.documentation.builders.ParameterBuilder
+import springfox.documentation.common.SpringVersion
+import springfox.documentation.common.Version
 import springfox.documentation.service.ResolvedMethodParameter
 import springfox.documentation.spi.DocumentationType
 import springfox.documentation.spi.schema.GenericTypeNamingStrategy
@@ -70,42 +72,47 @@ class ParameterRequiredReaderSpec extends DocumentationContextSpec implements Pa
             documentationContext(),
             Mock(GenericTypeNamingStrategy),
             operation)
+    and:
+    def springVersion = Mock(SpringVersion.class);
+    springVersion.getVersion() >> Version.parse(version)
 
     when:
-    def operationCommand = new ParameterRequiredReader(description)
+    def operationCommand = new ParameterRequiredReader(description, springVersion)
     operationCommand.apply(parameterContext)
 
     then:
     parameterContext.parameterBuilder().build().isRequired() == expected
 
     where:
-    paramAnnotations                                        | requestPattern           | expected
-    [apiParam(false), pathVariableRequired()]               | "/path/{required-param}" | true
-    [apiParam(true), pathVariableRequired()]                | "/path/{required-param}" | true
-    [apiParam(false), pathVariableOptional()]               | "/path/{optional-param}" | true
-    [apiParam(false), pathVariableOptional()]               | "/path"                  | false
-    [apiParam(true), pathVariableOptional()]                | "/path/{optional-param}" | true
-    [apiParam(true), pathVariableOptional()]                | "/path"                  | false
-    [apiParam(false), requestHeader(false, "", "")]         | "/path"                  | false
-    [requestHeader(true, "", "")]                           | "/path"                  | true
-    [requestHeader(false, "", "")]                          | "/path"                  | false
-    [apiParam(true)]                                        | "/path"                  | false
-    [apiParam(false)]                                       | "/path"                  | false
-    [requestParam(true, "", DEFAULT_NONE)]                  | "/path"                  | true
-    [requestParam(true, "", "")]                            | "/path"                  | false
-    [requestParam(true, "", null)]                          | "/path"                  | false
-    [requestParam(true, "", "default value")]               | "/path"                  | false
-    [requestParam(false, "", DEFAULT_NONE)]                 | "/path"                  | false
-    [requestParam(false, "", "")]                           | "/path"                  | false
-    [requestBody(false)]                                    | "/path"                  | false
-    [requestBody(true)]                                     | "/path"                  | true
-    [requestPart(false, "")]                                | "/path"                  | false
-    [requestPart(true, "")]                                 | "/path"                  | true
-    []                                                      | "/path"                  | false
-    [null]                                                  | "/path"                  | false
-    [apiParam(true), requestParam(false, "", DEFAULT_NONE)] | "/path"                  | false
-    [apiParam(false), requestParam(true, "", DEFAULT_NONE)] | "/path"                  | true
-    [apiParam(false), requestParam(true, "", DEFAULT_NONE)] | "/path"                  | true
+    paramAnnotations                                        | version         | requestPattern           | expected
+    [apiParam(false), pathVariableRequired()]               | "4.3.3.RELEASE" | "/path/{required-param}" | true
+    [apiParam(false), pathVariableRequired()]               | "4.2.0.RELEASE" | "/path/{required-param}" | true
+    [apiParam(true), pathVariableRequired()]                | "4.3.3.RELEASE" | "/path/{required-param}" | true
+    [apiParam(true), pathVariableRequired()]                | "4.2.0.RELEASE" | "/path/{required-param}" | true
+    [apiParam(false), pathVariableOptional()]               | "4.3.3.RELEASE" | "/path/{optional-param}" | true
+    [apiParam(false), pathVariableOptional()]               | "4.3.3.RELEASE" | "/path"                  | false
+    [apiParam(true), pathVariableOptional()]                | "4.3.3.RELEASE" | "/path/{optional-param}" | true
+    [apiParam(true), pathVariableOptional()]                | "4.3.3.RELEASE" | "/path"                  | false
+    [apiParam(false), requestHeader(false, "", "")]         | "4.3.3.RELEASE" | "/path"                  | false
+    [requestHeader(true, "", "")]                           | "4.3.3.RELEASE" | "/path"                  | true
+    [requestHeader(false, "", "")]                          | "4.3.3.RELEASE" | "/path"                  | false
+    [apiParam(true)]                                        | "4.3.3.RELEASE" | "/path"                  | false
+    [apiParam(false)]                                       | "4.3.3.RELEASE" | "/path"                  | false
+    [requestParam(true, "", DEFAULT_NONE)]                  | "4.3.3.RELEASE" | "/path"                  | true
+    [requestParam(true, "", "")]                            | "4.3.3.RELEASE" | "/path"                  | false
+    [requestParam(true, "", null)]                          | "4.3.3.RELEASE" | "/path"                  | false
+    [requestParam(true, "", "default value")]               | "4.3.3.RELEASE" | "/path"                  | false
+    [requestParam(false, "", DEFAULT_NONE)]                 | "4.3.3.RELEASE" | "/path"                  | false
+    [requestParam(false, "", "")]                           | "4.3.3.RELEASE" | "/path"                  | false
+    [requestBody(false)]                                    | "4.3.3.RELEASE" | "/path"                  | false
+    [requestBody(true)]                                     | "4.3.3.RELEASE" | "/path"                  | true
+    [requestPart(false, "")]                                | "4.3.3.RELEASE" | "/path"                  | false
+    [requestPart(true, "")]                                 | "4.3.3.RELEASE" | "/path"                  | true
+    []                                                      | "4.3.3.RELEASE" | "/path"                  | false
+    [null]                                                  | "4.3.3.RELEASE" | "/path"                  | false
+    [apiParam(true), requestParam(false, "", DEFAULT_NONE)] | "4.3.3.RELEASE" | "/path"                  | false
+    [apiParam(false), requestParam(true, "", DEFAULT_NONE)] | "4.3.3.RELEASE" | "/path"                  | true
+    [apiParam(false), requestParam(true, "", DEFAULT_NONE)] | "4.3.3.RELEASE" | "/path"                  | true
   }
 
   def pathVariableRequired() {

springfox-swagger-common/src/main/java/springfox/documentation/swagger/common/XForwardPrefixPathAdjuster.java

@@ -18,27 +18,33 @@
  */
 package springfox.documentation.swagger.common;
 
-import org.springframework.core.SpringVersion;
+import springfox.documentation.common.SpringVersion;
 import springfox.documentation.service.PathAdjuster;
 
 import javax.servlet.http.HttpServletRequest;
 
-import static springfox.documentation.swagger.common.SpringVersionCapability.*;
+import static springfox.documentation.common.SpringVersionCapability.*;
 
 public class XForwardPrefixPathAdjuster implements PathAdjuster {
   static final String X_FORWARDED_PREFIX = "X-Forwarded-Prefix";
 
   private final HttpServletRequest request;
+  private final SpringVersion springVersion;
 
   public XForwardPrefixPathAdjuster(HttpServletRequest request) {
+    this(request, new SpringVersion());
+  }
+
+  XForwardPrefixPathAdjuster(HttpServletRequest request, SpringVersion springVersion) {
     this.request = request;
+    this.springVersion = springVersion;
   }
 
   @Override
   public String adjustedPath(String path) {
     String prefix = request.getHeader(X_FORWARDED_PREFIX);
     if (prefix != null) {
-      if (!supportsXForwardPrefixHeader(SpringVersion.getVersion())) {
+      if (!supportsXForwardPrefixHeader(springVersion.getVersion())) {
         return prefix + path;
       } else {
         return prefix;