Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-15745: Convert create-core v2 API to annotations #450

Merged
merged 2 commits into from
Dec 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.CoreDescriptor;
import org.apache.solr.handler.RequestHandlerBase;
import org.apache.solr.handler.admin.api.CreateCoreAPI;
import org.apache.solr.handler.admin.api.InvokeClassAPI;
import org.apache.solr.handler.admin.api.OverseerOperationAPI;
import org.apache.solr.handler.admin.api.RejoinLeaderElectionAPI;
Expand Down Expand Up @@ -410,6 +411,7 @@ void call() throws Exception {
public Collection<Api> getApis() {
final List<Api> apis = Lists.newArrayList(coreAdminHandlerApi.getApis());
// Only some core-admin APIs use the v2 AnnotatedApi framework
apis.addAll(AnnotatedApi.getApis(new CreateCoreAPI(this)));
apis.addAll(AnnotatedApi.getApis(new InvokeClassAPI(this)));
apis.addAll(AnnotatedApi.getApis(new RejoinLeaderElectionAPI(this)));
apis.addAll(AnnotatedApi.getApis(new OverseerOperationAPI(this)));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* 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.api;

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.CreateCorePayload;
import org.apache.solr.handler.admin.CoreAdminHandler;

import java.util.HashMap;
import java.util.Locale;
import java.util.Map;

import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
import static org.apache.solr.common.params.CollectionAdminParams.PROPERTY_PREFIX;
import static org.apache.solr.common.params.CoreAdminParams.ACTION;
import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.CREATE;
import static org.apache.solr.handler.ClusterAPI.wrapParams;
import static org.apache.solr.handler.api.V2ApiUtils.flattenMapWithPrefix;
import static org.apache.solr.handler.api.V2ApiUtils.flattenToCommaDelimitedString;
import static org.apache.solr.security.PermissionNameProvider.Name.CORE_EDIT_PERM;

/**
* V2 API for creating a new core on the receiving node.
*
* This API (POST /v2/cores {'create': {...}}) is analogous to the v1 /admin/cores?action=CREATE command.
*
* @see CreateCorePayload
*/
@EndPoint(path = {"/cores"},
method = POST,
permission = CORE_EDIT_PERM)
public class CreateCoreAPI {
public static final String V2_CREATE_CORE_CMD = "create";

private final CoreAdminHandler coreAdminHandler;

public CreateCoreAPI(CoreAdminHandler coreAdminHandler) {
this.coreAdminHandler = coreAdminHandler;
}

@Command(name = V2_CREATE_CORE_CMD)
public void createCore(PayloadObj<CreateCorePayload> obj) throws Exception {
final CreateCorePayload v2Body = obj.get();
final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
v1Params.put(ACTION, CREATE.name().toLowerCase(Locale.ROOT));

if (v2Body.isTransient != null) {
v1Params.put("transient", v1Params.remove("isTransient"));
}
if (v2Body.properties != null) {
v1Params.remove("properties");
flattenMapWithPrefix(v2Body.properties, v1Params, PROPERTY_PREFIX);
}
if (v2Body.roles != null && !v2Body.roles.isEmpty()) {
v1Params.remove("roles"); // Not strictly needed, since the method below would overwrite it.
flattenToCommaDelimitedString(v1Params, v2Body.roles, "roles");
}

coreAdminHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.apache.solr.handler.api;

import java.util.List;
import java.util.Map;

/**
Expand All @@ -33,4 +34,10 @@ public static void flattenMapWithPrefix(Map<String, Object> toFlatten, Map<Strin

toFlatten.forEach((k, v) -> destination.put(additionalPrefix + k, v));
}

public static void flattenToCommaDelimitedString(Map<String, Object> destination, List<String> toFlatten, String newKey) {
final String flattenedStr = String.join(",", toFlatten);
destination.put(newKey, flattenedStr);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
/*
* 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.BeforeClass;
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for providing this detail! I tend to look at unit tests as "living documentation" so this is good!

* 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;

@BeforeClass
public static void ensureWorkingMockito() {
assumeWorkingMockito();
}

@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' " +
"}}");

assertEquals(CREATE.name().toLowerCase(Locale.ROOT), v1Params.get(ACTION));
assertEquals("someCoreName", v1Params.get(NAME));
assertEquals("someInstanceDir", v1Params.get(INSTANCE_DIR));
assertEquals("someDataDir", v1Params.get(DATA_DIR));
assertEquals("someUpdateLogDirectory", v1Params.get(ULOG_DIR));
assertEquals("some-schema-file-name", v1Params.get(SCHEMA));
assertEquals("some-config-file-name", v1Params.get(CONFIG));
assertEquals("someConfigSetName", v1Params.get(CONFIGSET));
assertEquals(true, v1Params.getPrimitiveBool(LOAD_ON_STARTUP));
assertEquals(true, v1Params.getPrimitiveBool(TRANSIENT));
assertEquals("someShardName", v1Params.get(SHARD));
assertEquals("someCollectionName", v1Params.get(COLLECTION));
assertEquals("TLOG", v1Params.get(REPLICA_TYPE));
assertEquals("someNodeName", v1Params.get(CORE_NODE_NAME));
assertEquals(123, v1Params.getPrimitiveInt(NUM_SHARDS));
assertEquals("role1,role2", v1Params.get(ROLES));
assertEquals("val1", v1Params.get("property.prop1"));
assertEquals("val2", v1Params.get("property.prop2"));
assertEquals(true, v1Params.getPrimitiveBool(NEW_COLLECTION));
assertEquals("requestTrackingId", v1Params.get(ASYNC));
}

private SolrParams captureConvertedV1Params(String path, String method, String v2RequestBody) throws Exception {
final HashMap<String, String> parts = new HashMap<>();
final Api api = apiBag.lookup(path, method, parts);
final SolrQueryResponse rsp = new SolrQueryResponse();
final LocalSolrQueryRequest req = new LocalSolrQueryRequest(null, Maps.newHashMap()) {
@Override
public List<CommandOperation> getCommands(boolean validateInput) {
if (v2RequestBody == null) return Collections.emptyList();
return ApiBag.getCommandOperations(new ContentStreamBase.StringStream(v2RequestBody), api.getCommandSchema(), true);
}

@Override
public Map<String, String> getPathTemplateValues() {
return parts;
}

@Override
public String getHttpMethod() {
return method;
}
};


api.call(req, rsp);
verify(mockCoreAdminHandler).handleRequestBody(queryRequestCaptor.capture(), any());
return queryRequestCaptor.getValue().getParams();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
*/
public class CoreApiMapping {
public enum Meta implements CommandMeta {
CREATE(CORES_COMMANDS, POST, CoreAdminAction.CREATE, "create", Collections.singletonMap("config", "configSet")),
UNLOAD(PER_CORE_COMMANDS, POST, CoreAdminAction.UNLOAD, "unload", null),
RELOAD(PER_CORE_COMMANDS, POST, CoreAdminAction.RELOAD, "reload", null),
STATUS(CORES_STATUS, GET, CoreAdminAction.STATUS, "status", null),
Expand Down Expand Up @@ -88,7 +87,6 @@ public String getParamSubstitute(String param) {

public enum EndPoint implements ApiMapping.V2EndPoint {
CORES_STATUS("cores.Status"),
CORES_COMMANDS("cores.Commands"),
PER_CORE_COMMANDS("cores.core.Commands");

final String specName;
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

@epugh epugh Dec 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// without that support it needs to be named something else to avoid conflicting with the 'transient' keyword in Java
@JsonProperty
public Boolean isTransient;

@JsonProperty
public String shard;

@JsonProperty
public String collection;

// TODO - what type is 'roles' expected to be?
@JsonProperty
public List<String> roles;

@JsonProperty
public String replicaType;

@JsonProperty
public Map<String, Object> properties;

@JsonProperty
public String coreNodeName;

@JsonProperty
public Integer numShards;

@JsonProperty
public Boolean newCollection;

@JsonProperty
public String async;
}