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 2021/12/08 12:01:47 UTC
[GitHub] [solr] gerlowskija opened a new pull request #450: SOLR-15745: Convert create-core v2 API to annotations
gerlowskija opened a new pull request #450:
URL: https://github.com/apache/solr/pull/450
# Description
Solr currently supports two ways of implementing v2 APIs: an older JSON file ('apispec') based approach, and a new framework that relies on annotations. The code base is a slow transition towards the annotation-based approach, but many APIs remain using the 'apispec' model.
# Solution
This PR switches a single API, the "create core" API, over to the new annotation-based framework.
# Tests
See V2CoresAPIMappingTest.
# 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 change in pull request #450: SOLR-15745: Convert create-core v2 API to annotations
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on a change in pull request #450:
URL: https://github.com/apache/solr/pull/450#discussion_r767067405
##########
File path: solr/core/src/test/org/apache/solr/handler/admin/V2CoresAPIMappingTest.java
##########
@@ -0,0 +1,146 @@
+/*
+ * 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.admin;
+
+import com.google.common.collect.Maps;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.api.Api;
+import org.apache.solr.api.ApiBag;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.CommandOperation;
+import org.apache.solr.common.util.ContentStreamBase;
+import org.apache.solr.handler.admin.api.CreateCoreAPI;
+import org.apache.solr.request.LocalSolrQueryRequest;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+
+import static org.apache.solr.common.params.CollectionAdminParams.NUM_SHARDS;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.common.params.CommonParams.ACTION;
+import static org.apache.solr.common.params.CoreAdminParams.*;
+import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.CREATE;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Unit tests for the /cores APIs.
+ *
+ * Note that the V2 requests made by these tests are not necessarily semantically valid. They shouldn't be taken as
+ * examples. In several instances, mutually exclusive JSON parameters are provided. This is done to exercise conversion
+ * of all parameters, even if particular combinations are never expected in the same request.
+ */
+public class V2CoresAPIMappingTest extends SolrTestCaseJ4 {
+ private ApiBag apiBag;
+ private ArgumentCaptor<SolrQueryRequest> queryRequestCaptor;
+ private CoreAdminHandler mockCoreAdminHandler;
+
+ @Before
+ public void setUpMocks() {
+ mockCoreAdminHandler = mock(CoreAdminHandler.class);
+ queryRequestCaptor = ArgumentCaptor.forClass(SolrQueryRequest.class);
+
+ apiBag = new ApiBag(false);
+ apiBag.registerObject(new CreateCoreAPI(mockCoreAdminHandler));
+ }
+
+ @Test
+ public void testCreateCoreAllProperties() throws Exception {
+ final SolrParams v1Params = captureConvertedV1Params("/cores", "POST",
+ "{'create': {" +
+ "'name': 'someCoreName', " +
+ "'instanceDir': 'someInstanceDir', " +
+ "'dataDir': 'someDataDir', " +
+ "'ulogDir': 'someUpdateLogDirectory', " +
+ "'schema': 'some-schema-file-name', " +
+ "'config': 'some-config-file-name', " +
+ "'configSet': 'someConfigSetName', " +
+ "'loadOnStartup': true, " +
+ "'isTransient': true, " +
+ "'shard': 'someShardName', " +
+ "'collection': 'someCollectionName', " +
+ "'replicaType': 'TLOG', " +
+ "'coreNodeName': 'someNodeName', " +
+ "'numShards': 123, " +
+ "'roles': ['role1', 'role2'], " +
+ "'properties': {'prop1': 'val1', 'prop2': 'val2'}, " +
+ "'newCollection': true, " +
+ "'async': 'requestTrackingId' " +
+ "}}");
+
+ // TODO Delete apispec file
Review comment:
Haha, yeah, these are remnants from a TODO list I wrote myself at some point. Will remove.
--
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 change in pull request #450: SOLR-15745: Convert create-core v2 API to annotations
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on a change in pull request #450:
URL: https://github.com/apache/solr/pull/450#discussion_r767068017
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/CreateCorePayload.java
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.client.solrj.request.beans;
+
+import org.apache.solr.common.annotation.JsonProperty;
+import org.apache.solr.common.util.ReflectMapWriter;
+
+import java.util.List;
+import java.util.Map;
+
+public class CreateCorePayload implements ReflectMapWriter {
+ @JsonProperty(required = true)
+ public String name;
+
+ @JsonProperty
+ public String instanceDir;
+
+ @JsonProperty
+ public String dataDir;
+
+ @JsonProperty
+ public String ulogDir;
+
+ @JsonProperty
+ public String schema;
+
+ @JsonProperty
+ public String config;
+
+ @JsonProperty
+ public String configSet;
+
+ @JsonProperty
+ public Boolean loadOnStartup;
+
+ // If our JsonProperty clone was more feature-rich here we could specify the property be called 'transient', but
Review comment:
Well that's what I am doing here for the v2 API. The property name in v2 is "isTransient".
The v1 query parameter is still named "transient". And we could change that, but it'd run afoul of backwards compatibility concerns. I'd need an 8.x release to sneak a deprecation into, which doesn't seem super likely at this point.
--
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 a change in pull request #450: SOLR-15745: Convert create-core v2 API to annotations
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #450:
URL: https://github.com/apache/solr/pull/450#discussion_r764887482
##########
File path: solr/core/src/test/org/apache/solr/handler/admin/V2CoresAPIMappingTest.java
##########
@@ -0,0 +1,146 @@
+/*
+ * 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.admin;
+
+import com.google.common.collect.Maps;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.api.Api;
+import org.apache.solr.api.ApiBag;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.CommandOperation;
+import org.apache.solr.common.util.ContentStreamBase;
+import org.apache.solr.handler.admin.api.CreateCoreAPI;
+import org.apache.solr.request.LocalSolrQueryRequest;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+
+import static org.apache.solr.common.params.CollectionAdminParams.NUM_SHARDS;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.common.params.CommonParams.ACTION;
+import static org.apache.solr.common.params.CoreAdminParams.*;
+import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.CREATE;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Unit tests for the /cores APIs.
+ *
+ * Note that the V2 requests made by these tests are not necessarily semantically valid. They shouldn't be taken as
+ * examples. In several instances, mutually exclusive JSON parameters are provided. This is done to exercise conversion
+ * of all parameters, even if particular combinations are never expected in the same request.
+ */
+public class V2CoresAPIMappingTest extends SolrTestCaseJ4 {
+ private ApiBag apiBag;
+ private ArgumentCaptor<SolrQueryRequest> queryRequestCaptor;
+ private CoreAdminHandler mockCoreAdminHandler;
+
+ @Before
+ public void setUpMocks() {
+ mockCoreAdminHandler = mock(CoreAdminHandler.class);
+ queryRequestCaptor = ArgumentCaptor.forClass(SolrQueryRequest.class);
+
+ apiBag = new ApiBag(false);
+ apiBag.registerObject(new CreateCoreAPI(mockCoreAdminHandler));
+ }
+
+ @Test
+ public void testCreateCoreAllProperties() throws Exception {
+ final SolrParams v1Params = captureConvertedV1Params("/cores", "POST",
+ "{'create': {" +
+ "'name': 'someCoreName', " +
+ "'instanceDir': 'someInstanceDir', " +
+ "'dataDir': 'someDataDir', " +
+ "'ulogDir': 'someUpdateLogDirectory', " +
+ "'schema': 'some-schema-file-name', " +
+ "'config': 'some-config-file-name', " +
+ "'configSet': 'someConfigSetName', " +
+ "'loadOnStartup': true, " +
+ "'isTransient': true, " +
+ "'shard': 'someShardName', " +
+ "'collection': 'someCollectionName', " +
+ "'replicaType': 'TLOG', " +
+ "'coreNodeName': 'someNodeName', " +
+ "'numShards': 123, " +
+ "'roles': ['role1', 'role2'], " +
+ "'properties': {'prop1': 'val1', 'prop2': 'val2'}, " +
+ "'newCollection': true, " +
+ "'async': 'requestTrackingId' " +
+ "}}");
+
+ // TODO Delete apispec file
Review comment:
;-). You did complete these two TODO's
--
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 #450: SOLR-15745: Convert create-core v2 API to annotations
Posted by GitBox <gi...@apache.org>.
gerlowskija merged pull request #450:
URL: https://github.com/apache/solr/pull/450
--
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 change in pull request #450: SOLR-15745: Convert create-core v2 API to annotations
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on a change in pull request #450:
URL: https://github.com/apache/solr/pull/450#discussion_r767067405
##########
File path: solr/core/src/test/org/apache/solr/handler/admin/V2CoresAPIMappingTest.java
##########
@@ -0,0 +1,146 @@
+/*
+ * 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.admin;
+
+import com.google.common.collect.Maps;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.api.Api;
+import org.apache.solr.api.ApiBag;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.CommandOperation;
+import org.apache.solr.common.util.ContentStreamBase;
+import org.apache.solr.handler.admin.api.CreateCoreAPI;
+import org.apache.solr.request.LocalSolrQueryRequest;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+
+import static org.apache.solr.common.params.CollectionAdminParams.NUM_SHARDS;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.common.params.CommonParams.ACTION;
+import static org.apache.solr.common.params.CoreAdminParams.*;
+import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.CREATE;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Unit tests for the /cores APIs.
+ *
+ * Note that the V2 requests made by these tests are not necessarily semantically valid. They shouldn't be taken as
+ * examples. In several instances, mutually exclusive JSON parameters are provided. This is done to exercise conversion
+ * of all parameters, even if particular combinations are never expected in the same request.
+ */
+public class V2CoresAPIMappingTest extends SolrTestCaseJ4 {
+ private ApiBag apiBag;
+ private ArgumentCaptor<SolrQueryRequest> queryRequestCaptor;
+ private CoreAdminHandler mockCoreAdminHandler;
+
+ @Before
+ public void setUpMocks() {
+ mockCoreAdminHandler = mock(CoreAdminHandler.class);
+ queryRequestCaptor = ArgumentCaptor.forClass(SolrQueryRequest.class);
+
+ apiBag = new ApiBag(false);
+ apiBag.registerObject(new CreateCoreAPI(mockCoreAdminHandler));
+ }
+
+ @Test
+ public void testCreateCoreAllProperties() throws Exception {
+ final SolrParams v1Params = captureConvertedV1Params("/cores", "POST",
+ "{'create': {" +
+ "'name': 'someCoreName', " +
+ "'instanceDir': 'someInstanceDir', " +
+ "'dataDir': 'someDataDir', " +
+ "'ulogDir': 'someUpdateLogDirectory', " +
+ "'schema': 'some-schema-file-name', " +
+ "'config': 'some-config-file-name', " +
+ "'configSet': 'someConfigSetName', " +
+ "'loadOnStartup': true, " +
+ "'isTransient': true, " +
+ "'shard': 'someShardName', " +
+ "'collection': 'someCollectionName', " +
+ "'replicaType': 'TLOG', " +
+ "'coreNodeName': 'someNodeName', " +
+ "'numShards': 123, " +
+ "'roles': ['role1', 'role2'], " +
+ "'properties': {'prop1': 'val1', 'prop2': 'val2'}, " +
+ "'newCollection': true, " +
+ "'async': 'requestTrackingId' " +
+ "}}");
+
+ // TODO Delete apispec file
Review comment:
Haha, yeah, these are remnants from a TODO list I wrote myself at some point. Will remove.
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/CreateCorePayload.java
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.client.solrj.request.beans;
+
+import org.apache.solr.common.annotation.JsonProperty;
+import org.apache.solr.common.util.ReflectMapWriter;
+
+import java.util.List;
+import java.util.Map;
+
+public class CreateCorePayload implements ReflectMapWriter {
+ @JsonProperty(required = true)
+ public String name;
+
+ @JsonProperty
+ public String instanceDir;
+
+ @JsonProperty
+ public String dataDir;
+
+ @JsonProperty
+ public String ulogDir;
+
+ @JsonProperty
+ public String schema;
+
+ @JsonProperty
+ public String config;
+
+ @JsonProperty
+ public String configSet;
+
+ @JsonProperty
+ public Boolean loadOnStartup;
+
+ // If our JsonProperty clone was more feature-rich here we could specify the property be called 'transient', but
Review comment:
Well that's what I am doing here for the v2 API. The property name in v2 is "isTransient".
The v1 query parameter is still named "transient". And we could change that, but it'd run afoul of backwards compatibility concerns. I'd need an 8.x release to sneak a deprecation into, which doesn't seem super likely at this point.
--
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 a change in pull request #450: SOLR-15745: Convert create-core v2 API to annotations
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #450:
URL: https://github.com/apache/solr/pull/450#discussion_r764885676
##########
File path: solr/core/src/test/org/apache/solr/handler/admin/V2CoresAPIMappingTest.java
##########
@@ -0,0 +1,146 @@
+/*
+ * 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.admin;
+
+import com.google.common.collect.Maps;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.api.Api;
+import org.apache.solr.api.ApiBag;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.CommandOperation;
+import org.apache.solr.common.util.ContentStreamBase;
+import org.apache.solr.handler.admin.api.CreateCoreAPI;
+import org.apache.solr.request.LocalSolrQueryRequest;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+
+import static org.apache.solr.common.params.CollectionAdminParams.NUM_SHARDS;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.common.params.CommonParams.ACTION;
+import static org.apache.solr.common.params.CoreAdminParams.*;
+import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.CREATE;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Unit tests for the /cores APIs.
+ *
+ * Note that the V2 requests made by these tests are not necessarily semantically valid. They shouldn't be taken as
+ * examples. In several instances, mutually exclusive JSON parameters are provided. This is done to exercise conversion
+ * of all parameters, even if particular combinations are never expected in the same request.
+ */
+public class V2CoresAPIMappingTest extends SolrTestCaseJ4 {
+ private ApiBag apiBag;
+ private ArgumentCaptor<SolrQueryRequest> queryRequestCaptor;
+ private CoreAdminHandler mockCoreAdminHandler;
+
+ @Before
+ public void setUpMocks() {
+ mockCoreAdminHandler = mock(CoreAdminHandler.class);
+ queryRequestCaptor = ArgumentCaptor.forClass(SolrQueryRequest.class);
+
+ apiBag = new ApiBag(false);
+ apiBag.registerObject(new CreateCoreAPI(mockCoreAdminHandler));
+ }
+
+ @Test
+ public void testCreateCoreAllProperties() throws Exception {
+ final SolrParams v1Params = captureConvertedV1Params("/cores", "POST",
+ "{'create': {" +
+ "'name': 'someCoreName', " +
+ "'instanceDir': 'someInstanceDir', " +
+ "'dataDir': 'someDataDir', " +
+ "'ulogDir': 'someUpdateLogDirectory', " +
+ "'schema': 'some-schema-file-name', " +
+ "'config': 'some-config-file-name', " +
+ "'configSet': 'someConfigSetName', " +
+ "'loadOnStartup': true, " +
+ "'isTransient': true, " +
+ "'shard': 'someShardName', " +
+ "'collection': 'someCollectionName', " +
+ "'replicaType': 'TLOG', " +
+ "'coreNodeName': 'someNodeName', " +
+ "'numShards': 123, " +
+ "'roles': ['role1', 'role2'], " +
+ "'properties': {'prop1': 'val1', 'prop2': 'val2'}, " +
+ "'newCollection': true, " +
+ "'async': 'requestTrackingId' " +
+ "}}");
+
+ // TODO Delete apispec file
Review comment:
I'm sure you haven't forgotton about this.
--
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 a change in pull request #450: SOLR-15745: Convert create-core v2 API to annotations
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #450:
URL: https://github.com/apache/solr/pull/450#discussion_r764884564
##########
File path: solr/core/src/test/org/apache/solr/handler/admin/V2CoresAPIMappingTest.java
##########
@@ -0,0 +1,146 @@
+/*
+ * 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.admin;
+
+import com.google.common.collect.Maps;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.api.Api;
+import org.apache.solr.api.ApiBag;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.CommandOperation;
+import org.apache.solr.common.util.ContentStreamBase;
+import org.apache.solr.handler.admin.api.CreateCoreAPI;
+import org.apache.solr.request.LocalSolrQueryRequest;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+
+import static org.apache.solr.common.params.CollectionAdminParams.NUM_SHARDS;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.common.params.CommonParams.ACTION;
+import static org.apache.solr.common.params.CoreAdminParams.*;
+import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.CREATE;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Unit tests for the /cores APIs.
+ *
+ * Note that the V2 requests made by these tests are not necessarily semantically valid. They shouldn't be taken as
Review comment:
Thanks for providing this detail! I tend to look at unit tests as "living documentation" so this is good!
--
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 a change in pull request #450: SOLR-15745: Convert create-core v2 API to annotations
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #450:
URL: https://github.com/apache/solr/pull/450#discussion_r764886640
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/CreateCorePayload.java
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.client.solrj.request.beans;
+
+import org.apache.solr.common.annotation.JsonProperty;
+import org.apache.solr.common.util.ReflectMapWriter;
+
+import java.util.List;
+import java.util.Map;
+
+public class CreateCorePayload implements ReflectMapWriter {
+ @JsonProperty(required = true)
+ public String name;
+
+ @JsonProperty
+ public String instanceDir;
+
+ @JsonProperty
+ public String dataDir;
+
+ @JsonProperty
+ public String ulogDir;
+
+ @JsonProperty
+ public String schema;
+
+ @JsonProperty
+ public String config;
+
+ @JsonProperty
+ public String configSet;
+
+ @JsonProperty
+ public Boolean loadOnStartup;
+
+ // If our JsonProperty clone was more feature-rich here we could specify the property be called 'transient', but
Review comment:
should we just pick a different parameter name and not use the word transient? I.e, should we go solve this by changing somethign elsewhere so this doesnt' have this issue?
--
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 a change in pull request #450: SOLR-15745: Convert create-core v2 API to annotations
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #450:
URL: https://github.com/apache/solr/pull/450#discussion_r764886640
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/CreateCorePayload.java
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.client.solrj.request.beans;
+
+import org.apache.solr.common.annotation.JsonProperty;
+import org.apache.solr.common.util.ReflectMapWriter;
+
+import java.util.List;
+import java.util.Map;
+
+public class CreateCorePayload implements ReflectMapWriter {
+ @JsonProperty(required = true)
+ public String name;
+
+ @JsonProperty
+ public String instanceDir;
+
+ @JsonProperty
+ public String dataDir;
+
+ @JsonProperty
+ public String ulogDir;
+
+ @JsonProperty
+ public String schema;
+
+ @JsonProperty
+ public String config;
+
+ @JsonProperty
+ public String configSet;
+
+ @JsonProperty
+ public Boolean loadOnStartup;
+
+ // If our JsonProperty clone was more feature-rich here we could specify the property be called 'transient', but
Review comment:
should we just pick a different parameter name and not sue the word transient?
--
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