[Java][Spring] Add java8, interfaceOnly, delegatePattern to generated server stubs

[david-noel]

PR checklist

• Read the contribution guidelines.
• Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
• Filed the PR against the correct branch: master for non-breaking changes and 3.0.0 branch for breaking (non-backward compatible) changes.

Description of the PR

Add support for java8 interfaces with default methods that delegate to non-default methods

The default methods have the SpringMVC annotations while the non-default methods have no annotations.

This feature is desired so that the RestController implementer does not need to duplicate method level(PathVariable, etc) annotations on the controller class. This also provides some benefits over the class delegation code generation. First it is easier to control how the RestControllers are added to the Spring context. Second, if needed, the implementer has more control in overriding the generated code.

I ran the sample scripts under bin but was unsure if I should add the updated sample files back into the repo under this pull request. There seem to be no interfaceOnly samples in the repo, so I did not add a new one for this pull request. Also, the existing samples do change, but they are whitespace/comment changes only. I could make the samples not change at all, but it would make the mustache template file less pleasant to read.

To close #6471

[wing328]

@david-noel please run spring-related petstore scripts to update petstore samples so that CIs can verify the change:

./bin/spring-all-pestore.sh

cc @bbdouglas @JFCote @sreeshas @jfiala @lukoyanov @cbornet @ePaul

[cbornet]

I don't understand the use for this : default methods look more powerful to me !

[david-noel]

@cbornet SpringMVC annotations placed on method parameters(ex. PathVariable) are not carried over when a RestController class overrides a method from the generated interface. It does not matter if the overridden method is a default method or a regular interface method. This forces the RestController developer to manually duplicate the annotations from the generated interface code onto their RestController. This can be error prone. The code in this PR removes this burden from the developer. At the same time, the developer can still override the default method if needed.

[cbornet]

@david-noel I see now what you're trying to do ! Then you should make the delegate-method also a default one so that the user is not forced to implement it.
One problem I have with this approach is that you now have two methods with about the same name and it's difficult for the implementer to know which one should be overridden.

[david-noel]

@cbornet I can make both methods be default. I'll have the implementation of the default method return an OK status like the interfaceOnly code generation(feels like these should throw an UnSupportedOperationException, but that is another issue). As for the potential method name confusion, I can add a "do" prefix to the method the user should override. This is a common prefix used by Spring so developers should be familiar with it.

How does this look? I'll commit it to the PR if you like it.

@Api(value = "fake_classname_test", description = "the fake_classname_test API")
public interface FakeClassnameTestApi {

    @ApiOperation(value = "To test class name in snake case", notes = "", response = Client.class, authorizations = {
        @Authorization(value = "api_key_query")
    }, tags={ "fake_classname_tags 123#$%^", })
    @ApiResponses(value = { 
        @ApiResponse(code = 200, message = "successful operation", response = Client.class) })
    @RequestMapping(value = "/fake_classname_test",
        produces = { "application/json" }, 
        consumes = { "application/json" },
        method = RequestMethod.PATCH)
    default ResponseEntity<Client> testClassname(@ApiParam(value = "client model" ,required=true )  @Valid @RequestBody Client body, @RequestHeader(value = "Accept", required = false) String accept) throws Exception {
        
        return doTestClassname(body);
    }

    // override this method
    default ResponseEntity<Client> doTestClassname(Client body) {
        return new ResponseEntity<Client>(HttpStatus.OK);
    }
}

[david-noel]

I'm not sure why the travis-ci build is failing. It looks like it might be environmental? Is there anything I need to do?

[cbornet]

I would use the operationId untouched for the method to override and prefix the one with annotations with _ (making it privatish by other languages conventions. In Java 9 we could probably make it private).

[wing328]

I'm not sure why the travis-ci build is failing. It looks like it might be environmental? Is there anything I need to do?

I'll restart it later (Java-related tests are covered by CircleCI)

[david-noel]

@cbornet updated. The code generated will now look like this.

public interface FakeClassnameTestApi {

    @ApiOperation(value = "To test class name in snake case", notes = "", response = Client.class, authorizations = {
        @Authorization(value = "api_key_query")
    }, tags={ "fake_classname_tags 123#$%^", })
    @ApiResponses(value = {
        @ApiResponse(code = 200, message = "successful operation", response = Client.class) })
    @RequestMapping(value = "/fake_classname_test",
        produces = { "application/json" },
        consumes = { "application/json" },
        method = RequestMethod.PATCH)
    default ResponseEntity<Client> _testClassname(@ApiParam(value = "client model" ,required=true )  @Valid @RequestBody Client body, @RequestHeader(value = "Accept", required = false) String accept) throws Exception {

        return testClassname(body);
    }

    // override this method
    default ResponseEntity<Client> testClassname(Client body) {
        return new ResponseEntity<Client>(HttpStatus.OK);
    }
}```

[wing328]

I'll merge this on coming weekend if no one has further feedback on this PR.

cc @bbdouglas @JFCote @sreeshas @jfiala @lukoyanov @cbornet

[wing328]

@david-noel when you've time, I wonder if you can resolve the merge conflicts.

[david-noel]

@wing328 resolved the conflicts and re ran the bin/spring-all-petstore.sh script.

Generated code now looks like this.

@Api(value = "fake", description = "the fake API")
public interface FakeApi {

    Logger log = LoggerFactory.getLogger(FakeApi.class);

    default Optional<ObjectMapper> getObjectMapper() {
        return Optional.empty();
    }

    default Optional<HttpServletRequest> getRequest() {
        return Optional.empty();
    }

    default Optional<String> getAcceptHeader() {
        return getRequest().map(r -> r.getHeader("Accept"));
    }

    @ApiOperation(value = "", nickname = "fakeOuterBooleanSerialize", notes = "Test serialization of outer boolean types", response = Boolean.class, tags={ "fake", })
    @ApiResponses(value = {
        @ApiResponse(code = 200, message = "Output boolean", response = Boolean.class) })
    @RequestMapping(value = "/fake/outer/boolean",
        method = RequestMethod.POST)
    default ResponseEntity<Boolean> _fakeOuterBooleanSerialize(@ApiParam(value = "Input boolean as post body"  )  @Valid @RequestBody Boolean body) {
        return fakeOuterBooleanSerialize(body);
    }

    // Override this method
    default ResponseEntity<Boolean> fakeOuterBooleanSerialize(Boolean body) {
        if(getObjectMapper().isPresent() && getAcceptHeader().isPresent()) {
            if (getAcceptHeader().get().contains("application/json")) {
                try {
                    return new ResponseEntity<>(getObjectMapper().get().readValue("{ }", Boolean.class), HttpStatus.NOT_IMPLEMENTED);
                } catch (IOException e) {
                    log.error("Couldn't serialize response for content type application/json", e);
                    return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
                }
            }
        } else {
            log.warn("ObjectMapper or HttpServletRequest not configured in default FakeApi interface so no example is generated");
        }
        return new ResponseEntity<>(HttpStatus.NOT_IMPLEMENTED);
    }

[cbornet]

LGTM

[mehdijouan]

That PR is exactly the option I was searching for ! 👍

[wing328]

@mehdijouan please help test it:

git checkout -b david-noel-master master
git pull https://github.com/david-noel/swagger-codegen.git master
mvn clean install

I'll take another look over the weekend as well.

[mehdijouan]

I tested it locally and it worked well!
I can simply override the interface without having to copy/paste annotations

[ollio]

This is a very good solution! I had started to Inject a delegate with default methods whish I can override.

This solution should do the trick in a better way.

In what version it will be available?