Skip to content

#369 - Spring Boot Support#589

Draft
kramano wants to merge 6 commits intoagrestio:masterfrom
kramano:#369
Draft

#369 - Spring Boot Support#589
kramano wants to merge 6 commits intoagrestio:masterfrom
kramano:#369

Conversation

@kramano
Copy link
Copy Markdown

@kramano kramano commented Nov 4, 2022

Add support for Spring Controllers.

  • find analogues for JAX-RS concepts in Spring Web and implement corresponding interfaces
  • port JAX-RS integration tests to ensure feature parity

Copy link
Copy Markdown
Contributor

@andrus andrus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! My comments inside individual files are all cosmetic in nature. Let me add a few thoughts here:

  • Can we combine the integration tests module with the main SB module? We do it for all the other Agrest modules (via maven-failsafe-plugin). So all the existing classes with *IT extension are integration tests. Is it doable with Spring Boot?
  • We take some shortcuts with JAX-RS tests, because JAX-RS is used as an engine to test agrest-cayenne. For SpringBoot I would like to have a bit more tests:
    • Let's add tests that are performing updates, and take EntityUpdate parameters. The docs are incomplete on this, but you will see some examples here, and of course in all the Cayenne tests that start with PUT and POST
    • Let's add the tests showing the use of AgRuntimeCustomizer

@@ -0,0 +1 @@
org.springframework.boot.autoconfigure.EnableAutoConfiguration=io/agrest.spring.starter.AgrestAutoConfiguration
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a Spring expert. So this is more of a question. Is it the correct syntax - io/agrest.spring.starter.... (the first separator in the class name is a slash, the rest are dots)?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, good catch. thanks.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can create a test for this condition

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep,I'll try. The problem here is that starter works as it is. But that syntax seems confusing for me too.

@kramano
Copy link
Copy Markdown
Author

kramano commented Nov 10, 2022

Regarding integration tests - I've tried to do this in single module first, but it didn't work well. I think it's doable but it looks like the way it's done now is clearer in terms of configurations - now it's just a simple Spring Boot project with controllers and tests. Putting controllers in tests creates some hassle. I'll have another look here.

Regarding more tests - yes, agreed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants