Skip to content

Commit

Permalink
Allow API to accept any index name without suffix
Browse files Browse the repository at this point in the history
Maps will support add import GeoJSON into an index. This
will be used by more layers like document, cluster. The restriction
was previously added since it was mainly used as Custom Vector map.
This change is already incorporated by front end.

Signed-off-by: Vijayan Balasubramanian <[email protected]>
(cherry picked from commit dc6b7e4)
  • Loading branch information
VijayanB committed Feb 2, 2023
1 parent 9f835cb commit ff990a9
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 62 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ public final class UploadGeoJSONRequestContent {
public static final ParseField FIELD_GEOSPATIAL = new ParseField("field");
public static final ParseField FIELD_GEOSPATIAL_TYPE = new ParseField("type");
public static final ParseField FIELD_DATA = new ParseField("data");
public static final String ACCEPTED_INDEX_SUFFIX_PATH = "-map";
private final String indexName;
private final String fieldName;
private final String fieldType;
Expand Down Expand Up @@ -67,22 +66,12 @@ public static UploadGeoJSONRequestContent create(Map<String, Object> input) {

private static String validateIndexName(Map<String, Object> input) {
String index = extractValueAsString(input, FIELD_INDEX.getPreferredName());
if (!Strings.hasText(index)) {
throw new IllegalArgumentException(
String.format(Locale.getDefault(), "field [ %s ] cannot be empty", FIELD_INDEX.getPreferredName())
);
}
if (!index.endsWith(ACCEPTED_INDEX_SUFFIX_PATH)) {
throw new IllegalArgumentException(
String.format(
Locale.getDefault(),
"field [ %s ] should end with suffix %s",
FIELD_INDEX.getPreferredName(),
ACCEPTED_INDEX_SUFFIX_PATH
)
);
if (Strings.hasText(index)) {
return index;
}
return index;
throw new IllegalArgumentException(
String.format(Locale.getDefault(), "field [ %s ] cannot be empty", FIELD_INDEX.getPreferredName())
);
}

public String getIndexName() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
import static org.opensearch.geospatial.GeospatialObjectBuilder.buildProperties;
import static org.opensearch.geospatial.GeospatialObjectBuilder.randomGeoJSONFeature;
import static org.opensearch.geospatial.GeospatialTestHelper.randomLowerCaseString;
import static org.opensearch.geospatial.GeospatialTestHelper.randomLowerCaseStringWithSuffix;
import static org.opensearch.geospatial.action.upload.geojson.UploadGeoJSONRequestContent.ACCEPTED_INDEX_SUFFIX_PATH;
import static org.opensearch.geospatial.action.upload.geojson.UploadGeoJSONRequestContent.FIELD_DATA;
import static org.opensearch.geospatial.shared.URLBuilder.getPluginURLPrefix;
import static org.opensearch.index.query.AbstractGeometryQueryBuilder.DEFAULT_SHAPE_FIELD_NAME;
Expand Down Expand Up @@ -162,7 +160,7 @@ public Map<String, Object> getDocument(String docID, String indexName) throws IO
// TODO This method is copied from unit test. Refactor to common class to share across tests
protected JSONObject buildUploadGeoJSONRequestContent(int totalGeoJSONObject, String index, String geoFieldName) {
JSONObject contents = new JSONObject();
String indexName = Strings.hasText(index) ? index : randomLowerCaseStringWithSuffix(ACCEPTED_INDEX_SUFFIX_PATH);
String indexName = Strings.hasText(index) ? index : randomLowerCaseString();
String fieldName = Strings.hasText(geoFieldName) ? geoFieldName : randomLowerCaseString();
contents.put(UploadGeoJSONRequestContent.FIELD_INDEX.getPreferredName(), indexName);
contents.put(UploadGeoJSONRequestContent.FIELD_GEOSPATIAL.getPreferredName(), fieldName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import static org.junit.Assert.assertTrue;
import static org.opensearch.geospatial.GeospatialObjectBuilder.buildProperties;
import static org.opensearch.geospatial.GeospatialObjectBuilder.randomGeoJSONFeature;
import static org.opensearch.geospatial.action.upload.geojson.UploadGeoJSONRequestContent.ACCEPTED_INDEX_SUFFIX_PATH;
import static org.opensearch.geospatial.action.upload.geojson.UploadGeoJSONRequestContent.FIELD_DATA;
import static org.opensearch.test.OpenSearchTestCase.randomBoolean;
import static org.opensearch.test.OpenSearchTestCase.randomIntBetween;
Expand Down Expand Up @@ -66,10 +65,7 @@ public static Map<String, Object> buildRequestContent(int featureCount) {
if (Randomness.get().nextBoolean()) {
contents.put(ContentBuilder.GEOJSON_FEATURE_ID_FIELD, randomLowerCaseString());
}
contents.put(
UploadGeoJSONRequestContent.FIELD_INDEX.getPreferredName(),
randomLowerCaseStringWithSuffix(ACCEPTED_INDEX_SUFFIX_PATH)
);
contents.put(UploadGeoJSONRequestContent.FIELD_INDEX.getPreferredName(), randomLowerCaseString());
contents.put(UploadGeoJSONRequestContent.FIELD_GEOSPATIAL.getPreferredName(), randomLowerCaseString());
contents.put(UploadGeoJSONRequestContent.FIELD_GEOSPATIAL_TYPE.getPreferredName(), "geo_shape");
JSONArray values = new JSONArray();
Expand All @@ -86,10 +82,6 @@ public static String randomLowerCaseString() {
return randomString().toLowerCase(Locale.getDefault());
}

public static String randomLowerCaseStringWithSuffix(String suffix) {
return String.format(Locale.getDefault(), "%s%s", randomString().toLowerCase(Locale.getDefault()), suffix);
}

/**
* Returns random {@link IndexResponse} by generating inputs using random functions.
* It is not guaranteed to generate every possible values, and it is not required since
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
import static org.opensearch.geospatial.GeospatialObjectBuilder.buildProperties;
import static org.opensearch.geospatial.GeospatialObjectBuilder.randomGeoJSONFeature;
import static org.opensearch.geospatial.GeospatialTestHelper.randomLowerCaseString;
import static org.opensearch.geospatial.GeospatialTestHelper.randomLowerCaseStringWithSuffix;
import static org.opensearch.geospatial.action.upload.geojson.UploadGeoJSONRequestContent.ACCEPTED_INDEX_SUFFIX_PATH;
import static org.opensearch.geospatial.action.upload.geojson.UploadGeoJSONRequestContent.FIELD_DATA;
import static org.opensearch.geospatial.action.upload.geojson.UploadGeoJSONRequestContent.GEOSPATIAL_DEFAULT_FIELD_NAME;

Expand All @@ -21,9 +19,18 @@
import org.opensearch.test.OpenSearchTestCase;

public class UploadGeoJSONRequestContentTests extends OpenSearchTestCase {
private String indexName;
private String fieldName;

@Override
public void setUp() throws Exception {
super.setUp();
indexName = randomLowerCaseString();
fieldName = randomLowerCaseString();
}

private Map<String, Object> buildRequestContent(String indexName, String fieldName) {
JSONObject contents = new JSONObject();
final var contents = new JSONObject();
contents.put(UploadGeoJSONRequestContent.FIELD_INDEX.getPreferredName(), indexName);
contents.put(UploadGeoJSONRequestContent.FIELD_GEOSPATIAL.getPreferredName(), fieldName);
contents.put(UploadGeoJSONRequestContent.FIELD_GEOSPATIAL_TYPE.getPreferredName(), "geo_shape");
Expand All @@ -36,10 +43,8 @@ private Map<String, Object> buildRequestContent(String indexName, String fieldNa
}

public void testCreate() {
final String indexName = randomLowerCaseStringWithSuffix(ACCEPTED_INDEX_SUFFIX_PATH);
final String fieldName = "location";
Map<String, Object> contents = buildRequestContent(indexName, fieldName);
UploadGeoJSONRequestContent content = UploadGeoJSONRequestContent.create(contents);
final var content = UploadGeoJSONRequestContent.create(contents);
assertNotNull(content);
assertEquals(fieldName, content.getFieldName());
assertEquals(indexName, content.getIndexName());
Expand All @@ -55,32 +60,12 @@ public void testCreateEmptyIndexName() {
}

public void testCreateEmptyGeospatialFieldName() {
UploadGeoJSONRequestContent content = UploadGeoJSONRequestContent.create(
buildRequestContent(randomLowerCaseStringWithSuffix(ACCEPTED_INDEX_SUFFIX_PATH), "")
);
final var content = UploadGeoJSONRequestContent.create(buildRequestContent(randomLowerCaseString(), ""));
assertNotNull(content);
assertEquals("wrong field name", GEOSPATIAL_DEFAULT_FIELD_NAME, content.getFieldName());
}

public void testCreateInvalidIndexName() {
final String indexName = randomLowerCaseString();
final String fieldName = "location";
Map<String, Object> contents = buildRequestContent(indexName, fieldName);
contents.remove(UploadGeoJSONRequestContent.FIELD_GEOSPATIAL_TYPE.getPreferredName());
IllegalArgumentException invalidIndexName = assertThrows(
IllegalArgumentException.class,
() -> UploadGeoJSONRequestContent.create(contents)
);
assertEquals(
"wrong exception message",
"field [ index ] should end with suffix " + ACCEPTED_INDEX_SUFFIX_PATH,
invalidIndexName.getMessage()
);
}

public void testCreateEmptyGeospatialFieldType() {
final String indexName = randomLowerCaseStringWithSuffix(ACCEPTED_INDEX_SUFFIX_PATH);
final String fieldName = "location";
Map<String, Object> contents = buildRequestContent(indexName, fieldName);
contents.remove(UploadGeoJSONRequestContent.FIELD_GEOSPATIAL_TYPE.getPreferredName());
IllegalArgumentException invalidIndexName = assertThrows(
Expand All @@ -89,5 +74,4 @@ public void testCreateEmptyGeospatialFieldType() {
);
assertTrue(invalidIndexName.getMessage().contains("[ type ] cannot be empty"));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
package org.opensearch.geospatial.rest.action.upload.geojson;

import static org.opensearch.geospatial.GeospatialTestHelper.randomLowerCaseString;
import static org.opensearch.geospatial.GeospatialTestHelper.randomLowerCaseStringWithSuffix;
import static org.opensearch.geospatial.action.upload.geojson.UploadGeoJSONRequestContent.*;

import java.io.IOException;
Expand All @@ -31,7 +30,7 @@ public class RestUploadGeoJSONActionIT extends GeospatialRestTestCase {

public void testGeoJSONUploadSuccessPostMethod() throws IOException {

final String index = randomLowerCaseStringWithSuffix(ACCEPTED_INDEX_SUFFIX_PATH);
final String index = randomLowerCaseString();
assertIndexNotExists(index);
Response response = uploadGeoJSONFeatures(NUMBER_OF_FEATURES_TO_ADD, index, null);
assertEquals(RestStatus.OK, RestStatus.fromCode(response.getStatusLine().getStatusCode()));
Expand All @@ -41,8 +40,7 @@ public void testGeoJSONUploadSuccessPostMethod() throws IOException {

public void testGeoJSONUploadFailIndexExists() throws IOException {

String index = randomLowerCaseStringWithSuffix(ACCEPTED_INDEX_SUFFIX_PATH);
;
String index = randomLowerCaseString();
String geoFieldName = randomLowerCaseString();
Map<String, String> geoFields = new HashMap<>();
geoFields.put(geoFieldName, "geo_shape");
Expand All @@ -57,7 +55,7 @@ public void testGeoJSONUploadFailIndexExists() throws IOException {

public void testGeoJSONUploadSuccessPutMethod() throws IOException {

String index = randomLowerCaseStringWithSuffix(ACCEPTED_INDEX_SUFFIX_PATH);
String index = randomLowerCaseString();
Response response = uploadGeoJSONFeaturesIntoExistingIndex(NUMBER_OF_FEATURES_TO_ADD, index, null);
assertEquals(RestStatus.OK, RestStatus.fromCode(response.getStatusLine().getStatusCode()));
assertIndexExists(index);
Expand All @@ -66,7 +64,7 @@ public void testGeoJSONUploadSuccessPutMethod() throws IOException {

public void testGeoJSONPutMethodUploadIndexExists() throws IOException {

String index = randomLowerCaseStringWithSuffix(ACCEPTED_INDEX_SUFFIX_PATH);
String index = randomLowerCaseString();
String geoFieldName = randomLowerCaseString();
Response response = uploadGeoJSONFeaturesIntoExistingIndex(NUMBER_OF_FEATURES_TO_ADD, index, geoFieldName);
assertEquals(RestStatus.OK, RestStatus.fromCode(response.getStatusLine().getStatusCode()));
Expand Down

0 comments on commit ff990a9

Please sign in to comment.