Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve main #6053

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
157 changes: 157 additions & 0 deletions Kitodo/src/main/java/org/kitodo/production/KitodoProduction.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
/*
* (c) Kitodo. Key to digital objects e. V. <[email protected]>
*
* This file is part of the Kitodo project.
*
* It is licensed under GNU General Public License version 3 or later.
*
* For the full copyright and license information, please read the
* GPL3-License.txt file that was distributed with this source code.
*/

package org.kitodo.production;

import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.UndeclaredThrowableException;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.jar.Manifest;

import javax.servlet.ServletContext;
import javax.servlet.ServletContextEvent;
import javax.servlet.ServletContextListener;
import javax.servlet.annotation.WebListener;
import javax.servlet.http.HttpSessionEvent;
import javax.servlet.http.HttpSessionListener;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.kitodo.config.ConfigCore;
import org.kitodo.config.enums.ParameterCore;
import org.kitodo.production.helper.tasks.TaskManager;
import org.kitodo.production.interfaces.activemq.ActiveMQDirector;
import org.kitodo.production.security.SecurityUserDetails;
import org.kitodo.production.services.ServiceManager;
import org.springframework.security.core.context.SecurityContextImpl;

/**
* Kitodo.Production is the workflow management module of the Kitodo suite. It
* supports the digitization process of various types of materials.
*/
@WebListener
public class KitodoProduction implements ServletContextListener, HttpSessionListener {
private static final Logger logger = LogManager.getLogger(KitodoProduction.class);
private static CompletableFuture<KitodoProduction> instance = new CompletableFuture<>();
private ServletContext context;
private Optional<Manifest> manifest;
private KitodoVersion version = new KitodoVersion();
matthias-ronge marked this conversation as resolved.
Show resolved Hide resolved
private ActiveMQDirector activeMQDirector;

/* This is the main() function. The class is instantiated by the servlet
* container, which then calls this function. */
@Override
public void contextInitialized(ServletContextEvent event) {
context = event.getServletContext();
manifest = retrieveManifestFileAsStream(context);
manifest.ifPresent(version::setupFromManifest);
instance.complete(this);
startActiveMQ();
}

private static final Optional<Manifest> retrieveManifestFileAsStream(ServletContext context) {
Copy link
Member

@solth solth Jul 9, 2024

Choose a reason for hiding this comment

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

Suggested change
private static final Optional<Manifest> retrieveManifestFileAsStream(ServletContext context) {
private static Optional<Manifest> retrieveManifestFileAsStream(ServletContext context) {

My IDE reports: "Such code might indicate an error or an incorrect assumption about the effect of the final keyword. Static methods are not subject to runtime polymorphism, so the only purpose of the final keyword used with static methods is to ensure the method will not be hidden in a subclass."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I once did read that one should make all methods used in the constructor final, because if they are overloaded in a subclass, it could have confusing effects. I always do that; if the methods are under the constructor, you can see nicely in the “outline” what belongs to the constructors and where the actual class begins. For me, that is an improvement in the clarity of the code. In this case, it is particularly exciting because the “constructor” is not a constructor in the Java sense, but a method that is called by the Servlet Container to construct the class.

outline_

Copy link
Member

@solth solth Aug 23, 2024

Choose a reason for hiding this comment

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

I am not convinced having a structure outline in your IDE that you find more intuitive is a good enough reason to overrule this formal recommendation, but I can live with it if you really think it helps you.

try (InputStream manifestResource = context.getResourceAsStream("/META-INF/MANIFEST.MF")) {
if (Objects.nonNull(manifestResource)) {
return Optional.of(new Manifest(manifestResource));
}
} catch (IOException e) {
logger.error(e.getMessage(), e);
context.log(e.getMessage());
}
return Optional.empty();
}

/**
* Start up the active MQ connection.
*/
private void startActiveMQ() {
if (ConfigCore.getOptionalString(ParameterCore.ACTIVE_MQ_HOST_URL).isPresent()) {
activeMQDirector = new ActiveMQDirector();
Thread connectAsynchronously = new Thread(activeMQDirector);
connectAsynchronously.setName(ActiveMQDirector.class.getSimpleName());
connectAsynchronously.setDaemon(true);
connectAsynchronously.start();
}
}

/**
* Returns the application’s main class.
*
* @return the main class
* @throws UndeclaredThrowableException
* if the servlet container is shut down before the web
* application is started
*/
public static KitodoProduction getInstance() {
try {
return instance.get();
} catch (InterruptedException | ExecutionException e) {
logger.fatal(e);
throw new UndeclaredThrowableException(e);
}
}

/**
* Returns the servlet context.
*
* @return the servlet context
*/
public ServletContext getServletContext() {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this method? My IDE shows it is unused, and the two interfaces that this method implements do not seem to require the method either.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's just a getter. You might want to get the servlet context to get the application's working directory, for example, or the server version. It's here for convenience. At the moment, it's not used yet.

Copy link
Member

Choose a reason for hiding this comment

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

Please remove the method if it's unused and not required at the moment. Doesn't matter if it's just a getter. It should be added if required in the future.

return context;
}

/**
* Returns the manifest. If there is one. Otherwise, it can also be empty.
*
* @return the manifest
*/
public Optional<Manifest> getManifest() {
Copy link
Member

Choose a reason for hiding this comment

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

See above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is my idea that the main class of the application also provides essential characteristics of the application via getters. The manifest is such.

Copy link
Member

Choose a reason for hiding this comment

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

Please remove unused getters if they are not technically required, see above.

return manifest;
}

/**
* Returns application version information.
*
* @return version information
*/
public KitodoVersion getVersionInformation() {
return version;
}

@Override
public void contextDestroyed(ServletContextEvent sce) {
TaskManager.shutdownNow();
if (Objects.nonNull(activeMQDirector)) {
activeMQDirector.shutDown();
}
}

@Override
public void sessionCreated(HttpSessionEvent se) {
// nothing is done here
}

@Override
public void sessionDestroyed(HttpSessionEvent se) {
Object securityContextObject = se.getSession().getAttribute("SPRING_SECURITY_CONTEXT");
if (securityContextObject instanceof SecurityContextImpl) {
SecurityContextImpl securityContext = (SecurityContextImpl) securityContextObject;
Object principal = securityContext.getAuthentication().getPrincipal();
if (principal instanceof SecurityUserDetails) {
ServiceManager.getSessionService().expireSessionsOfUser((SecurityUserDetails) principal);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,38 +9,29 @@
* GPL3-License.txt file that was distributed with this source code.
*/

package org.kitodo.production.version;
package org.kitodo.production;

import java.util.jar.Attributes;
import java.util.jar.Manifest;

public class KitodoVersion {

private static String version = "N/A";
private static String buildVersion = "N/A";
private static String buildDate = "N/A";

/**
* Private constructor to hide the implicit public one.
*/
private KitodoVersion() {

}

/**
* Setup KitodoVersion form manifest.
*
* @param manifest as Manifest
*/
public static void setupFromManifest(Manifest manifest) {
void setupFromManifest(Manifest manifest) {
Attributes mainAttributes = manifest.getMainAttributes();

version = getValueOrThrowException(mainAttributes, "Implementation-Version");
buildVersion = version;
buildDate = getValueOrThrowException(mainAttributes, "Implementation-Build-Date");
}

private static String getValueOrThrowException(Attributes attributes, String attributeName) {
private String getValueOrThrowException(Attributes attributes, String attributeName) {
String value = attributes.getValue(attributeName);
if (null == value) {
throw new IllegalArgumentException(
Expand All @@ -49,15 +40,11 @@ private static String getValueOrThrowException(Attributes attributes, String att
return value;
}

public static String getVersion() {
public String getVersion() {
return version;
}

public static String getBuildVersion() {
return buildVersion;
}

public static String getBuildDate() {
public String getBuildDate() {
return buildDate;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@

import org.kitodo.config.ConfigCore;
import org.kitodo.config.enums.ParameterCore;
import org.kitodo.production.KitodoProduction;
import org.kitodo.production.helper.Helper;
import org.kitodo.production.version.KitodoVersion;

/**
* Helper form - used for some single methods which don't match yet to other
Expand All @@ -30,7 +30,7 @@
public class HelperForm implements Serializable {

public String getVersion() {
return KitodoVersion.getBuildVersion();
return KitodoProduction.getInstance().getVersionInformation().getVersion();
}

public boolean getAnonymized() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ static synchronized TaskManager singleton() {
* exit the task manager as well as its managed threads during container
* shutdown.
*/
static void shutdownNow() {
public static void shutdownNow() {
stopAndDeleteAllTasks();
singleton().taskSitter.shutdownNow();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@
* constructor is private) a caring class is needed which will be available for
* instantiation to the servlet container.
*/
@WebListener
public class TaskSitter implements Runnable, ServletContextListener {
public class TaskSitter implements Runnable {
/**
* The field autoRunLimit holds the number of threads which at most are
* allowed to be started automatically. It is by default initialised by the
Expand All @@ -55,27 +54,6 @@ public class TaskSitter implements Runnable, ServletContextListener {
setAutoRunningThreads(true);
}

/**
* When the servlet is unloaded, i.e. on container shutdown, the TaskManager
* shall be shut down gracefully.
*
* @see javax.servlet.ServletContextListener#contextDestroyed(javax.servlet.ServletContextEvent)
*/
@Override
public void contextDestroyed(ServletContextEvent arg) {
TaskManager.shutdownNow();
}

/**
* Currently, there is nothing to do when the servlet is loading.
*
* @see javax.servlet.ServletContextListener#contextInitialized(javax.servlet.ServletContextEvent)
*/
@Override
public void contextInitialized(ServletContextEvent argument) {
// nothing is done here
}

/**
* Returns whether the TaskManager’s
* autorun mode is on or not.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@
* The class ActiveMQDirector also provides a basic ExceptionListener
* implementation as required for the connection.
*/
@WebListener
public class ActiveMQDirector implements Runnable, ServletContextListener {
public class ActiveMQDirector implements Runnable {
private static final Logger logger = LogManager.getLogger(ActiveMQDirector.class);

// When implementing new services, add them to this list
Expand All @@ -64,21 +63,6 @@ public class ActiveMQDirector implements Runnable, ServletContextListener {
private static Session session = null;
private static MessageProducer resultsTopic;

/**
* The method is called by the web container on startup
* and is used to start up the active MQ connection. All processors from
* {@link #services} are registered.
*/
@Override
public void contextInitialized(ServletContextEvent initialisation) {
if (ConfigCore.getOptionalString(ParameterCore.ACTIVE_MQ_HOST_URL).isPresent()) {
Thread connectAsynchronously = new Thread(new ActiveMQDirector());
connectAsynchronously.setName(ActiveMQDirector.class.getSimpleName());
connectAsynchronously.setDaemon(true);
connectAsynchronously.start();
}
}

@Override
public void run() {
String activeMQHost = ConfigCore.getParameter(ParameterCore.ACTIVE_MQ_HOST_URL);
Expand Down Expand Up @@ -226,8 +210,7 @@ public static MessageProducer getResultsTopic() {
* The method contextDestroyed is called by the web container on shutdown.
* It shuts down all listeners, the session and last, the connection.
*/
@Override
public void contextDestroyed(ServletContextEvent destruction) {
public void shutDown() {
// Shut down all message consumers on any queues
for (ActiveMQProcessor service : services) {
MessageConsumer messageConsumer = service.getMessageConsumer();
Expand Down
Loading