search

dice-group / ida-pg

GNU Affero General Public License v3.0
6 stars 4 forks source link

Fixed spring-boot:run #63

Closed Cortys closed 6 years ago

Cortys commented 6 years ago

The spring boot maven plugin does not fully support a web.xml based configuration. I migrated it to an equivalent programmatic config.

This makes it possible to use mvn spring-boot:run. Additionally spring-boot-devtools extends that mvn goal with auto-reload on file change, simplifying development outside of IDEs.

RicardoUsbeck commented 6 years ago

You uploaded classpath and laptop specific files. Remove them.

Cortys commented 6 years ago

Those were already checked in on master before. Wasn't sure about your policy on that so I kept them in. Will fix in a sec...

nikit-srivastava commented 6 years ago

Hi @Cortys ,

We were looking forward to implement these changes as part of the list of tasks assigned to team members. It's good that you did it on your own. However, there are a few things that I would like to point out in your changes:

1. Deletion of xml context files

We kept the xml files as a future use case where we would be able to change the context (bean implementations) of the application without having to recompile or changing the java code itself.

2. Using spring boot plugin

As I explained in the previous point, we would like to keep the xml files (web and spring contexts). Hence, I would rather suggest the use of maven tomcat plugin to start the instance.

Let me know if you have any further questions

Cortys commented 6 years ago

Hi, thanks for the feedback!

I only went ahead and did this because I prefer not to use fullblown IDEs like Eclipse and that was a bit cumbersome before.

Would using XML for the beans but not using the web.xml for the servlet description be ok then? That should address your concerns while keeping spring-boot:run working.

nikit-srivastava commented 6 years ago

Hi, I see there are still a lot of settings files that are being touched. I do not totally understand why the changes to facets are made? Also, one of our future use case could involve using a database. In that case we would like to have an option where we could switch between different databases by having config in separate xml files.

A web.xml allows us to change that without having to recompile the code. But, in the implementation you are proposing, we will have to change that into a the java code itself and recompile.

Also, having a web xml makes it easier for us to opt for another application server such as weblogic.

Is there any way we could keep the current design intact and only have something like: mvn tomcat7:deploy

Wouldn't that solve your problem of being too reliant on Eclipse?

RicardoUsbeck commented 6 years ago

Let's discuss those changes tomorrow in detail as I also have some wishes here :)

nikit-srivastava commented 6 years ago

@Cortys I tried running the rest service from your branch using mvn spring-boot:run. The server starts as it should but our UI is not able to send requests to rest service anymore because of the CORS issue. This happened before as well but, after we put @CrossOrigin(origins = "*") on our rest controller it accepted queries from UI.

nikit-srivastava commented 6 years ago

Perfect 👍 I've completed the integration testing and everything works like it should. I will merge it once we have a "go ahead" from @RicardoUsbeck

Cortys commented 6 years ago

@nikit91 :+1: The issue was that the Access-Control-Allow-Credentials header is not set in Spring Boot 2 by default anymore (see https://github.com/spring-projects/spring-boot/issues/12488). Should have looked a bit closer at the breaking changes...