On Apr 30, 2023, at 23:37, Mark Thomas <ma...@apache.org> wrote: On 28/04/2023 03:58, li...@apache.org wrote:
This is an automated email from the ASF dual-hosted git repository. lihan pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/main by this push: new fa0b2b196d Polish fa0b2b196d is described below commit fa0b2b196d8525a662e9edec258650865da465ed Author: lihan <li...@apache.org> AuthorDate: Fri Apr 28 10:57:48 2023 +0800 Polish --- java/org/apache/catalina/core/ContainerBase.java | 18 +++++++-------- java/org/apache/catalina/core/StandardServer.java | 26 +++++++++------------- .../apache/catalina/valves/JsonAccessLogValve.java | 2 +- 3 files changed, 21 insertions(+), 25 deletions(-) diff --git a/java/org/apache/catalina/core/ContainerBase.java b/java/org/apache/catalina/core/ContainerBase.java index 9dc018be15..784c9032ef 100644 --- a/java/org/apache/catalina/core/ContainerBase.java +++ b/java/org/apache/catalina/core/ContainerBase.java @@ -377,7 +377,7 @@ public abstract class ContainerBase extends LifecycleMBeanBase implements Contai this.cluster = cluster; // Stop the old component if necessary - if (getState().isAvailable() && (oldCluster != null) && (oldCluster instanceof Lifecycle)) { + if (getState().isAvailable() && (oldCluster instanceof Lifecycle)) { try { ((Lifecycle) oldCluster).stop(); } catch (LifecycleException e) { @@ -390,7 +390,7 @@ public abstract class ContainerBase extends LifecycleMBeanBase implements Contai cluster.setContainer(this); } - if (getState().isAvailable() && (cluster != null) && (cluster instanceof Lifecycle)) { + if (getState().isAvailable() && (cluster instanceof Lifecycle)) { try { ((Lifecycle) cluster).start(); } catch (LifecycleException e) { @@ -580,7 +580,7 @@ public abstract class ContainerBase extends LifecycleMBeanBase implements Contai this.realm = realm; // Stop the old component if necessary - if (getState().isAvailable() && (oldRealm != null) && (oldRealm instanceof Lifecycle)) { + if (getState().isAvailable() && (oldRealm instanceof Lifecycle)) { try { ((Lifecycle) oldRealm).stop(); } catch (LifecycleException e) { @@ -592,7 +592,7 @@ public abstract class ContainerBase extends LifecycleMBeanBase implements Contai if (realm != null) { realm.setContainer(this); } - if (getState().isAvailable() && (realm != null) && (realm instanceof Lifecycle)) { + if (getState().isAvailable() && (realm instanceof Lifecycle)) { try { ((Lifecycle) realm).start(); } catch (LifecycleException e) { @@ -832,8 +832,8 @@ public abstract class ContainerBase extends LifecycleMBeanBase implements Contai } // Start our child containers, if any - Container children[] = findChildren(); - List<Future<Void>> results = new ArrayList<>(); + Container[] children = findChildren(); + List<Future<Void>> results = new ArrayList<>(children.length); for (Container child : children) { results.add(startStopExecutor.submit(new StartChild(child))); } @@ -897,8 +897,8 @@ public abstract class ContainerBase extends LifecycleMBeanBase implements Contai } // Stop our child containers, if any - Container children[] = findChildren(); - List<Future<Void>> results = new ArrayList<>(); + Container[] children = findChildren(); + List<Future<Void>> results = new ArrayList<>(children.length); for (Container child : children) { results.add(startStopExecutor.submit(new StopChild(child))); } @@ -992,7 +992,7 @@ public abstract class ContainerBase extends LifecycleMBeanBase implements Contai } AccessLogAdapter adapter = null; - Valve valves[] = getPipeline().getValves(); + Valve[] valves = getPipeline().getValves(); for (Valve valve : valves) { if (valve instanceof AccessLog) { if (adapter == null) { diff --git a/java/org/apache/catalina/core/StandardServer.java b/java/org/apache/catalina/core/StandardServer.java index eb5e91e932..09a223fa80 100644 --- a/java/org/apache/catalina/core/StandardServer.java +++ b/java/org/apache/catalina/core/StandardServer.java @@ -135,7 +135,7 @@ public final class StandardServer extends LifecycleMBeanBase implements Server { /** * The set of Services associated with this Server. */ - private Service services[] = new Service[0]; + private Service[] services = new Service[0]; private final Object servicesLock = new Object(); @@ -175,12 +175,12 @@ public final class StandardServer extends LifecycleMBeanBase implements Server { /** * The number of threads available to process utility tasks in this service. */ - protected int utilityThreads = 2; + private int utilityThreads = 2; This is changing the public API. We can do that in 11.0.x but not in earlier versions. If we do make this change in 11.0.x then we really should deprecated the field in at least 10.1.x to provide users with advance warning of the change. Sorry Mark, I am so confusing that why this called changed the public API. According my review, users can not access this field directly with Server instance as the modifier is protected and whom can not extend StandardServer to do same operation as the modifier of StandardServer is final. So only us can access this field directly in StandardServer, no? Or maybe I miss something? : ( The same comment applies to all the fields changed from protected to private. To be clear, I am in favour of this change. I'd prefer us to be using public getters and setters rather than protected fields. But any such change needs to be communicated to the users. +1 Thanks Han Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
