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 2022/09/19 14:28:19 UTC
[solr] branch main updated: Remove ApiRegistrar abstraction (#1023)
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 a414b3c1b34 Remove ApiRegistrar abstraction (#1023)
a414b3c1b34 is described below
commit a414b3c1b34a78f9bfb46304ccd9475e0a789b60
Author: Jason Gerlowski <ge...@apache.org>
AuthorDate: Mon Sep 19 10:28:14 2022 -0400
Remove ApiRegistrar abstraction (#1023)
Earlier on during v2 API refactoring, I had created ApiRegistrar as a
way to bundle together the registration of API classes that were
multiplying and cluttering up CoreContainer.load.
In hindsight, this was based on a misunderstanding that there was a
reason to register APIs (directly) in CoreContainer.load in the first
place. But there's not: both core and container-level APIs can be
registered just fine via the 'getApis()' method present on their
associated requestHandler. The only APIs that cannot use this approach,
and must be registered explicitly in CoreContainer.load are those v2
APIs that don't have a v1/requestHandler equivalent.
With this in mind, I'm removing my ill-conceived attempt at an
ApiRegistrar class, to simplify the overall picture for API
registration.
---
.../java/org/apache/solr/core/CoreContainer.java | 3 -
.../solr/handler/admin/CollectionsHandler.java | 44 +++++++++++++
.../org/apache/solr/handler/api/ApiRegistrar.java | 76 ----------------------
.../solr/handler/admin/TestApiFramework.java | 6 +-
.../solr/handler/admin/TestCollectionAPIs.java | 6 +-
.../admin/api/V2CollectionAPIMappingTest.java | 14 +++-
.../handler/admin/api/V2ShardsAPIMappingTest.java | 10 ++-
7 files changed, 70 insertions(+), 89 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 fd1d7c1c682..cd5c267eda2 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -126,7 +126,6 @@ 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.ApiRegistrar;
import org.apache.solr.handler.component.ShardHandlerFactory;
import org.apache.solr.handler.designer.SchemaDesignerAPI;
import org.apache.solr.jersey.CoreContainerFactory;
@@ -817,8 +816,6 @@ public class CoreContainer {
createHandler(
COLLECTIONS_HANDLER_PATH, cfg.getCollectionsHandlerClass(), CollectionsHandler.class);
final CollectionsAPI collectionsAPI = new CollectionsAPI(collectionsHandler);
- ApiRegistrar.registerCollectionApis(containerHandlers.getApiBag(), collectionsHandler);
- ApiRegistrar.registerShardApis(containerHandlers.getApiBag(), collectionsHandler);
containerHandlers.getApiBag().registerObject(collectionsAPI);
containerHandlers.getApiBag().registerObject(collectionsAPI.collectionsCommands);
final CollectionBackupsAPI collectionBackupsAPI = new CollectionBackupsAPI(collectionsHandler);
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
index becfff21dd3..eff2f287ce8 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
@@ -141,6 +141,8 @@ import java.util.concurrent.TimeoutException;
import java.util.stream.Collectors;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
+import org.apache.solr.api.AnnotatedApi;
+import org.apache.solr.api.Api;
import org.apache.solr.client.solrj.SolrResponse;
import org.apache.solr.client.solrj.impl.HttpSolrClient;
import org.apache.solr.client.solrj.impl.HttpSolrClient.Builder;
@@ -197,6 +199,24 @@ import org.apache.solr.core.backup.repository.BackupRepository;
import org.apache.solr.core.snapshots.CollectionSnapshotMetaData;
import org.apache.solr.core.snapshots.SolrSnapshotManager;
import org.apache.solr.handler.RequestHandlerBase;
+import org.apache.solr.handler.admin.api.AddReplicaAPI;
+import org.apache.solr.handler.admin.api.AddReplicaPropertyAPI;
+import org.apache.solr.handler.admin.api.BalanceShardUniqueAPI;
+import org.apache.solr.handler.admin.api.CollectionStatusAPI;
+import org.apache.solr.handler.admin.api.CreateShardAPI;
+import org.apache.solr.handler.admin.api.DeleteCollectionAPI;
+import org.apache.solr.handler.admin.api.DeleteReplicaAPI;
+import org.apache.solr.handler.admin.api.DeleteReplicaPropertyAPI;
+import org.apache.solr.handler.admin.api.DeleteShardAPI;
+import org.apache.solr.handler.admin.api.ForceLeaderAPI;
+import org.apache.solr.handler.admin.api.MigrateDocsAPI;
+import org.apache.solr.handler.admin.api.ModifyCollectionAPI;
+import org.apache.solr.handler.admin.api.MoveReplicaAPI;
+import org.apache.solr.handler.admin.api.RebalanceLeadersAPI;
+import org.apache.solr.handler.admin.api.ReloadCollectionAPI;
+import org.apache.solr.handler.admin.api.SetCollectionPropertyAPI;
+import org.apache.solr.handler.admin.api.SplitShardAPI;
+import org.apache.solr.handler.admin.api.SyncShardAPI;
import org.apache.solr.logging.MDCLoggingContext;
import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
@@ -2048,6 +2068,30 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
return Boolean.TRUE;
}
+ @Override
+ public Collection<Api> getApis() {
+ final List<Api> apis = new ArrayList<>();
+ apis.addAll(AnnotatedApi.getApis(new SplitShardAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new CreateShardAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new AddReplicaAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new DeleteShardAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new SyncShardAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new ForceLeaderAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new DeleteReplicaAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new AddReplicaPropertyAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new BalanceShardUniqueAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new DeleteCollectionAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new DeleteReplicaPropertyAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new MigrateDocsAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new ModifyCollectionAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new MoveReplicaAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new RebalanceLeadersAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new ReloadCollectionAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new SetCollectionPropertyAPI(this)));
+ apis.addAll(AnnotatedApi.getApis(new CollectionStatusAPI(this)));
+ return apis;
+ }
+
// These "copy" methods were once SolrParams.getAll but were moved here as there is no universal
// way that a SolrParams can be represented in a Map; there are various choices.
diff --git a/solr/core/src/java/org/apache/solr/handler/api/ApiRegistrar.java b/solr/core/src/java/org/apache/solr/handler/api/ApiRegistrar.java
deleted file mode 100644
index a0de011d3ca..00000000000
--- a/solr/core/src/java/org/apache/solr/handler/api/ApiRegistrar.java
+++ /dev/null
@@ -1,76 +0,0 @@
-/*
- * 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.api.ApiBag;
-import org.apache.solr.handler.admin.CollectionsHandler;
-import org.apache.solr.handler.admin.api.AddReplicaAPI;
-import org.apache.solr.handler.admin.api.AddReplicaPropertyAPI;
-import org.apache.solr.handler.admin.api.BalanceShardUniqueAPI;
-import org.apache.solr.handler.admin.api.CollectionStatusAPI;
-import org.apache.solr.handler.admin.api.CreateShardAPI;
-import org.apache.solr.handler.admin.api.DeleteCollectionAPI;
-import org.apache.solr.handler.admin.api.DeleteReplicaAPI;
-import org.apache.solr.handler.admin.api.DeleteReplicaPropertyAPI;
-import org.apache.solr.handler.admin.api.DeleteShardAPI;
-import org.apache.solr.handler.admin.api.ForceLeaderAPI;
-import org.apache.solr.handler.admin.api.MigrateDocsAPI;
-import org.apache.solr.handler.admin.api.ModifyCollectionAPI;
-import org.apache.solr.handler.admin.api.MoveReplicaAPI;
-import org.apache.solr.handler.admin.api.RebalanceLeadersAPI;
-import org.apache.solr.handler.admin.api.ReloadCollectionAPI;
-import org.apache.solr.handler.admin.api.SetCollectionPropertyAPI;
-import org.apache.solr.handler.admin.api.SplitShardAPI;
-import org.apache.solr.handler.admin.api.SyncShardAPI;
-
-/**
- * Registers annotation-based V2 APIs with an {@link ApiBag}
- *
- * <p>Historically these APIs were registered directly by code in {@link
- * org.apache.solr.core.CoreContainer}, but as the number of annotation-based v2 APIs grew this
- * became increasingly unwieldy. So {@link ApiRegistrar} serves as a single place where all APIs
- * under a particular path can be registered together.
- */
-public class ApiRegistrar {
-
- public static void registerCollectionApis(ApiBag apiBag, CollectionsHandler collectionsHandler) {
- apiBag.registerObject(new AddReplicaPropertyAPI(collectionsHandler));
- apiBag.registerObject(new BalanceShardUniqueAPI(collectionsHandler));
- apiBag.registerObject(new DeleteCollectionAPI(collectionsHandler));
- apiBag.registerObject(new DeleteReplicaPropertyAPI(collectionsHandler));
- apiBag.registerObject(new MigrateDocsAPI(collectionsHandler));
- apiBag.registerObject(new ModifyCollectionAPI(collectionsHandler));
- apiBag.registerObject(new MoveReplicaAPI(collectionsHandler));
- apiBag.registerObject(new RebalanceLeadersAPI(collectionsHandler));
- apiBag.registerObject(new ReloadCollectionAPI(collectionsHandler));
- apiBag.registerObject(new SetCollectionPropertyAPI(collectionsHandler));
- apiBag.registerObject(new CollectionStatusAPI(collectionsHandler));
- }
-
- public static void registerShardApis(ApiBag apiBag, CollectionsHandler collectionsHandler) {
- apiBag.registerObject(new SplitShardAPI(collectionsHandler));
- apiBag.registerObject(new CreateShardAPI(collectionsHandler));
- apiBag.registerObject(new AddReplicaAPI(collectionsHandler));
- apiBag.registerObject(new DeleteShardAPI(collectionsHandler));
- apiBag.registerObject(new SyncShardAPI(collectionsHandler));
- apiBag.registerObject(new ForceLeaderAPI(collectionsHandler));
- // really this is a replica API, but since there's only 1 API on the replica path, it's included
- // here for simplicity.
- apiBag.registerObject(new DeleteReplicaAPI(collectionsHandler));
- }
-}
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/TestApiFramework.java b/solr/core/src/test/org/apache/solr/handler/admin/TestApiFramework.java
index e586151454e..1a0f37312ae 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/TestApiFramework.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/TestApiFramework.java
@@ -61,7 +61,6 @@ import org.apache.solr.handler.CollectionsAPI;
import org.apache.solr.handler.PingRequestHandler;
import org.apache.solr.handler.SchemaHandler;
import org.apache.solr.handler.SolrConfigHandler;
-import org.apache.solr.handler.api.ApiRegistrar;
import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.request.SolrQueryRequestBase;
@@ -81,8 +80,9 @@ public class TestApiFramework extends SolrTestCaseJ4 {
new TestCollectionAPIs.MockCollectionsHandler();
containerHandlers.put(COLLECTIONS_HANDLER_PATH, collectionsHandler);
containerHandlers.getApiBag().registerObject(new CollectionsAPI(collectionsHandler));
- ApiRegistrar.registerCollectionApis(containerHandlers.getApiBag(), collectionsHandler);
- ApiRegistrar.registerShardApis(containerHandlers.getApiBag(), collectionsHandler);
+ for (Api api : collectionsHandler.getApis()) {
+ containerHandlers.getApiBag().register(api);
+ }
containerHandlers.put(CORES_HANDLER_PATH, new CoreAdminHandler(mockCC));
containerHandlers.put(CONFIGSETS_HANDLER_PATH, new ConfigSetsHandler(mockCC));
out.put("getRequestHandlers", containerHandlers);
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java b/solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java
index bee88cfda84..97a09a7816a 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java
@@ -47,7 +47,6 @@ import org.apache.solr.common.util.Utils;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.handler.ClusterAPI;
import org.apache.solr.handler.CollectionsAPI;
-import org.apache.solr.handler.api.ApiRegistrar;
import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.response.SolrQueryResponse;
@@ -85,8 +84,9 @@ public class TestCollectionAPIs extends SolrTestCaseJ4 {
final CollectionsAPI collectionsAPI = new CollectionsAPI(collectionsHandler);
apiBag.registerObject(new CollectionsAPI(collectionsHandler));
apiBag.registerObject(collectionsAPI.collectionsCommands);
- ApiRegistrar.registerCollectionApis(apiBag, collectionsHandler);
- ApiRegistrar.registerShardApis(apiBag, collectionsHandler);
+ for (Api api : collectionsHandler.getApis()) {
+ apiBag.register(api);
+ }
ClusterAPI clusterAPI = new ClusterAPI(collectionsHandler, null);
apiBag.registerObject(clusterAPI);
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/api/V2CollectionAPIMappingTest.java b/solr/core/src/test/org/apache/solr/handler/admin/api/V2CollectionAPIMappingTest.java
index 7724f530588..660d96617eb 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/api/V2CollectionAPIMappingTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/api/V2CollectionAPIMappingTest.java
@@ -31,7 +31,6 @@ import org.apache.solr.common.params.SolrParams;
import org.apache.solr.handler.admin.CollectionsHandler;
import org.apache.solr.handler.admin.TestCollectionAPIs;
import org.apache.solr.handler.admin.V2ApiMappingTest;
-import org.apache.solr.handler.api.ApiRegistrar;
import org.junit.Test;
/**
@@ -52,7 +51,18 @@ import org.junit.Test;
public class V2CollectionAPIMappingTest extends V2ApiMappingTest<CollectionsHandler> {
@Override
public void populateApiBag() {
- ApiRegistrar.registerCollectionApis(apiBag, getRequestHandler());
+ final CollectionsHandler collectionsHandler = getRequestHandler();
+ apiBag.registerObject(new AddReplicaPropertyAPI(collectionsHandler));
+ apiBag.registerObject(new BalanceShardUniqueAPI(collectionsHandler));
+ apiBag.registerObject(new DeleteCollectionAPI(collectionsHandler));
+ apiBag.registerObject(new DeleteReplicaPropertyAPI(collectionsHandler));
+ apiBag.registerObject(new MigrateDocsAPI(collectionsHandler));
+ apiBag.registerObject(new ModifyCollectionAPI(collectionsHandler));
+ apiBag.registerObject(new MoveReplicaAPI(collectionsHandler));
+ apiBag.registerObject(new RebalanceLeadersAPI(collectionsHandler));
+ apiBag.registerObject(new ReloadCollectionAPI(collectionsHandler));
+ apiBag.registerObject(new SetCollectionPropertyAPI(collectionsHandler));
+ apiBag.registerObject(new CollectionStatusAPI(collectionsHandler));
}
@Override
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/api/V2ShardsAPIMappingTest.java b/solr/core/src/test/org/apache/solr/handler/admin/api/V2ShardsAPIMappingTest.java
index 36140d307a4..e555393db62 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/api/V2ShardsAPIMappingTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/api/V2ShardsAPIMappingTest.java
@@ -54,7 +54,6 @@ import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.handler.admin.CollectionsHandler;
import org.apache.solr.handler.admin.V2ApiMappingTest;
-import org.apache.solr.handler.api.ApiRegistrar;
import org.junit.Test;
/**
@@ -70,7 +69,14 @@ public class V2ShardsAPIMappingTest extends V2ApiMappingTest<CollectionsHandler>
@Override
public void populateApiBag() {
- ApiRegistrar.registerShardApis(apiBag, getRequestHandler());
+ final CollectionsHandler collectionsHandler = getRequestHandler();
+ apiBag.registerObject(new SplitShardAPI(collectionsHandler));
+ apiBag.registerObject(new CreateShardAPI(collectionsHandler));
+ apiBag.registerObject(new AddReplicaAPI(collectionsHandler));
+ apiBag.registerObject(new DeleteShardAPI(collectionsHandler));
+ apiBag.registerObject(new SyncShardAPI(collectionsHandler));
+ apiBag.registerObject(new ForceLeaderAPI(collectionsHandler));
+ apiBag.registerObject(new DeleteReplicaAPI(collectionsHandler));
}
@Override