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

Allow API to accept any index name without suffix #182

Merged
merged 1 commit into from
Jan 17, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 Ex
// 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 Exception {

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 Exception {

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 Exception {

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 Exception {

public void testGeoJSONPutMethodUploadIndexExists() throws Exception {

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