Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions apps/faces-example2/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,20 @@
<version>3.2.2</version>
</plugin>

<!-- Spring Boot Maven Plugin for executable JAR/WAR packaging -->
<plugin>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-maven-plugin</artifactId>
<version>3.2.0</version>
<executions>
<execution>
<goals>
<goal>repackage</goal>
</goals>
</execution>
</executions>
</plugin>

<!-- JaCoCo for code coverage -->
<plugin>
<groupId>org.jacoco</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,25 @@
* specific language governing permissions and limitations
* under the License.
*/

package org.apache.struts.webapp.example2;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;

/**
* Spring Boot main application class for the Struts Faces Example 2 migration.
* This class serves as the entry point for the Spring Boot application,
* replacing the traditional web.xml and servlet configuration.
*
* The @SpringBootApplication annotation enables:
* - @Configuration: Tags the class as a source of bean definitions
* - @EnableAutoConfiguration: Enables Spring Boot's auto-configuration
* - @ComponentScan: Enables component scanning in the current package and sub-packages
*/
@SpringBootApplication
public class TestApplication {
public class Example2Application {

public static void main(String[] args) {
SpringApplication.run(Example2Application.class, args);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,136 @@
*/
package org.apache.struts.webapp.example2.config;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.util.List;

import jakarta.annotation.PreDestroy;

import org.apache.struts.webapp.example2.UserDatabase;
import org.apache.struts.webapp.example2.memory.MemoryUserDatabase;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.io.Resource;

/**
* Spring configuration class that replaces the MemoryDatabasePlugIn from Struts.
* This class initializes the in-memory user database from an XML file and provides
* it as a Spring bean for dependency injection.
*/
@Configuration
public class DatabaseConfiguration {

private static final Logger log = LoggerFactory.getLogger(DatabaseConfiguration.class);

@Value("${app.database.path:classpath:database.xml}")
private Resource databaseResource;

private MemoryUserDatabase database;

/**
* Creates and initializes the UserDatabase bean.
* The database is loaded from the configured XML file path.
*
* @return The initialized UserDatabase instance
* @throws Exception if the database cannot be loaded
*/
@Bean
public UserDatabase userDatabase() throws Exception {
Comment on lines +60 to +61
Copy link
Author

Choose a reason for hiding this comment

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

🟡 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:

  1. Spring's inferred destroy method calls close() on the userDatabase bean
  2. @PreDestroy cleanup() on DatabaseConfiguration calls database.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().

Suggested change
@Bean
public UserDatabase userDatabase() throws Exception {
@Bean(destroyMethod = "")
public UserDatabase userDatabase() throws Exception {
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

log.info("Initializing memory database from '{}'", databaseResource);

database = new MemoryUserDatabase();

try {
if (databaseResource.exists() && databaseResource.isFile()) {
File file = databaseResource.getFile();
database.setPathname(file.getAbsolutePath());
database.open();
} else if (databaseResource.exists()) {
File tempFile = createTempDatabaseFile();
database.setPathname(tempFile.getAbsolutePath());
try (InputStream is = databaseResource.getInputStream()) {
copyInputStreamToFile(is, tempFile);
}
database.open();
} else {
log.warn("Database resource '{}' does not exist, creating empty database", databaseResource);
File tempFile = createTempDatabaseFile();
database.setPathname(tempFile.getAbsolutePath());
createEmptyDatabaseFile(tempFile);
database.open();
}
} catch (Exception e) {
log.error("Failed to initialize database from '{}': {}", databaseResource, e.getMessage());
throw e;
}

log.info("Memory database initialized successfully");
return database;
}

/**
* Creates a temporary file for the database when running from a JAR.
* This allows the database to be saved during runtime.
*
* @return The temporary file
* @throws IOException if the file cannot be created
*/
private File createTempDatabaseFile() throws IOException {
File tempDir = Files.createTempDirectory("struts-example2").toFile();
tempDir.deleteOnExit();
File tempFile = new File(tempDir, "database.xml");
tempFile.deleteOnExit();
return tempFile;
}

/**
* Copies an input stream to a file.
*
* @param is The input stream
* @param file The target file
* @throws IOException if copying fails
*/
private void copyInputStreamToFile(InputStream is, File file) throws IOException {
Files.copy(is, file.toPath());
}

/**
* Creates a database XML file with sample data.
* This provides initial test data for the application.
*
* @param file The file to create
* @throws IOException if file creation fails
*/
private void createEmptyDatabaseFile(File file) throws IOException {
String sampleDb = """
<?xml version='1.0'?>
<database>
<user username="user"
password="pass"
fullName="John Q. User"
fromAddress="John.User@somewhere.com">
<subscription host="mail.yahoo.com" type="imap"
username="jquser" password="foo"/>
<subscription host="mail.hotmail.com" type="pop3"
username="user1234" password="bar"/>
</user>
</database>
""";
Files.writeString(file.toPath(), sampleDb);
}

/**
* Provides a list of server types for subscription forms.
* This replaces the setupCache method from MemoryDatabasePlugIn.
*
* @return List of server type options
*/
@Bean
public List<LabelValueBean> serverTypes() {
return List.of(
Expand All @@ -34,6 +156,26 @@ public List<LabelValueBean> serverTypes() {
);
}

/**
* Cleanup method called when the application shuts down.
* Saves and closes the database.
*/
@PreDestroy
public void cleanup() {
log.info("Finalizing memory database");
if (database != null) {
try {
database.close();
} catch (Exception e) {
log.error("Error closing memory database: {}", e.getMessage());
}
}
}

/**
* Simple bean to hold label-value pairs for dropdown options.
* This replaces the Struts LabelValueBean.
*/
public static class LabelValueBean {
private final String label;
private final String value;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts.webapp.example2.controller;

import jakarta.servlet.http.HttpSession;

import org.apache.struts.webapp.example2.Constants;
import org.apache.struts.webapp.example2.User;
import org.springframework.stereotype.Controller;
import org.springframework.ui.Model;
import org.springframework.web.bind.annotation.GetMapping;

@Controller
public class WelcomeController {

@GetMapping({"/", "/welcome"})
public String welcome(HttpSession session, Model model) {
User user = (User) session.getAttribute(Constants.USER_KEY);
if (user != null) {
model.addAttribute("user", user);
}
return "welcome";
}

@GetMapping("/mainMenu")
public String mainMenu(HttpSession session, Model model) {
User user = (User) session.getAttribute(Constants.USER_KEY);
if (user == null) {
return "redirect:/editLogon";
}
model.addAttribute("user", user);
return "mainMenu";
}
}
8 changes: 8 additions & 0 deletions apps/faces-example2/src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,11 @@ spring.thymeleaf.suffix=.html
# Message source configuration
spring.messages.basename=messages
spring.messages.encoding=UTF-8

# Database configuration
# Uses the existing database.xml from webapp/WEB-INF for sample data
app.database.path=classpath:database.xml
Copy link
Author

Choose a reason for hiding this comment

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

🟡 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.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


# Logging
logging.level.org.apache.struts.webapp.example2=DEBUG
logging.level.org.springframework.web=INFO