Complete Spring Boot Migration for faces-example2#9
Complete Spring Boot Migration for faces-example2#9devin-ai-integration[bot] wants to merge 2 commits intotrunkfrom
Conversation
- Add Example2Application.java as the main Spring Boot application class - Update DatabaseConfiguration to include UserDatabase bean with MemoryUserDatabase - Add spring-boot-maven-plugin for executable JAR/WAR packaging - Update application.properties with database configuration - DatabaseConfiguration creates sample data if database.xml not found on classpath - Remove duplicate TestApplication.java (now using Example2Application) The application can now be run with: mvn spring-boot:run Co-Authored-By: Jack Meigel <jack.meigel@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
- Add WelcomeController to handle / and /welcome routes - Add mainMenu route for logged-in users - Completes the Spring Boot migration with proper landing pages Co-Authored-By: Jack Meigel <jack.meigel@cognition.ai>
|
|
||
| # Database configuration | ||
| # Uses the existing database.xml from webapp/WEB-INF for sample data | ||
| app.database.path=classpath:database.xml |
There was a problem hiding this comment.
🟡 Default classpath:database.xml resource does not exist, so configured database path never resolves
The application.properties configures app.database.path=classpath:database.xml, which is injected as a Spring Resource into DatabaseConfiguration.databaseResource. However, there is no database.xml file in src/main/resources/ — the only database.xml is located at src/main/webapp/WEB-INF/database.xml, which is NOT on the classpath.
Root Cause and Impact
Because the resource doesn't exist on the classpath, databaseResource.exists() at DatabaseConfiguration.java:67 always returns false. This means the code always falls through to the else branch at line 78, which logs a warning and creates a hardcoded sample database via createEmptyDatabaseFile().
The comment in application.properties:16 — "Uses the existing database.xml from webapp/WEB-INF for sample data" — is misleading since that file is never actually loaded.
Impact: The first two branches (lines 67-77) of the userDatabase() method are dead code — they can never be reached with the default configuration. Any real database.xml customizations placed in WEB-INF/ will be silently ignored, and users will always get the hardcoded sample data instead of what they expect from the configured path.
Prompt for agents
Copy the database.xml file from apps/faces-example2/src/main/webapp/WEB-INF/database.xml to apps/faces-example2/src/main/resources/database.xml so that the classpath:database.xml resource actually resolves. This ensures the configured app.database.path property works as documented. Alternatively, change the property value in apps/faces-example2/src/main/resources/application.properties line 17 to point to a valid resource location.
Was this helpful? React with 👍 or 👎 to provide feedback.
| @Bean | ||
| public UserDatabase userDatabase() throws Exception { |
There was a problem hiding this comment.
🟡 Database close()/save() called twice on shutdown due to @PreDestroy plus Spring's inferred destroy method
The @PreDestroy cleanup() method at DatabaseConfiguration.java:164 explicitly calls database.close(). However, Spring's @Bean annotation defaults to destroyMethod = "(inferred)", which auto-detects and invokes public no-arg close() or shutdown() methods on returned beans. Since UserDatabase declares public void close() throws Exception (UserDatabase.java:61), Spring will also automatically call close() on the userDatabase bean during shutdown.
Detailed Explanation
This results in MemoryUserDatabase.close() being called twice during application shutdown:
- Spring's inferred destroy method calls
close()on theuserDatabasebean @PreDestroy cleanup()onDatabaseConfigurationcallsdatabase.close()on the same instance
Each close() invocation triggers save() (MemoryUserDatabase.java:106-110), which performs a full file-write-and-rename dance to disk. While the second save() doesn't crash (it's effectively idempotent), it performs unnecessary file I/O during shutdown.
Fix: Either remove the @PreDestroy cleanup() method entirely (relying on Spring's inferred destroy), or annotate the @Bean with an explicit destroyMethod to prevent double invocation, e.g. @Bean(destroyMethod = "close") and remove cleanup(), or @Bean(destroyMethod = "") to disable inference and keep cleanup().
| @Bean | |
| public UserDatabase userDatabase() throws Exception { | |
| @Bean(destroyMethod = "") | |
| public UserDatabase userDatabase() throws Exception { | |
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
This PR completes the Spring Boot migration for
apps/faces-example2/by adding the missing components needed to run the application as a standalone Spring Boot app.Changes:
Example2Application.javaas the main Spring Boot entry point (renamed from test'sTestApplication.java)DatabaseConfigurationto provide aUserDatabasebean usingMemoryUserDatabase, replacing the legacy StrutsMemoryDatabasePlugInspring-boot-maven-pluginfor executable JAR/WAR packagingapplication.propertieswith database path and logging configurationWelcomeControllerto handle root (/),/welcome, and/mainMenuroutesThe
DatabaseConfigurationhandles three scenarios for database initialization:Updates Since Last Revision
WelcomeController.javato provide landing page routes that were missing, enabling the application to serve the welcome page at the root URLReview & Testing Checklist for Human
JAVA_HOME=/usr/lib/jvm/java-17-openjdk-amd64 mvn spring-boot:runand confirm the app starts without errorsdeleteOnExit()approach increateTempDatabaseFile()may not clean up in all scenarios and data won't persist across restartsRecommended test plan:
Demo recording of end-to-end testing:
View original video (rec-45274a05ba684759a03251e1bb8b7bfd-edited.mp4)
Notes
The sample credentials in
createEmptyDatabaseFile()are intentionally hardcoded demo data (matching the originalwebapp/WEB-INF/database.xml). This is expected for a demo application.All 80 existing tests pass with these changes.
Link to Devin run: https://jack-meigel.devinenterprise.com/sessions/ed9261f012204fac84a73039ee15f61d
Requested by: Jack Meigel (@cogjack)