You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ge...@apache.org on 2023/01/13 15:18:11 UTC

[solr] branch main updated: SOLR-16347: Skip JerseyApp creation when v2 disabled (#1210)

This is an automated email from the ASF dual-hosted git repository.

gerlowskija pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/main by this push:
     new 73a56cf5860 SOLR-16347: Skip JerseyApp creation when v2 disabled (#1210)
73a56cf5860 is described below

commit 73a56cf58603693f4795ec186ef3937e9a81f289
Author: Jason Gerlowski <ge...@apache.org>
AuthorDate: Fri Jan 13 10:18:06 2023 -0500

    SOLR-16347: Skip JerseyApp creation when v2 disabled (#1210)
    
    Prior to this commit, the "disable.v2.api" flag covered some aspects of
    request routing, but wasn't consulted during requestHandler
    parsing/bootstrapping or CoreContainer initialization.
    
    This commit creates a small utility method in V2ApiUtils to simplify
    the enabled/disabled check, and uses it in a few additional places to
    make sure Solr isn't doing any unnecessary work when v2 has explicitly
    been turned off.
---
 .../java/org/apache/solr/core/CoreContainer.java   | 84 ++++++++++++----------
 .../src/java/org/apache/solr/core/PluginBag.java   | 11 ++-
 .../src/java/org/apache/solr/core/SolrCore.java    |  5 +-
 .../org/apache/solr/handler/api/V2ApiUtils.java    |  4 ++
 .../apache/solr/servlet/CoreContainerProvider.java |  5 --
 .../apache/solr/servlet/SolrDispatchFilter.java    |  3 +-
 .../apache/solr/handler/api/V2ApiUtilsTest.java    | 39 ++++++++++
 7 files changed, 105 insertions(+), 46 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
index 00eb7454b8d..a32b552fe99 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -125,6 +125,7 @@ import org.apache.solr.handler.admin.SecurityConfHandlerZk;
 import org.apache.solr.handler.admin.ZookeeperInfoHandler;
 import org.apache.solr.handler.admin.ZookeeperReadAPI;
 import org.apache.solr.handler.admin.ZookeeperStatusHandler;
+import org.apache.solr.handler.api.V2ApiUtils;
 import org.apache.solr.handler.component.ShardHandlerFactory;
 import org.apache.solr.handler.designer.SchemaDesignerAPI;
 import org.apache.solr.jersey.InjectionFactories;
@@ -711,6 +712,14 @@ public class CoreContainer {
     return objectCache;
   }
 
+  private void registerV2ApiIfEnabled(Object apiObject) {
+    if (containerHandlers.getApiBag() == null) {
+      return;
+    }
+
+    containerHandlers.getApiBag().registerObject(apiObject);
+  }
+
   // -------------------------------------------------------------------
   // Initialization / Cleanup
   // -------------------------------------------------------------------
@@ -776,14 +785,15 @@ public class CoreContainer {
       pkiAuthenticationSecurityBuilder.initializeMetrics(solrMetricsContext, "/authentication/pki");
 
       packageStoreAPI = new PackageStoreAPI(this);
-      containerHandlers.getApiBag().registerObject(packageStoreAPI.readAPI);
-      containerHandlers.getApiBag().registerObject(packageStoreAPI.writeAPI);
+      registerV2ApiIfEnabled(packageStoreAPI.readAPI);
+      registerV2ApiIfEnabled(packageStoreAPI.writeAPI);
 
       packageLoader = new SolrPackageLoader(this);
-      containerHandlers.getApiBag().registerObject(packageLoader.getPackageAPI().editAPI);
-      containerHandlers.getApiBag().registerObject(packageLoader.getPackageAPI().readAPI);
+      registerV2ApiIfEnabled(packageLoader.getPackageAPI().editAPI);
+      registerV2ApiIfEnabled(packageLoader.getPackageAPI().readAPI);
+
       ZookeeperReadAPI zookeeperReadAPI = new ZookeeperReadAPI(this);
-      containerHandlers.getApiBag().registerObject(zookeeperReadAPI);
+      registerV2ApiIfEnabled(zookeeperReadAPI);
     }
 
     MDCLoggingContext.setNode(this);
@@ -815,19 +825,19 @@ public class CoreContainer {
         createHandler(
             COLLECTIONS_HANDLER_PATH, cfg.getCollectionsHandlerClass(), CollectionsHandler.class);
     final CollectionsAPI collectionsAPI = new CollectionsAPI(collectionsHandler);
-    containerHandlers.getApiBag().registerObject(collectionsAPI);
-    containerHandlers.getApiBag().registerObject(collectionsAPI.collectionsCommands);
+    registerV2ApiIfEnabled(collectionsAPI);
+    registerV2ApiIfEnabled(collectionsAPI.collectionsCommands);
     final CollectionBackupsAPI collectionBackupsAPI = new CollectionBackupsAPI(collectionsHandler);
-    containerHandlers.getApiBag().registerObject(collectionBackupsAPI);
+    registerV2ApiIfEnabled(collectionBackupsAPI);
     configSetsHandler =
         createHandler(
             CONFIGSETS_HANDLER_PATH, cfg.getConfigSetsHandlerClass(), ConfigSetsHandler.class);
     ClusterAPI clusterAPI = new ClusterAPI(collectionsHandler, configSetsHandler);
-    containerHandlers.getApiBag().registerObject(clusterAPI);
-    containerHandlers.getApiBag().registerObject(clusterAPI.commands);
+    registerV2ApiIfEnabled(clusterAPI);
+    registerV2ApiIfEnabled(clusterAPI.commands);
 
     if (isZooKeeperAware()) {
-      containerHandlers.getApiBag().registerObject(new SchemaDesignerAPI(this));
+      registerV2ApiIfEnabled(new SchemaDesignerAPI(this));
     } // else Schema Designer not available in standalone (non-cloud) mode
 
     /*
@@ -1031,8 +1041,8 @@ public class CoreContainer {
       containerPluginsRegistry.refresh();
       getZkController().zkStateReader.registerClusterPropertiesListener(containerPluginsRegistry);
       ContainerPluginsApi containerPluginsApi = new ContainerPluginsApi(this);
-      containerHandlers.getApiBag().registerObject(containerPluginsApi.readAPI);
-      containerHandlers.getApiBag().registerObject(containerPluginsApi.editAPI);
+      registerV2ApiIfEnabled(containerPluginsApi.readAPI);
+      registerV2ApiIfEnabled(containerPluginsApi.editAPI);
 
       // initialize the placement plugin factory wrapper
       // with the plugin configuration from the registry
@@ -1056,29 +1066,31 @@ public class CoreContainer {
               });
     }
 
-    final CoreContainer thisCCRef = this;
-    // Init the Jersey app once all CC endpoints have been registered
-    containerHandlers
-        .getJerseyEndpoints()
-        .register(
-            new AbstractBinder() {
-              @Override
-              protected void configure() {
-                bindFactory(new InjectionFactories.SingletonFactory<>(thisCCRef))
-                    .to(CoreContainer.class)
-                    .in(Singleton.class);
-              }
-            })
-        .register(
-            new AbstractBinder() {
-              @Override
-              protected void configure() {
-                bindFactory(new InjectionFactories.SingletonFactory<>(nodeKeyPair))
-                    .to(SolrNodeKeyPair.class)
-                    .in(Singleton.class);
-              }
-            });
-    jerseyAppHandler = new ApplicationHandler(containerHandlers.getJerseyEndpoints());
+    if (V2ApiUtils.isEnabled()) {
+      final CoreContainer thisCCRef = this;
+      // Init the Jersey app once all CC endpoints have been registered
+      containerHandlers
+          .getJerseyEndpoints()
+          .register(
+              new AbstractBinder() {
+                @Override
+                protected void configure() {
+                  bindFactory(new InjectionFactories.SingletonFactory<>(thisCCRef))
+                      .to(CoreContainer.class)
+                      .in(Singleton.class);
+                }
+              })
+          .register(
+              new AbstractBinder() {
+                @Override
+                protected void configure() {
+                  bindFactory(new InjectionFactories.SingletonFactory<>(nodeKeyPair))
+                      .to(SolrNodeKeyPair.class)
+                      .in(Singleton.class);
+                }
+              });
+      jerseyAppHandler = new ApplicationHandler(containerHandlers.getJerseyEndpoints());
+    }
 
     // Do Node setup logic after all handlers have been registered.
     if (isZooKeeperAware()) {
diff --git a/solr/core/src/java/org/apache/solr/core/PluginBag.java b/solr/core/src/java/org/apache/solr/core/PluginBag.java
index 5ecf681ad62..3711e2c0f17 100644
--- a/solr/core/src/java/org/apache/solr/core/PluginBag.java
+++ b/solr/core/src/java/org/apache/solr/core/PluginBag.java
@@ -42,6 +42,7 @@ import org.apache.solr.api.JerseyResource;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.handler.RequestHandlerBase;
+import org.apache.solr.handler.api.V2ApiUtils;
 import org.apache.solr.handler.component.SearchComponent;
 import org.apache.solr.jersey.JerseyApplications;
 import org.apache.solr.pkg.PackagePluginHolder;
@@ -63,6 +64,8 @@ public class PluginBag<T> implements AutoCloseable {
   private final Class<T> klass;
   private SolrCore core;
   private final SolrConfig.SolrPluginInfo meta;
+
+  private final boolean loadV2ApisIfPresent;
   private final ApiBag apiBag;
   private final ResourceConfig jerseyResources;
 
@@ -73,7 +76,8 @@ public class PluginBag<T> implements AutoCloseable {
 
   /** Pass needThreadSafety=true if plugins can be added and removed concurrently with lookups. */
   public PluginBag(Class<T> klass, SolrCore core, boolean needThreadSafety) {
-    if (klass == SolrRequestHandler.class) {
+    if (klass == SolrRequestHandler.class && V2ApiUtils.isEnabled()) {
+      this.loadV2ApisIfPresent = true;
       this.apiBag = new ApiBag(core != null);
       this.infoBeanByResource = new JerseyMetricsLookupRegistry();
       this.jerseyResources =
@@ -81,6 +85,7 @@ public class PluginBag<T> implements AutoCloseable {
               ? new JerseyApplications.CoreContainerApp(infoBeanByResource)
               : new JerseyApplications.SolrCoreApp(core, infoBeanByResource);
     } else {
+      this.loadV2ApisIfPresent = false;
       this.apiBag = null;
       this.jerseyResources = null;
       this.infoBeanByResource = null;
@@ -218,7 +223,7 @@ public class PluginBag<T> implements AutoCloseable {
       }
     }
 
-    if (apiBag != null) {
+    if (loadV2ApisIfPresent) {
       if (plugin.isLoaded()) {
         T inst = plugin.get();
         if (inst instanceof ApiSupport) {
@@ -226,7 +231,7 @@ public class PluginBag<T> implements AutoCloseable {
           if (registerApi == null) registerApi = apiSupport.registerV2();
           if (disableHandler == null) disableHandler = !apiSupport.registerV1();
 
-          if (registerApi) {
+          if (registerApi && V2ApiUtils.isEnabled()) {
             Collection<Api> apis = apiSupport.getApis();
             if (apis != null) {
               Map<String, String> nameSubstitutes = singletonMap(HANDLER_NAME, name);
diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java
index 33f209e1f77..75cb384d4fd 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -107,6 +107,7 @@ import org.apache.solr.handler.IndexFetcher;
 import org.apache.solr.handler.ReplicationHandler;
 import org.apache.solr.handler.RequestHandlerBase;
 import org.apache.solr.handler.SolrConfigHandler;
+import org.apache.solr.handler.api.V2ApiUtils;
 import org.apache.solr.handler.component.HighlightComponent;
 import org.apache.solr.handler.component.SearchComponent;
 import org.apache.solr.logging.MDCLoggingContext;
@@ -1136,7 +1137,9 @@ public class SolrCore implements SolrInfoBean, Closeable {
       reqHandlers = new RequestHandlers(this);
       reqHandlers.initHandlersFromConfig(solrConfig);
       jerseyAppHandler =
-          new ApplicationHandler(reqHandlers.getRequestHandlers().getJerseyEndpoints());
+          (V2ApiUtils.isEnabled())
+              ? new ApplicationHandler(reqHandlers.getRequestHandlers().getJerseyEndpoints())
+              : null;
 
       // cause the executor to stall so firstSearcher events won't fire
       // until after inform() has been called for all components.
diff --git a/solr/core/src/java/org/apache/solr/handler/api/V2ApiUtils.java b/solr/core/src/java/org/apache/solr/handler/api/V2ApiUtils.java
index 132671a5c5d..d459a85caf6 100644
--- a/solr/core/src/java/org/apache/solr/handler/api/V2ApiUtils.java
+++ b/solr/core/src/java/org/apache/solr/handler/api/V2ApiUtils.java
@@ -31,6 +31,10 @@ public class V2ApiUtils {
     /* Private ctor prevents instantiation */
   }
 
+  public static boolean isEnabled() {
+    return !"true".equals(System.getProperty("disable.v2.api", "false"));
+  }
+
   public static void flattenMapWithPrefix(
       Map<String, Object> toFlatten, Map<String, Object> destination, String additionalPrefix) {
     if (toFlatten == null || toFlatten.isEmpty() || destination == null) {
diff --git a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java
index aad7d66946c..65568e584d4 100644
--- a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java
+++ b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java
@@ -91,7 +91,6 @@ public class CoreContainerProvider implements ServletContextListener {
   private RateLimitManager rateLimitManager;
   private final CountDownLatch init = new CountDownLatch(1);
   private String registryName;
-  private final boolean isV2Enabled = !"true".equals(System.getProperty("disable.v2.api", "false"));
   // AFAIK the only reason we need this is to support JettySolrRunner for tests. In tests we might
   // have multiple CoreContainers in the same JVM, but I *think* that doesn't happen in a real
   // server.
@@ -477,10 +476,6 @@ public class CoreContainerProvider implements ServletContextListener {
     this.rateLimitManager = rateLimitManager;
   }
 
-  public boolean isV2Enabled() {
-    return isV2Enabled;
-  }
-
   private static class ContextInitializationKey {
     private final ServletContext ctx;
     private final CountDownLatch initializing = new CountDownLatch(1);
diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
index 97f5e254025..29003c78d53 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
@@ -50,6 +50,7 @@ import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.NodeRoles;
 import org.apache.solr.core.SolrCore;
+import org.apache.solr.handler.api.V2ApiUtils;
 import org.apache.solr.logging.MDCLoggingContext;
 import org.apache.solr.logging.MDCSnapshot;
 import org.apache.solr.security.AuditEvent;
@@ -94,7 +95,7 @@ public class SolrDispatchFilter extends BaseSolrFilter implements PathExcluder {
 
   private List<Pattern> excludePatterns;
 
-  public final boolean isV2Enabled = !"true".equals(System.getProperty("disable.v2.api", "false"));
+  public final boolean isV2Enabled = V2ApiUtils.isEnabled();
 
   public HttpClient getHttpClient() {
     try {
diff --git a/solr/core/src/test/org/apache/solr/handler/api/V2ApiUtilsTest.java b/solr/core/src/test/org/apache/solr/handler/api/V2ApiUtilsTest.java
new file mode 100644
index 00000000000..5089a36dd70
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/handler/api/V2ApiUtilsTest.java
@@ -0,0 +1,39 @@
+/*
+ * 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.solr.handler.api;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.junit.Test;
+
+public class V2ApiUtilsTest extends SolrTestCaseJ4 {
+
+  @Test
+  public void testReadsDisableV2ApiSysprop() {
+    System.clearProperty("disable.v2.api");
+    assertTrue("v2 API should be enabled if sysprop not specified", V2ApiUtils.isEnabled());
+
+    System.setProperty("disable.v2.api", "false");
+    assertTrue("v2 API should be enabled if sysprop explicitly enables it", V2ApiUtils.isEnabled());
+
+    System.setProperty("disable.v2.api", "asdf");
+    assertTrue("v2 API should be enabled if sysprop has unexpected value", V2ApiUtils.isEnabled());
+
+    System.setProperty("disable.v2.api", "true");
+    assertFalse(
+        "v2 API should be disabled if sysprop explicitly disables it", V2ApiUtils.isEnabled());
+  }
+}