From e55f5b875efe58702c38364256f3321c35107b42 Mon Sep 17 00:00:00 2001 From: ys-kalyakin Date: Sat, 15 Feb 2025 19:12:08 +0300 Subject: [PATCH] create action optimization --- .../struts/action/RequestProcessor.java | 51 +++++----- .../chain/commands/servlet/CreateAction.java | 39 ++++---- .../struts/chain/contexts/ActionContext.java | 6 +- .../main/java/org/apache/struts/util/Try.java | 94 +++++++++++++++++++ 4 files changed, 141 insertions(+), 49 deletions(-) create mode 100644 core/src/main/java/org/apache/struts/util/Try.java diff --git a/core/src/main/java/org/apache/struts/action/RequestProcessor.java b/core/src/main/java/org/apache/struts/action/RequestProcessor.java index 4c157afbe..47732bd5c 100644 --- a/core/src/main/java/org/apache/struts/action/RequestProcessor.java +++ b/core/src/main/java/org/apache/struts/action/RequestProcessor.java @@ -22,7 +22,8 @@ import java.io.IOException; import java.io.Serializable; -import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.Locale; import jakarta.servlet.RequestDispatcher; @@ -86,7 +87,7 @@ public class RequestProcessor implements Serializable { * initialized, keyed by the fully qualified Java class name of the * Action class.

*/ - protected HashMap actions = new HashMap<>(); + protected final Map actions = new ConcurrentHashMap<>(); /** *

The ModuleConfiguration with which we are @@ -125,9 +126,7 @@ public void destroy() { */ public void init(ActionServlet servlet, ModuleConfig moduleConfig) throws ServletException { - synchronized (actions) { - actions.clear(); - } + actions.clear(); this.servlet = servlet; this.moduleConfig = moduleConfig; @@ -198,9 +197,7 @@ public void process(HttpServletRequest request, HttpServletResponse response) ActionForward forward = processException(request, response, e, form, mapping); processForwardConfig(request, response, forward); return; - } catch (IOException e) { - throw e; - } catch (ServletException e) { + } catch (IOException | ServletException e) { throw e; } @@ -241,9 +238,11 @@ public void process(HttpServletRequest request, HttpServletResponse response) * the current request. * @throws IOException if an input/output error occurs */ - protected Action processActionCreate(HttpServletRequest request, - HttpServletResponse response, ActionMapping mapping) - throws IOException { + protected Action processActionCreate( + HttpServletRequest request, + HttpServletResponse response, + ActionMapping mapping + ) throws IOException { // Acquire the Action instance we will be using (if there is one) String className = mapping.getType(); @@ -252,21 +251,11 @@ protected Action processActionCreate(HttpServletRequest request, // If there were a mapping property indicating whether // an Action were a singleton or not ([true]), // could we just instantiate and return a new instance here? - Action instance; - - synchronized (actions) { - // Return any existing Action instance of this class - instance = actions.get(className); - - if (instance != null) { - log.trace(" Returning existing Action instance"); - - return (instance); - } - + Action action = actions.computeIfAbsent(className, key -> { // Create and return a new Action instance log.trace(" Creating new Action instance"); + Action instance; try { instance = (Action) RequestUtils.applicationInstance(className); @@ -278,20 +267,24 @@ protected Action processActionCreate(HttpServletRequest request, mapping.getPath(), mapping.toString())) .setCause(e).log(); - response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, - getInternal().getMessage("actionCreate", mapping.getPath())); - return (null); } - actions.put(className, instance); - if (instance.getServlet() == null) { instance.setServlet(this.servlet); } + + return instance; + }); + + if (action == null) { + response.sendError( + HttpServletResponse.SC_INTERNAL_SERVER_ERROR, + getInternal().getMessage("actionCreate", mapping.getPath()) + ); } - return (instance); + return (action); } /** diff --git a/core/src/main/java/org/apache/struts/chain/commands/servlet/CreateAction.java b/core/src/main/java/org/apache/struts/chain/commands/servlet/CreateAction.java index 3719a4986..2d1780653 100644 --- a/core/src/main/java/org/apache/struts/chain/commands/servlet/CreateAction.java +++ b/core/src/main/java/org/apache/struts/chain/commands/servlet/CreateAction.java @@ -20,9 +20,6 @@ */ package org.apache.struts.chain.commands.servlet; -import java.util.HashMap; -import java.util.Map; - import org.apache.struts.action.Action; import org.apache.struts.action.ActionServlet; import org.apache.struts.chain.Constants; @@ -31,9 +28,13 @@ import org.apache.struts.chain.contexts.ServletActionContext; import org.apache.struts.config.ActionConfig; import org.apache.struts.config.ModuleConfig; +import org.apache.struts.util.Try; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + /** *

Concrete implementation of AbstractCreateAction for use in * a Servlet API chain. Expects that the ActionContext passed into it can @@ -52,9 +53,11 @@ public class CreateAction /* :TODO The Action class' dependency on having its "servlet" property set * requires this API-dependent subclass of AbstractCreateAction. */ - protected synchronized Action getAction(ActionContext context, String type, - ActionConfig actionConfig) - throws Exception { + protected Action getAction( + ActionContext context, + String type, + ActionConfig actionConfig + ) throws Exception { ServletActionContext saContext = (ServletActionContext) context; ActionServlet actionServlet = saContext.getActionServlet(); @@ -62,24 +65,26 @@ protected synchronized Action getAction(ActionContext context, String type, ModuleConfig moduleConfig = actionConfig.getModuleConfig(); String actionsKey = Constants.ACTIONS_KEY + moduleConfig.getPrefix(); @SuppressWarnings("unchecked") - Map actions = (Map) context.getApplicationScope().get(actionsKey); + Map> actions = (Map>) context.getApplicationScope().get(actionsKey); if (actions == null) { - actions = new HashMap<>(); - context.getApplicationScope().put(actionsKey, actions); + synchronized (this) { + //noinspection unchecked + actions = (Map>) context.getApplicationScope().get(actionsKey); + if (actions == null) { + actions = new ConcurrentHashMap<>(); + context.getApplicationScope().put(actionsKey, actions); + } + } } - Action action = null; + Action action; try { if (actionConfig.isSingleton()) { - synchronized (actions) { - action = actions.get(type); - if (action == null) { - action = createAction(context, type); - actions.put(type, action); - } - } + action = actions + .computeIfAbsent(type, key -> Try.ofCallable(() -> createAction(context, type))) + .get(); } else { action = createAction(context, type); } diff --git a/core/src/main/java/org/apache/struts/chain/contexts/ActionContext.java b/core/src/main/java/org/apache/struts/chain/contexts/ActionContext.java index e15351816..3b1f090fd 100644 --- a/core/src/main/java/org/apache/struts/chain/contexts/ActionContext.java +++ b/core/src/main/java/org/apache/struts/chain/contexts/ActionContext.java @@ -38,9 +38,9 @@ * session-scoped resources and services

*/ public interface ActionContext extends Context { - public static final String APPLICATION_SCOPE = "application"; - public static final String SESSION_SCOPE = "session"; - public static final String REQUEST_SCOPE = "request"; + String APPLICATION_SCOPE = "application"; + String SESSION_SCOPE = "session"; + String REQUEST_SCOPE = "request"; // ------------------------------- // General Application Support diff --git a/core/src/main/java/org/apache/struts/util/Try.java b/core/src/main/java/org/apache/struts/util/Try.java new file mode 100644 index 000000000..eedac00aa --- /dev/null +++ b/core/src/main/java/org/apache/struts/util/Try.java @@ -0,0 +1,94 @@ +/* + * $Id$ + * + * 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.util; + +import java.util.concurrent.Callable; + +/** + * The {@code Try} class represents a computation that may result in a successfully computed value + * or a checked exception. + * + * @param the type of the result produced by the computation + */ +public abstract class Try { + + /** + * Wraps a checked computation into a {@code Try} instance. If the computation throws an exception, + * a {@code Failure} containing the exception is returned; otherwise, a {@code Success} with the + * result is returned. + * + * @param callable the checked computation to execute + * @param the result type of the computation + * @return a {@code Try} representing the success or failure of the computation + */ + public static Try ofCallable(Callable callable) { + try { + return new Success<>(callable.call()); + } catch (Exception e) { + return new Failure<>(e); + } + } + + /** + * Returns the result value if the computation is successful. If this is a {@code Failure}, throws + * the contained exception. + * + * @return the successfully computed value + * @throws Exception if this is a {@code Failure}, wrapping the original checked exception + */ + public abstract T get() throws Exception; + + /** + * Represents result of a successfully computed value + * + * @param the type of the result produced by the computation + */ + public static class Success extends Try { + private final T result; + + public Success(T result) { + this.result = result; + } + + @Override + public T get() { + return result; + } + } + + /** + * Represents an exception + * + * @param the type of the result produced by the computation + */ + public static class Failure extends Try { + private final Exception e; + + public Failure(Exception e) { + this.e = e; + } + + @Override + public T get() throws Exception { + throw e; + } + } +} \ No newline at end of file