refactor todo-java template and fix bugs.

[wangmingliang-ms]

This PR refactored code and fixed some issues (including those in comments of PR #799 and several others) of SimpleTodo templates for Java AppService and Container Apps.

[jongio]

@v-xuto - Can you please run a test pass on all java templates? Thanks!

I will generate the templates now...

[jongio]

/azp run azure-dev - repoman

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[weikanglim]

Started running template tests validation: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=1980514&view=results

FYI @jongio

[weikanglim]

@jdubois Could use you or your team's insights on what might be idiomatic Java or otherwise based on your experiences. Otherwise, code changes looks fine pending validation.

[brunoborges]

We should strongly consider the removal of the Lombok dependency.

IDEs don't support this out of the box and developers would have to install extra plugins/extensions to be able to work on this code.

Please let's keep the template simple and with minimal dependencies.

[wangmingliang-ms]

@brunoborges you are right, Lombok is removed.

[v-xuto]

@v-xuto - Can you please run a test pass on all java templates? Thanks!

I will generate the templates now...

@jongio We have tested the java templates (todo-java-mongo and todo-java-mongo-aca) only in Windows Desktop, both of them test results are PASS.

[brunoborges]

Can you make sure they also work fine on Linux? You may use WSL

[v-xuto]

Can you make sure they also work fine on Linux? You may use WSL

We have tested the java templates in WSL, the results are PASS.

[weikanglim]

/azp run azure-dev - repoman

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[weikanglim]

Re-running repoman after latest repoman CI changes

[weikanglim]

/azp run azure-dev - repoman

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[azure-sdk]

Repoman Generation Results

Repoman pushed changes to remotes for the following projects:

Project: todo-csharp-cosmos-sql

Remote: azure-samples-staging

Branch: pr/1122

You can initialize this project with:

azd init -t Azure-Samples/todo-csharp-cosmos-sql -b pr/1122

View Changes | Compare Changes

---

Project: todo-csharp-sql-swa-func

Remote: azure-samples-staging

Branch: pr/1122

You can initialize this project with:

azd init -t Azure-Samples/todo-csharp-sql-swa-func -b pr/1122

View Changes | Compare Changes

---

Project: todo-csharp-sql

Remote: azure-samples-staging

Branch: pr/1122

You can initialize this project with:

azd init -t Azure-Samples/todo-csharp-sql -b pr/1122

View Changes | Compare Changes

---

Project: todo-java-mongo-aca

Remote: azure-samples-staging

Branch: pr/1122

You can initialize this project with:

azd init -t Azure-Samples/todo-java-mongo-aca -b pr/1122

View Changes | Compare Changes

---

Project: todo-java-mongo

Remote: azure-samples-staging

Branch: pr/1122

You can initialize this project with:

azd init -t Azure-Samples/todo-java-mongo -b pr/1122

View Changes | Compare Changes

---

Project: todo-nodejs-mongo-aca

Remote: azure-samples-staging

Branch: pr/1122

You can initialize this project with:

azd init -t Azure-Samples/todo-nodejs-mongo-aca -b pr/1122

View Changes | Compare Changes

---

Project: todo-nodejs-mongo-swa-func

Remote: azure-samples-staging

Branch: pr/1122

You can initialize this project with:

azd init -t Azure-Samples/todo-nodejs-mongo-swa-func -b pr/1122

View Changes | Compare Changes

---

Project: todo-nodejs-mongo

Remote: azure-samples-staging

Branch: pr/1122

You can initialize this project with:

azd init -t Azure-Samples/todo-nodejs-mongo -b pr/1122

View Changes | Compare Changes

---

Project: todo-nodejs-mongo-terraform

Remote: azure-samples-staging

Branch: pr/1122

You can initialize this project with:

azd init -t Azure-Samples/todo-nodejs-mongo-terraform -b pr/1122

View Changes | Compare Changes

---

Project: todo-python-mongo-aca

Remote: azure-samples-staging

Branch: pr/1122

You can initialize this project with:

azd init -t Azure-Samples/todo-python-mongo-aca -b pr/1122

View Changes | Compare Changes

---

Project: todo-python-mongo-swa-func

Remote: azure-samples-staging

Branch: pr/1122

You can initialize this project with:

azd init -t Azure-Samples/todo-python-mongo-swa-func -b pr/1122

View Changes | Compare Changes

---

Project: todo-python-mongo

Remote: azure-samples-staging

Branch: pr/1122

You can initialize this project with:

azd init -t Azure-Samples/todo-python-mongo -b pr/1122

View Changes | Compare Changes

---

Project: todo-python-mongo-terraform

Remote: azure-samples-staging

Branch: pr/1122

You can initialize this project with:

azd init -t Azure-Samples/todo-python-mongo-terraform -b pr/1122

View Changes | Compare Changes

---

[wangmingliang-ms]

@weikanglim @jongio is there anything I should fix?

[weikanglim]

@wangmingliang-ms No, the test failures are unrelated to your changes. We were in the middle of a release. Overall, the changes look good to me, and I'll merge it at the next appropriate window.

[weikanglim]

@wangmingliang-ms Can you verify what additional changes we might need so that running ./mvnw -P openapigen compile doesn't result in a huge diff in the java files?

[wangmingliang-ms]

@wangmingliang-ms Can you verify what additional changes we might need so that running ./mvnw -P openapigen compile doesn't result in a huge diff in the java files?

The huge diff you mentioned should be mainly caused by 2 reasons:

1. Did not use useTags=true when generating code, resulting in only one ListsApi generated, instead of the separate ItemsApi and ListsApi we expected, I submitted another commit to fix this problem see 7647439.
2. Code style: the generated code uses 2 spaces indention instead of the 4 spaces commonly used in the Java world. In addition, the generated code also contains some completely unused imports, see below screenshot. I didn't find proper settings to fix these 2 problems of OpenApi generator
   right part is the code generated by OpenApi generator.
   

[wangmingliang-ms]

The code style issue can be solved by some maven plugins e.g. io.spring.javaformat:spring-javaformat-maven-plugin. see commit ce21ce4
but none of them can organize the imports.

[jdubois]

For formatting, please have a look at https://github.com/jhipster/prettier-java (it’s under JHipster, to be transparent).
You’ll have perfect formatting and it works with IDEs and CI easily as it’s Prettier.

[weikanglim]

@wangmingliang-ms The formatting from javaformat-maven ended up not being very readable:

azure-dev/templates/todo/api/java/src/main/java/com/microsoft/azure/simpletodo/api/ItemsApi.java

Lines 45 to 60 in c6fdca1

	@Operation(operationId = "createItem", summary = "Creates a new Todo item within a list", tags = { "Items" },
	responses = {
	@ApiResponse(responseCode = "201", description = "A Todo item result",
	content = { @Content(mediaType = "application/json",
	schema = @Schema(implementation = TodoItem.class)) }),
	@ApiResponse(responseCode = "404", description = "Todo list not found") })
	@RequestMapping(method = RequestMethod.POST, value = "/lists/{listId}/items", produces = { "application/json" },
	consumes = { "application/json" })
	default ResponseEntity<TodoItem> createItem(
	@Parameter(name = "listId", description = "The Todo list unique identifier",
	required = true) @PathVariable("listId") String listId,
	@Parameter(name = "TodoItem",
	description = "The Todo Item") @Valid @RequestBody(required = false) TodoItem todoItem) {
	getRequest().ifPresent(request -> {
	for (MediaType mediaType : MediaType.parseMediaTypes(request.getHeader("Accept"))) {
	if (mediaType.isCompatibleWith(MediaType.valueOf("application/json"))) {

So I opted for using prettier which has a much better result:

azure-dev/templates/todo/api/java/src/main/java/com/microsoft/azure/simpletodo/api/ItemsApi.java

Lines 50 to 65 in 9966e02

	@Operation(
	operationId = "createItem",
	summary = "Creates a new Todo item within a list",
	tags = { "Items" },
	responses = {
	@ApiResponse(
	responseCode = "201",
	description = "A Todo item result",
	content = { @Content(mediaType = "application/json", schema = @Schema(implementation = TodoItem.class)) }
	),
	@ApiResponse(responseCode = "404", description = "Todo list not found"),
	}
	)
	@RequestMapping(
	method = RequestMethod.POST,
	value = "/lists/{listId}/items",

After these changes, I can see the minimal codegen diff of only the intended, modified changes to our model classes, so I think we're in a good spot.

[weikanglim]

@jdubois @wangmingliang-ms One final thing to check about prettier, the import order seems to not prefer import sections, and instead always alphabetical:

azure-dev/templates/todo/api/java/src/main/java/com/microsoft/azure/simpletodo/api/ItemsApi.java

Lines 8 to 30 in 9966e02

	import com.microsoft.azure.simpletodo.model.TodoItem;
	import com.microsoft.azure.simpletodo.model.TodoState;
	import io.swagger.v3.oas.annotations.Operation;
	import io.swagger.v3.oas.annotations.Parameter;
	import io.swagger.v3.oas.annotations.Parameters;
	import io.swagger.v3.oas.annotations.media.Content;
	import io.swagger.v3.oas.annotations.media.Schema;
	import io.swagger.v3.oas.annotations.responses.ApiResponse;
	import io.swagger.v3.oas.annotations.security.SecurityRequirement;
	import io.swagger.v3.oas.annotations.tags.Tag;
	import java.math.BigDecimal;
	import java.util.List;
	import java.util.List;
	import java.util.Map;
	import java.util.Optional;
	import javax.annotation.Generated;
	import javax.validation.Valid;
	import javax.validation.constraints.*;
	import org.springframework.http.HttpStatus;
	import org.springframework.http.MediaType;
	import org.springframework.http.ResponseEntity;
	import org.springframework.validation.annotation.Validated;
	import org.springframework.web.bind.annotation.*;

Which seems like it's a hardcoded and opinionated import format according to their readme. I don't have a concern here as long as we have a consistent format. But do you feel strongly otherwise?

I just wanted to get to a point where the user using the sample could understand the openapigen diff easily, so if they wanted to use the template as a starting point for their application, it would make total sense. I think that's accomplished here but let me know otherwise. Would love to get this merged sooner than later.

[wangmingliang-ms]

I think it's OK, no style can satisfy all users, sections or not does not affect readability actually. What's more, google code style is almost the most popular.