You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/07/28 17:58:50 UTC
[GitHub] [solr] gerlowskija opened a new pull request, #955: SOLR-15736: Move configset logic into v2 APis
gerlowskija opened a new pull request, #955:
URL: https://github.com/apache/solr/pull/955
# Description
Prior to this commit, the logic for all configset APIs lived in ConfigSetsHandler. This made the file slightly unwieldy, and made it easy for the v1 APIs to change without updates to their v2 counterparts.
# Solution
This PR moves the logic for each configset endpoint into its own (v2 annotated) API class. These API classes are then called from ConfigSetsHandler. This ensures that the v2 API is kept abreast of any changes in the configset logic.
# Tests
Mostly leans on the existing TestConfigSetsAPI test, which has surprisingly solid coverage of both the v1 and v2 APIs.
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `main` branch.
- [ ] I have run `./gradlew check`.
- [x] I have added tests for my changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #955:
URL: https://github.com/apache/solr/pull/955#discussion_r937085915
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -801,7 +815,11 @@ public void load() {
ClusterAPI clusterAPI = new ClusterAPI(collectionsHandler, configSetsHandler);
containerHandlers.getApiBag().registerObject(clusterAPI);
containerHandlers.getApiBag().registerObject(clusterAPI.commands);
- containerHandlers.getApiBag().registerObject(clusterAPI.configSetCommands);
+ containerHandlers.getApiBag().registerObject(new CreateConfigSetAPI(this));
+ containerHandlers.getApiBag().registerObject(new DeleteConfigSetAPI(this));
+ containerHandlers.getApiBag().registerObject(new ListConfigSetsAPI(this));
+ containerHandlers.getApiBag().registerObject(new UploadConfigSetAPI(this));
+ containerHandlers.getApiBag().registerObject(new UploadConfigSetFileAPI(this));
Review Comment:
I've moved these out to ApiRegistrar, which at least gives us some short-term improvement here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija merged pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
gerlowskija merged PR #955:
URL: https://github.com/apache/solr/pull/955
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #955:
URL: https://github.com/apache/solr/pull/955#discussion_r935971223
##########
solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSetAPI.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * 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.configsets;
+
+
+import org.apache.commons.io.IOUtils;
+import org.apache.solr.api.EndPoint;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.ConfigSetParams;
+import org.apache.solr.core.ConfigSetService;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipInputStream;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.PUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
+
+/**
+ * V2 API for uploading a new configset (or overwriting an existing one).
+ *
+ * <p>This API (PUT /v2/cluster/configs/configsetName) is analogous to the v1
+ * /admin/configs?action=UPLOAD command.
+ *
+ */
+public class UploadConfigSetAPI extends ConfigSetAPIBase {
+
+ public static final String CONFIGSET_NAME_PLACEHOLDER = "name";
+
+
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ public UploadConfigSetAPI(CoreContainer coreContainer) {
+ super(coreContainer);
+ }
+
+ @EndPoint(method = PUT, path = "/cluster/configs/{name}", permission = CONFIG_EDIT_PERM)
+ public void uploadConfigSet(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+ ensureConfigSetUploadEnabled();
+
+ final String configSetName = req.getPathTemplateValues().get("name");
+ boolean overwritesExisting = configSetService.checkConfigExists(configSetName);
+ boolean requestIsTrusted = isTrusted(req.getUserPrincipal(), coreContainer.getAuthenticationPlugin());
+ // Get upload parameters
+ boolean allowOverwrite = req.getParams().getBool(ConfigSetParams.OVERWRITE, true);
+ boolean cleanup = req.getParams().getBool(ConfigSetParams.CLEANUP, false);
+ final InputStream inputStream = ensureNonEmptyInputStream(req);
+
+ if (overwritesExisting && !allowOverwrite) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The configuration " + configSetName + " already exists in zookeeper");
Review Comment:
👍 - I think that string-literal came from ConfigsetHandler, but I'll amend it since it's a quick fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #955:
URL: https://github.com/apache/solr/pull/955#discussion_r936982031
##########
solr/core/src/java/org/apache/solr/handler/configsets/CreateConfigSetAPI.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.configsets;
+
+import com.google.common.collect.Maps;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.solr.api.Command;
+import org.apache.solr.api.EndPoint;
+import org.apache.solr.api.PayloadObj;
+import org.apache.solr.client.solrj.request.beans.CreateConfigPayload;
+import org.apache.solr.cloud.ConfigSetCmds;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.ConfigSetParams;
+import org.apache.solr.core.CoreContainer;
+
+import java.util.Map;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
+import static org.apache.solr.common.params.CommonParams.NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DEFAULT_CONFIGSET_NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DISABLE_CREATE_AUTH_CHECKS;
+import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
+
+/**
+ * V2 API for creating a new configset as a copy of an existing one.
+ *
+ * <p>This API (POST /v2/cluster/configs {"create": {...}}) is analogous to
+ * the v1 /admin/configs?action=CREATE command.
+ *
+ */
+@EndPoint(method = POST, path = "/cluster/configs", permission = CONFIG_EDIT_PERM)
+public class CreateConfigSetAPI extends ConfigSetAPIBase {
+
+ public CreateConfigSetAPI(CoreContainer coreContainer) {
+ super(coreContainer);
+ }
+
+ @Command(name = "create")
+ @SuppressWarnings("unchecked")
+ public void create(PayloadObj<CreateConfigPayload> obj) throws Exception {
+ final CreateConfigPayload createConfigPayload = obj.get();
+ final String baseConfigSetName = (createConfigPayload.baseConfigSet != null) ? createConfigPayload.baseConfigSet : DEFAULT_CONFIGSET_NAME;
+ if (configSetService.checkConfigExists(createConfigPayload.name)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "ConfigSet already exists: " + createConfigPayload.name);
+ }
+
+ // is there a base config that already exists
+ if (! configSetService.checkConfigExists(baseConfigSetName)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Base ConfigSet does not exist: " + baseConfigSetName);
+ }
+
+ if (!DISABLE_CREATE_AUTH_CHECKS
+ && !isTrusted(obj.getRequest().getUserPrincipal(), coreContainer.getAuthenticationPlugin())
+ && isCurrentlyTrusted(baseConfigSetName)) {
+ throw new SolrException(
+ SolrException.ErrorCode.UNAUTHORIZED,
+ "Can't create a configset with an unauthenticated request from a trusted "
+ + ConfigSetCmds.BASE_CONFIGSET);
+ }
+
+ final Map<String, Object> configsetCommandMsg = Maps.newHashMap();
Review Comment:
Definitely for another issue :-). I'm glad to hear you agree with the ideas.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #955:
URL: https://github.com/apache/solr/pull/955#discussion_r935970495
##########
solr/core/src/java/org/apache/solr/handler/configsets/CreateConfigSetAPI.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.configsets;
+
+import com.google.common.collect.Maps;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.solr.api.Command;
+import org.apache.solr.api.EndPoint;
+import org.apache.solr.api.PayloadObj;
+import org.apache.solr.client.solrj.request.beans.CreateConfigPayload;
+import org.apache.solr.cloud.ConfigSetCmds;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.ConfigSetParams;
+import org.apache.solr.core.CoreContainer;
+
+import java.util.Map;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
+import static org.apache.solr.common.params.CommonParams.NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DEFAULT_CONFIGSET_NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DISABLE_CREATE_AUTH_CHECKS;
+import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
+
+/**
+ * V2 API for creating a new configset as a copy of an existing one.
+ *
+ * <p>This API (POST /v2/cluster/configs {"create": {...}}) is analogous to
+ * the v1 /admin/configs?action=CREATE command.
+ *
+ */
+@EndPoint(method = POST, path = "/cluster/configs", permission = CONFIG_EDIT_PERM)
+public class CreateConfigSetAPI extends ConfigSetAPIBase {
+
+ public CreateConfigSetAPI(CoreContainer coreContainer) {
+ super(coreContainer);
+ }
+
+ @Command(name = "create")
+ @SuppressWarnings("unchecked")
+ public void create(PayloadObj<CreateConfigPayload> obj) throws Exception {
+ final CreateConfigPayload createConfigPayload = obj.get();
+ final String baseConfigSetName = (createConfigPayload.baseConfigSet != null) ? createConfigPayload.baseConfigSet : DEFAULT_CONFIGSET_NAME;
+ if (configSetService.checkConfigExists(createConfigPayload.name)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "ConfigSet already exists: " + createConfigPayload.name);
+ }
+
+ // is there a base config that already exists
+ if (! configSetService.checkConfigExists(baseConfigSetName)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Base ConfigSet does not exist: " + baseConfigSetName);
+ }
+
+ if (!DISABLE_CREATE_AUTH_CHECKS
+ && !isTrusted(obj.getRequest().getUserPrincipal(), coreContainer.getAuthenticationPlugin())
+ && isCurrentlyTrusted(baseConfigSetName)) {
+ throw new SolrException(
+ SolrException.ErrorCode.UNAUTHORIZED,
+ "Can't create a configset with an unauthenticated request from a trusted "
+ + ConfigSetCmds.BASE_CONFIGSET);
+ }
+
+ final Map<String, Object> configsetCommandMsg = Maps.newHashMap();
Review Comment:
I agree on both counts: (1) I'm not sure whether we really need the overseer to do this work, and (2) if we do it'd be awesome to send those messages as strongly typed objects. No reason to keep dealing with everything as a map.
I'm leery of getting into that with this PR or effort though - the v2 API effort is already daunting without stopping to fix or reimplement the guts of individual APIs, no matter how much they deserve it. (And this one does for sure)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #955:
URL: https://github.com/apache/solr/pull/955#discussion_r937068437
##########
solr/core/src/java/org/apache/solr/handler/configsets/CreateConfigSetAPI.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.configsets;
+
+import com.google.common.collect.Maps;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.solr.api.Command;
+import org.apache.solr.api.EndPoint;
+import org.apache.solr.api.PayloadObj;
+import org.apache.solr.client.solrj.request.beans.CreateConfigPayload;
+import org.apache.solr.cloud.ConfigSetCmds;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.ConfigSetParams;
+import org.apache.solr.core.CoreContainer;
+
+import java.util.Map;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
+import static org.apache.solr.common.params.CommonParams.NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DEFAULT_CONFIGSET_NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DISABLE_CREATE_AUTH_CHECKS;
+import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
+
+/**
+ * V2 API for creating a new configset as a copy of an existing one.
+ *
+ * <p>This API (POST /v2/cluster/configs {"create": {...}}) is analogous to
+ * the v1 /admin/configs?action=CREATE command.
+ *
+ */
+@EndPoint(method = POST, path = "/cluster/configs", permission = CONFIG_EDIT_PERM)
+public class CreateConfigSetAPI extends ConfigSetAPIBase {
+
+ public CreateConfigSetAPI(CoreContainer coreContainer) {
+ super(coreContainer);
+ }
+
+ @Command(name = "create")
+ @SuppressWarnings("unchecked")
+ public void create(PayloadObj<CreateConfigPayload> obj) throws Exception {
+ final CreateConfigPayload createConfigPayload = obj.get();
+ final String baseConfigSetName = (createConfigPayload.baseConfigSet != null) ? createConfigPayload.baseConfigSet : DEFAULT_CONFIGSET_NAME;
+ if (configSetService.checkConfigExists(createConfigPayload.name)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "ConfigSet already exists: " + createConfigPayload.name);
+ }
+
+ // is there a base config that already exists
+ if (! configSetService.checkConfigExists(baseConfigSetName)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Base ConfigSet does not exist: " + baseConfigSetName);
+ }
+
+ if (!DISABLE_CREATE_AUTH_CHECKS
+ && !isTrusted(obj.getRequest().getUserPrincipal(), coreContainer.getAuthenticationPlugin())
+ && isCurrentlyTrusted(baseConfigSetName)) {
+ throw new SolrException(
+ SolrException.ErrorCode.UNAUTHORIZED,
+ "Can't create a configset with an unauthenticated request from a trusted "
+ + ConfigSetCmds.BASE_CONFIGSET);
+ }
+
+ final Map<String, Object> configsetCommandMsg = Maps.newHashMap();
Review Comment:
I personally think avoiding the generic `<>` makes for cleaner syntax, but to each their own. Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #955:
URL: https://github.com/apache/solr/pull/955#discussion_r935104951
##########
solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSetAPI.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * 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.configsets;
+
+
+import org.apache.commons.io.IOUtils;
+import org.apache.solr.api.EndPoint;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.ConfigSetParams;
+import org.apache.solr.core.ConfigSetService;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipInputStream;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.PUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
+
+/**
+ * V2 API for uploading a new configset (or overwriting an existing one).
+ *
+ * <p>This API (PUT /v2/cluster/configs/configsetName) is analogous to the v1
+ * /admin/configs?action=UPLOAD command.
+ *
+ */
+public class UploadConfigSetAPI extends ConfigSetAPIBase {
+
+ public static final String CONFIGSET_NAME_PLACEHOLDER = "name";
+
+
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ public UploadConfigSetAPI(CoreContainer coreContainer) {
+ super(coreContainer);
+ }
+
+ @EndPoint(method = PUT, path = "/cluster/configs/{name}", permission = CONFIG_EDIT_PERM)
+ public void uploadConfigSet(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+ ensureConfigSetUploadEnabled();
+
+ final String configSetName = req.getPathTemplateValues().get("name");
+ boolean overwritesExisting = configSetService.checkConfigExists(configSetName);
+ boolean requestIsTrusted = isTrusted(req.getUserPrincipal(), coreContainer.getAuthenticationPlugin());
+ // Get upload parameters
+ boolean allowOverwrite = req.getParams().getBool(ConfigSetParams.OVERWRITE, true);
+ boolean cleanup = req.getParams().getBool(ConfigSetParams.CLEANUP, false);
+ final InputStream inputStream = ensureNonEmptyInputStream(req);
+
+ if (overwritesExisting && !allowOverwrite) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The configuration " + configSetName + " already exists in zookeeper");
Review Comment:
No need to reference ZooKeeper in error messages relating to configSets. ConfigSetService is pluggable and the backend is more of a detail.
##########
solr/core/src/java/org/apache/solr/handler/configsets/CreateConfigSetAPI.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.configsets;
+
+import com.google.common.collect.Maps;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.solr.api.Command;
+import org.apache.solr.api.EndPoint;
+import org.apache.solr.api.PayloadObj;
+import org.apache.solr.client.solrj.request.beans.CreateConfigPayload;
+import org.apache.solr.cloud.ConfigSetCmds;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.ConfigSetParams;
+import org.apache.solr.core.CoreContainer;
+
+import java.util.Map;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
+import static org.apache.solr.common.params.CommonParams.NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DEFAULT_CONFIGSET_NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DISABLE_CREATE_AUTH_CHECKS;
+import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
+
+/**
+ * V2 API for creating a new configset as a copy of an existing one.
+ *
+ * <p>This API (POST /v2/cluster/configs {"create": {...}}) is analogous to
+ * the v1 /admin/configs?action=CREATE command.
+ *
+ */
+@EndPoint(method = POST, path = "/cluster/configs", permission = CONFIG_EDIT_PERM)
+public class CreateConfigSetAPI extends ConfigSetAPIBase {
+
+ public CreateConfigSetAPI(CoreContainer coreContainer) {
+ super(coreContainer);
+ }
+
+ @Command(name = "create")
+ @SuppressWarnings("unchecked")
+ public void create(PayloadObj<CreateConfigPayload> obj) throws Exception {
+ final CreateConfigPayload createConfigPayload = obj.get();
+ final String baseConfigSetName = (createConfigPayload.baseConfigSet != null) ? createConfigPayload.baseConfigSet : DEFAULT_CONFIGSET_NAME;
+ if (configSetService.checkConfigExists(createConfigPayload.name)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "ConfigSet already exists: " + createConfigPayload.name);
+ }
+
+ // is there a base config that already exists
+ if (! configSetService.checkConfigExists(baseConfigSetName)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Base ConfigSet does not exist: " + baseConfigSetName);
+ }
+
+ if (!DISABLE_CREATE_AUTH_CHECKS
+ && !isTrusted(obj.getRequest().getUserPrincipal(), coreContainer.getAuthenticationPlugin())
+ && isCurrentlyTrusted(baseConfigSetName)) {
+ throw new SolrException(
+ SolrException.ErrorCode.UNAUTHORIZED,
+ "Can't create a configset with an unauthenticated request from a trusted "
+ + ConfigSetCmds.BASE_CONFIGSET);
+ }
+
+ final Map<String, Object> configsetCommandMsg = Maps.newHashMap();
Review Comment:
I find it very sad that configsetCommandMsg needs to exist, and must be translated from CreateConfigPayload. Wouldn't it be great if we could send "ReflectMapWriter" stuff directly to runConfigSetCommand? and get the same POJO (CreateConfigPayload) back on the other end?
##########
solr/core/src/java/org/apache/solr/handler/configsets/CreateConfigSetAPI.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.configsets;
+
+import com.google.common.collect.Maps;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.solr.api.Command;
+import org.apache.solr.api.EndPoint;
+import org.apache.solr.api.PayloadObj;
+import org.apache.solr.client.solrj.request.beans.CreateConfigPayload;
+import org.apache.solr.cloud.ConfigSetCmds;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.ConfigSetParams;
+import org.apache.solr.core.CoreContainer;
+
+import java.util.Map;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
+import static org.apache.solr.common.params.CommonParams.NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DEFAULT_CONFIGSET_NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DISABLE_CREATE_AUTH_CHECKS;
+import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
+
+/**
+ * V2 API for creating a new configset as a copy of an existing one.
+ *
+ * <p>This API (POST /v2/cluster/configs {"create": {...}}) is analogous to
+ * the v1 /admin/configs?action=CREATE command.
+ *
+ */
+@EndPoint(method = POST, path = "/cluster/configs", permission = CONFIG_EDIT_PERM)
+public class CreateConfigSetAPI extends ConfigSetAPIBase {
+
+ public CreateConfigSetAPI(CoreContainer coreContainer) {
+ super(coreContainer);
+ }
+
+ @Command(name = "create")
+ @SuppressWarnings("unchecked")
+ public void create(PayloadObj<CreateConfigPayload> obj) throws Exception {
+ final CreateConfigPayload createConfigPayload = obj.get();
+ final String baseConfigSetName = (createConfigPayload.baseConfigSet != null) ? createConfigPayload.baseConfigSet : DEFAULT_CONFIGSET_NAME;
+ if (configSetService.checkConfigExists(createConfigPayload.name)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "ConfigSet already exists: " + createConfigPayload.name);
+ }
+
+ // is there a base config that already exists
+ if (! configSetService.checkConfigExists(baseConfigSetName)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Base ConfigSet does not exist: " + baseConfigSetName);
+ }
+
+ if (!DISABLE_CREATE_AUTH_CHECKS
+ && !isTrusted(obj.getRequest().getUserPrincipal(), coreContainer.getAuthenticationPlugin())
+ && isCurrentlyTrusted(baseConfigSetName)) {
+ throw new SolrException(
+ SolrException.ErrorCode.UNAUTHORIZED,
+ "Can't create a configset with an unauthenticated request from a trusted "
+ + ConfigSetCmds.BASE_CONFIGSET);
+ }
+
+ final Map<String, Object> configsetCommandMsg = Maps.newHashMap();
Review Comment:
On further reflection, the fact that this is communicated to the Overseer at all instead of executed directly is silly (I think).
If as a project we forgo use of the Overseer for some things like this as I indicate in this comment https://issues.apache.org/jira/browse/SOLR-15146?focusedCommentId=17574033&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17574033 it could lead to wonderful simplifications in SolrCloud code.
##########
solr/core/src/java/org/apache/solr/handler/configsets/CreateConfigSetAPI.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.configsets;
+
+import com.google.common.collect.Maps;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.solr.api.Command;
+import org.apache.solr.api.EndPoint;
+import org.apache.solr.api.PayloadObj;
+import org.apache.solr.client.solrj.request.beans.CreateConfigPayload;
+import org.apache.solr.cloud.ConfigSetCmds;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.ConfigSetParams;
+import org.apache.solr.core.CoreContainer;
+
+import java.util.Map;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
+import static org.apache.solr.common.params.CommonParams.NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DEFAULT_CONFIGSET_NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DISABLE_CREATE_AUTH_CHECKS;
+import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
+
+/**
+ * V2 API for creating a new configset as a copy of an existing one.
+ *
+ * <p>This API (POST /v2/cluster/configs {"create": {...}}) is analogous to
+ * the v1 /admin/configs?action=CREATE command.
+ *
+ */
+@EndPoint(method = POST, path = "/cluster/configs", permission = CONFIG_EDIT_PERM)
+public class CreateConfigSetAPI extends ConfigSetAPIBase {
+
+ public CreateConfigSetAPI(CoreContainer coreContainer) {
+ super(coreContainer);
+ }
+
+ @Command(name = "create")
+ @SuppressWarnings("unchecked")
+ public void create(PayloadObj<CreateConfigPayload> obj) throws Exception {
+ final CreateConfigPayload createConfigPayload = obj.get();
+ final String baseConfigSetName = (createConfigPayload.baseConfigSet != null) ? createConfigPayload.baseConfigSet : DEFAULT_CONFIGSET_NAME;
Review Comment:
Maybe this default belongs in CreateConfigPayload where it will be self-documenting as a default and keeps the logic here simpler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #955:
URL: https://github.com/apache/solr/pull/955#discussion_r937085439
##########
solr/core/src/java/org/apache/solr/handler/configsets/CreateConfigSetAPI.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.configsets;
+
+import com.google.common.collect.Maps;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.solr.api.Command;
+import org.apache.solr.api.EndPoint;
+import org.apache.solr.api.PayloadObj;
+import org.apache.solr.client.solrj.request.beans.CreateConfigPayload;
+import org.apache.solr.cloud.ConfigSetCmds;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.ConfigSetParams;
+import org.apache.solr.core.CoreContainer;
+
+import java.util.Map;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
+import static org.apache.solr.common.params.CommonParams.NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DEFAULT_CONFIGSET_NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DISABLE_CREATE_AUTH_CHECKS;
+import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
+
+/**
+ * V2 API for creating a new configset as a copy of an existing one.
+ *
+ * <p>This API (POST /v2/cluster/configs {"create": {...}}) is analogous to
+ * the v1 /admin/configs?action=CREATE command.
+ *
+ */
+@EndPoint(method = POST, path = "/cluster/configs", permission = CONFIG_EDIT_PERM)
+public class CreateConfigSetAPI extends ConfigSetAPIBase {
+
+ public CreateConfigSetAPI(CoreContainer coreContainer) {
+ super(coreContainer);
+ }
+
+ @Command(name = "create")
+ @SuppressWarnings("unchecked")
+ public void create(PayloadObj<CreateConfigPayload> obj) throws Exception {
+ final CreateConfigPayload createConfigPayload = obj.get();
+ final String baseConfigSetName = (createConfigPayload.baseConfigSet != null) ? createConfigPayload.baseConfigSet : DEFAULT_CONFIGSET_NAME;
Review Comment:
Done. (Though it ended up not being as big of a win as it initially looked: setting the default in CreateConfigPayload means that we need an empty/null check in ConfigSetsHandler to prevent us from accidentally overwriting it. But the self-documenting aspect is still a "win" for sure!)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #955:
URL: https://github.com/apache/solr/pull/955#discussion_r935952977
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -801,7 +815,11 @@ public void load() {
ClusterAPI clusterAPI = new ClusterAPI(collectionsHandler, configSetsHandler);
containerHandlers.getApiBag().registerObject(clusterAPI);
containerHandlers.getApiBag().registerObject(clusterAPI.commands);
- containerHandlers.getApiBag().registerObject(clusterAPI.configSetCommands);
+ containerHandlers.getApiBag().registerObject(new CreateConfigSetAPI(this));
+ containerHandlers.getApiBag().registerObject(new DeleteConfigSetAPI(this));
+ containerHandlers.getApiBag().registerObject(new ListConfigSetsAPI(this));
+ containerHandlers.getApiBag().registerObject(new UploadConfigSetAPI(this));
+ containerHandlers.getApiBag().registerObject(new UploadConfigSetFileAPI(this));
Review Comment:
Yeah, it's not ideal for sure. Core-level APIs are registered out of solrconfig.xml's, so they don't have the same downside. It's just the Solr/container-level APIs. But still, it won't scale out as we split up more APIs.
I did create an ApiRegistrar class to group things together, so that CoreContainer can register whole groups of APIs with a single call. So maybe I'll go that route with these APIs in the short term. I should've done that out of the gate, my mistake.
As for an annotation-discovery framework - I'd love to use an existing one. But IMO there are too many good options out there to justify writing our own. I've proposed JAX-RS in the past as a fix for a wider set of problems but the idea had lukewarm reception, but maybe opinion will have changed on that, or maybe there's some lighter alternative to solve this problem only.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
epugh commented on PR #955:
URL: https://github.com/apache/solr/pull/955#issuecomment-1203203304
Thanks for jumping on Zoom and walking me through this @gerlowskija today ;-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] madrob commented on a diff in pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
madrob commented on code in PR #955:
URL: https://github.com/apache/solr/pull/955#discussion_r934745731
##########
solr/core/src/test/org/apache/solr/handler/admin/TestConfigsApi.java:
##########
@@ -17,19 +17,11 @@
package org.apache.solr.handler.admin;
-import static org.apache.solr.client.solrj.SolrRequest.METHOD.DELETE;
-import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
-import static org.apache.solr.handler.admin.TestCollectionAPIs.compareOutput;
-
-import java.util.Map;
import org.apache.solr.SolrTestCaseJ4;
-import org.apache.solr.api.ApiBag;
-import org.apache.solr.common.cloud.ZkNodeProps;
-import org.apache.solr.handler.ClusterAPI;
-import org.apache.solr.response.SolrQueryResponse;
public class TestConfigsApi extends SolrTestCaseJ4 {
+ /*
Review Comment:
Are there any tests left in this class?
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -801,7 +815,11 @@ public void load() {
ClusterAPI clusterAPI = new ClusterAPI(collectionsHandler, configSetsHandler);
containerHandlers.getApiBag().registerObject(clusterAPI);
containerHandlers.getApiBag().registerObject(clusterAPI.commands);
- containerHandlers.getApiBag().registerObject(clusterAPI.configSetCommands);
+ containerHandlers.getApiBag().registerObject(new CreateConfigSetAPI(this));
+ containerHandlers.getApiBag().registerObject(new DeleteConfigSetAPI(this));
+ containerHandlers.getApiBag().registerObject(new ListConfigSetsAPI(this));
+ containerHandlers.getApiBag().registerObject(new UploadConfigSetAPI(this));
+ containerHandlers.getApiBag().registerObject(new UploadConfigSetFileAPI(this));
Review Comment:
This is fine for now, but I think we're going to want a better approach for it, as we convert more APIs I suspect the list will get long and unwieldy and easy to miss something or make mistakes. Maybe an annotation based discovery framework, hopefully that won't make startup time too long.
##########
solr/core/src/java/org/apache/solr/request/DelegatingSolrQueryRequest.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * 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.request;
+
+import io.opentracing.Span;
+import io.opentracing.Tracer;
+import org.apache.solr.cloud.CloudDescriptor;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.CommandOperation;
+import org.apache.solr.common.util.ContentStream;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.servlet.HttpSolrCall;
+import org.apache.solr.util.RTimerTree;
+
+import java.security.Principal;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A {@link SolrQueryRequest} implementation that defers to a delegate in all cases.
+ *
+ * Used primarily in cases where developers want to customize one or more SolrQueryRequest methods while deferring the
+ * remainder to an existing instances.
+ */
+public class DelegatingSolrQueryRequest implements SolrQueryRequest {
Review Comment:
This feels like something that should be in SolrJ
##########
solr/core/src/java/org/apache/solr/handler/configsets/CreateConfigSetAPI.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.configsets;
+
+import com.google.common.collect.Maps;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.solr.api.Command;
+import org.apache.solr.api.EndPoint;
+import org.apache.solr.api.PayloadObj;
+import org.apache.solr.client.solrj.request.beans.CreateConfigPayload;
+import org.apache.solr.cloud.ConfigSetCmds;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.ConfigSetParams;
+import org.apache.solr.core.CoreContainer;
+
+import java.util.Map;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
+import static org.apache.solr.common.params.CommonParams.NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DEFAULT_CONFIGSET_NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DISABLE_CREATE_AUTH_CHECKS;
+import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
+
+/**
+ * V2 API for creating a new configset as a copy of an existing one.
+ *
+ * <p>This API (POST /v2/cluster/configs {"create": {...}}) is analogous to
+ * the v1 /admin/configs?action=CREATE command.
+ *
+ */
+@EndPoint(method = POST, path = "/cluster/configs", permission = CONFIG_EDIT_PERM)
+public class CreateConfigSetAPI extends ConfigSetAPIBase {
+
+ public CreateConfigSetAPI(CoreContainer coreContainer) {
+ super(coreContainer);
+ }
+
+ @Command(name = "create")
+ @SuppressWarnings("unchecked")
Review Comment:
Do we have to have this?
##########
solr/core/src/java/org/apache/solr/handler/configsets/CreateConfigSetAPI.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.configsets;
+
+import com.google.common.collect.Maps;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.solr.api.Command;
+import org.apache.solr.api.EndPoint;
+import org.apache.solr.api.PayloadObj;
+import org.apache.solr.client.solrj.request.beans.CreateConfigPayload;
+import org.apache.solr.cloud.ConfigSetCmds;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.ConfigSetParams;
+import org.apache.solr.core.CoreContainer;
+
+import java.util.Map;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
+import static org.apache.solr.common.params.CommonParams.NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DEFAULT_CONFIGSET_NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DISABLE_CREATE_AUTH_CHECKS;
+import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
+
+/**
+ * V2 API for creating a new configset as a copy of an existing one.
+ *
+ * <p>This API (POST /v2/cluster/configs {"create": {...}}) is analogous to
+ * the v1 /admin/configs?action=CREATE command.
+ *
+ */
+@EndPoint(method = POST, path = "/cluster/configs", permission = CONFIG_EDIT_PERM)
+public class CreateConfigSetAPI extends ConfigSetAPIBase {
+
+ public CreateConfigSetAPI(CoreContainer coreContainer) {
+ super(coreContainer);
+ }
+
+ @Command(name = "create")
+ @SuppressWarnings("unchecked")
+ public void create(PayloadObj<CreateConfigPayload> obj) throws Exception {
+ final CreateConfigPayload createConfigPayload = obj.get();
+ final String baseConfigSetName = (createConfigPayload.baseConfigSet != null) ? createConfigPayload.baseConfigSet : DEFAULT_CONFIGSET_NAME;
+ if (configSetService.checkConfigExists(createConfigPayload.name)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "ConfigSet already exists: " + createConfigPayload.name);
+ }
+
+ // is there a base config that already exists
+ if (! configSetService.checkConfigExists(baseConfigSetName)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Base ConfigSet does not exist: " + baseConfigSetName);
+ }
+
+ if (!DISABLE_CREATE_AUTH_CHECKS
+ && !isTrusted(obj.getRequest().getUserPrincipal(), coreContainer.getAuthenticationPlugin())
+ && isCurrentlyTrusted(baseConfigSetName)) {
+ throw new SolrException(
+ SolrException.ErrorCode.UNAUTHORIZED,
+ "Can't create a configset with an unauthenticated request from a trusted "
+ + ConfigSetCmds.BASE_CONFIGSET);
+ }
+
+ final Map<String, Object> configsetCommandMsg = Maps.newHashMap();
Review Comment:
`new HashMap<>()` is simpler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #955:
URL: https://github.com/apache/solr/pull/955#discussion_r935956337
##########
solr/core/src/java/org/apache/solr/request/DelegatingSolrQueryRequest.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * 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.request;
+
+import io.opentracing.Span;
+import io.opentracing.Tracer;
+import org.apache.solr.cloud.CloudDescriptor;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.CommandOperation;
+import org.apache.solr.common.util.ContentStream;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.servlet.HttpSolrCall;
+import org.apache.solr.util.RTimerTree;
+
+import java.security.Principal;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A {@link SolrQueryRequest} implementation that defers to a delegate in all cases.
+ *
+ * Used primarily in cases where developers want to customize one or more SolrQueryRequest methods while deferring the
+ * remainder to an existing instances.
+ */
+public class DelegatingSolrQueryRequest implements SolrQueryRequest {
Review Comment:
I agree. I actually went to put it there initially until I realized that `SolrQueryRequest` is a solr-core class. SolrJ can't reference it!
I think I was conflating it with the conceptually similar `SolrRequest`.
##########
solr/core/src/java/org/apache/solr/request/DelegatingSolrQueryRequest.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * 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.request;
+
+import io.opentracing.Span;
+import io.opentracing.Tracer;
+import org.apache.solr.cloud.CloudDescriptor;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.CommandOperation;
+import org.apache.solr.common.util.ContentStream;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.servlet.HttpSolrCall;
+import org.apache.solr.util.RTimerTree;
+
+import java.security.Principal;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A {@link SolrQueryRequest} implementation that defers to a delegate in all cases.
+ *
+ * Used primarily in cases where developers want to customize one or more SolrQueryRequest methods while deferring the
+ * remainder to an existing instances.
+ */
+public class DelegatingSolrQueryRequest implements SolrQueryRequest {
Review Comment:
I agree. I actually went to put it there initially until I realized that `SolrQueryRequest` is a solr-core class. SolrJ can't reference it!
I think I was conflating it with the conceptually similar `SolrRequest` that SolrJ has.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #955:
URL: https://github.com/apache/solr/pull/955#discussion_r935956337
##########
solr/core/src/java/org/apache/solr/request/DelegatingSolrQueryRequest.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * 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.request;
+
+import io.opentracing.Span;
+import io.opentracing.Tracer;
+import org.apache.solr.cloud.CloudDescriptor;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.CommandOperation;
+import org.apache.solr.common.util.ContentStream;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.servlet.HttpSolrCall;
+import org.apache.solr.util.RTimerTree;
+
+import java.security.Principal;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A {@link SolrQueryRequest} implementation that defers to a delegate in all cases.
+ *
+ * Used primarily in cases where developers want to customize one or more SolrQueryRequest methods while deferring the
+ * remainder to an existing instances.
+ */
+public class DelegatingSolrQueryRequest implements SolrQueryRequest {
Review Comment:
I agree, but `SolrQueryRequest` is a solr-core specific class: it's not visible from SolrJ code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #955:
URL: https://github.com/apache/solr/pull/955#discussion_r937067799
##########
solr/core/src/java/org/apache/solr/handler/configsets/CreateConfigSetAPI.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.configsets;
+
+import com.google.common.collect.Maps;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.solr.api.Command;
+import org.apache.solr.api.EndPoint;
+import org.apache.solr.api.PayloadObj;
+import org.apache.solr.client.solrj.request.beans.CreateConfigPayload;
+import org.apache.solr.cloud.ConfigSetCmds;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.ConfigSetParams;
+import org.apache.solr.core.CoreContainer;
+
+import java.util.Map;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
+import static org.apache.solr.common.params.CommonParams.NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DEFAULT_CONFIGSET_NAME;
+import static org.apache.solr.handler.admin.ConfigSetsHandler.DISABLE_CREATE_AUTH_CHECKS;
+import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
+
+/**
+ * V2 API for creating a new configset as a copy of an existing one.
+ *
+ * <p>This API (POST /v2/cluster/configs {"create": {...}}) is analogous to
+ * the v1 /admin/configs?action=CREATE command.
+ *
+ */
+@EndPoint(method = POST, path = "/cluster/configs", permission = CONFIG_EDIT_PERM)
+public class CreateConfigSetAPI extends ConfigSetAPIBase {
+
+ public CreateConfigSetAPI(CoreContainer coreContainer) {
+ super(coreContainer);
+ }
+
+ @Command(name = "create")
+ @SuppressWarnings("unchecked")
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #955:
URL: https://github.com/apache/solr/pull/955#discussion_r937074450
##########
solr/core/src/test/org/apache/solr/handler/admin/TestConfigsApi.java:
##########
@@ -17,19 +17,11 @@
package org.apache.solr.handler.admin;
-import static org.apache.solr.client.solrj.SolrRequest.METHOD.DELETE;
-import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
-import static org.apache.solr.handler.admin.TestCollectionAPIs.compareOutput;
-
-import java.util.Map;
import org.apache.solr.SolrTestCaseJ4;
-import org.apache.solr.api.ApiBag;
-import org.apache.solr.common.cloud.ZkNodeProps;
-import org.apache.solr.handler.ClusterAPI;
-import org.apache.solr.response.SolrQueryResponse;
public class TestConfigsApi extends SolrTestCaseJ4 {
+ /*
Review Comment:
In hindsight, the single test here has significant overlap with TestConfigSetsApi, so I think I _am_ going to remove it altogether.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #955: SOLR-15736: Move configset logic into v2 APIs
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #955:
URL: https://github.com/apache/solr/pull/955#discussion_r935954554
##########
solr/core/src/test/org/apache/solr/handler/admin/TestConfigsApi.java:
##########
@@ -17,19 +17,11 @@
package org.apache.solr.handler.admin;
-import static org.apache.solr.client.solrj.SolrRequest.METHOD.DELETE;
-import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
-import static org.apache.solr.handler.admin.TestCollectionAPIs.compareOutput;
-
-import java.util.Map;
import org.apache.solr.SolrTestCaseJ4;
-import org.apache.solr.api.ApiBag;
-import org.apache.solr.common.cloud.ZkNodeProps;
-import org.apache.solr.handler.ClusterAPI;
-import org.apache.solr.response.SolrQueryResponse;
public class TestConfigsApi extends SolrTestCaseJ4 {
+ /*
Review Comment:
Uh, no haha. But I plan to uncomment these tests before taking the PR forward.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org