Skip to content

Commit

Permalink
Mappings: Remove support for field access by short name
Browse files Browse the repository at this point in the history 
When multiple fields under object fields share the same name, accessing
by short name is ambiguous.  This removes support for short names,
always requiring the full name when used in queries.

closes elastic#8872
  • Loading branch information
@rjernst
rjernst committed Feb 12, 2015
1 parent 9b75d3e commit b524980
Show file tree
Hide file tree
Showing 13 changed files with 74 additions and 113 deletions.
3 changes: 2 additions & 1 deletion src/main/java/org/elasticsearch/index/mapper/DocumentFieldMappers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,9 @@ public Map.Entry<String, Analyzer> apply(FieldMapper<?> input) {
return new DocumentFieldMappers(fieldMappers, indexAnalyzer, searchAnalyzer, searchQuoteAnalyzer);
}

// TODO: replace all uses of this with fullName, or change the meaning of name to be fullName
public FieldMappers name(String name) {
return fieldMappers.name(name);
return fieldMappers.fullName(name);
}

public FieldMappers indexName(String indexName) {
Expand Down
33 changes: 10 additions & 23 deletions src/main/java/org/elasticsearch/index/mapper/FieldMappersLookup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ MappersLookup removeMappers(Iterable<?> mappers) {

/** Create a new empty instance. */
public FieldMappersLookup() {
this(new CopyOnWriteHashSet<FieldMapper<?>>(), new MappersLookup(new CopyOnWriteHashMap<String, FieldMappers>(), new CopyOnWriteHashMap<String, FieldMappers>(), new CopyOnWriteHashMap<String, FieldMappers>()));
this(new CopyOnWriteHashSet<FieldMapper<?>>(),
new MappersLookup(new CopyOnWriteHashMap<String, FieldMappers>(),
new CopyOnWriteHashMap<String, FieldMappers>(),
new CopyOnWriteHashMap<String, FieldMappers>()));
}

private FieldMappersLookup(CopyOnWriteHashSet<FieldMapper<?>> mappers, MappersLookup lookup) {
Expand All @@ -130,13 +133,6 @@ public FieldMappersLookup copyAndRemoveAll(Collection<?> mappersToRemove) {
}
}

/**
* Returns the field mappers based on the mapper name.
*/
public FieldMappers name(String name) {
return lookup.name.get(name);
}

/**
* Returns the field mappers based on the mapper index name.
*/
Expand All @@ -152,7 +148,7 @@ public FieldMappers fullName(String fullName) {
}

/**
* Returns a list of the index names of a simple match regex like pattern against full name, name and index name.
* Returns a list of the index names of a simple match regex like pattern against full name and index name.
*/
public List<String> simpleMatchToIndexNames(String pattern) {
List<String> fields = Lists.newArrayList();
Expand All @@ -161,15 +157,13 @@ public List<String> simpleMatchToIndexNames(String pattern) {
fields.add(fieldMapper.names().indexName());
} else if (Regex.simpleMatch(pattern, fieldMapper.names().indexName())) {
fields.add(fieldMapper.names().indexName());
} else if (Regex.simpleMatch(pattern, fieldMapper.names().name())) {
fields.add(fieldMapper.names().indexName());
}
}
return fields;
}

/**
* Returns a list of the full names of a simple match regex like pattern against full name, name and index name.
* Returns a list of the full names of a simple match regex like pattern against full name and index name.
*/
public List<String> simpleMatchToFullName(String pattern) {
List<String> fields = Lists.newArrayList();
Expand All @@ -178,33 +172,26 @@ public List<String> simpleMatchToFullName(String pattern) {
fields.add(fieldMapper.names().fullName());
} else if (Regex.simpleMatch(pattern, fieldMapper.names().indexName())) {
fields.add(fieldMapper.names().fullName());
} else if (Regex.simpleMatch(pattern, fieldMapper.names().name())) {
fields.add(fieldMapper.names().fullName());
}
}
return fields;
}

/**
* Tries to find first based on {@link #fullName(String)}, then by {@link #indexName(String)}, and last
* by {@link #name(String)}.
* Tries to find first based on {@link #fullName(String)}, then by {@link #indexName(String)}.
*/
@Nullable
public FieldMappers smartName(String name) {
FieldMappers fieldMappers = fullName(name);
if (fieldMappers != null) {
return fieldMappers;
}
fieldMappers = indexName(name);
if (fieldMappers != null) {
return fieldMappers;
}
return name(name);
return indexName(name);
}

/**
* Tries to find first based on {@link #fullName(String)}, then by {@link #indexName(String)}, and last
* by {@link #name(String)} and return the first mapper for it (see {@link org.elasticsearch.index.mapper.FieldMappers#mapper()}).
* Tries to find first based on {@link #fullName(String)}, then by {@link #indexName(String)}
* and return the first mapper for it (see {@link org.elasticsearch.index.mapper.FieldMappers#mapper()}).
*/
@Nullable
public FieldMapper<?> smartNameFieldMapper(String name) {
Expand Down
23 changes: 2 additions & 21 deletions src/main/java/org/elasticsearch/index/mapper/MapperService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -558,17 +558,6 @@ public Filter searchFilter(String... types) {
}
}

/**
* Returns {@link FieldMappers} for all the {@link FieldMapper}s that are registered
* under the given name across all the different {@link DocumentMapper} types.
*
* @param name The name to return all the {@link FieldMappers} for across all {@link DocumentMapper}s.
* @return All the {@link FieldMappers} for across all {@link DocumentMapper}s
*/
public FieldMappers name(String name) {
return fieldMappers.name(name);
}

/**
* Returns {@link FieldMappers} for all the {@link FieldMapper}s that are registered
* under the given indexName across all the different {@link DocumentMapper} types.
Expand Down Expand Up @@ -706,11 +695,7 @@ public FieldMappers smartNameFieldMappers(String smartName) {
if (mappers != null) {
return mappers;
}
mappers = indexName(smartName);
if (mappers != null) {
return mappers;
}
return name(smartName);
return indexName(smartName);
}

public SmartNameFieldMappers smartName(String smartName, @Nullable String[] types) {
Expand Down Expand Up @@ -755,10 +740,6 @@ public SmartNameFieldMappers smartName(String smartName) {
if (fieldMappers != null) {
return new SmartNameFieldMappers(this, fieldMappers, null, false);
}
fieldMappers = name(smartName);
if (fieldMappers != null) {
return new SmartNameFieldMappers(this, fieldMappers, null, false);
}
return null;
}

Expand Down Expand Up @@ -833,7 +814,7 @@ public ObjectMapper resolveClosestNestedObjectMapper(String fieldName) {
public static boolean isMetadataField(String fieldName) {
return META_FIELDS.contains(fieldName);
}

public static class SmartNameObjectMapper {
private final ObjectMapper mapper;
private final DocumentMapper docMapper;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/elasticsearch/indices/ttl/IndicesTTLService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ private List<IndexShard> getShardsToPurge() {
}

// should be optimized with the hasTTL flag
FieldMappers ttlFieldMappers = indexService.mapperService().name(TTLFieldMapper.NAME);
FieldMappers ttlFieldMappers = indexService.mapperService().fullName(TTLFieldMapper.NAME);
if (ttlFieldMappers == null) {
continue;
}
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/org/elasticsearch/document/DocumentActionsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,19 +102,19 @@ public void testIndexActions() throws Exception {
getResult = client().prepareGet("test", "type1", "1").setOperationThreaded(false).execute().actionGet();
assertThat(getResult.getIndex(), equalTo(getConcreteIndexName()));
assertThat("cycle #" + i, getResult.getSourceAsString(), equalTo(source("1", "test").string()));
assertThat("cycle(map) #" + i, (String) ((Map) getResult.getSourceAsMap().get("type1")).get("name"), equalTo("test"));
assertThat("cycle(map) #" + i, (String) getResult.getSourceAsMap().get("name"), equalTo("test"));
getResult = client().get(getRequest("test").type("type1").id("1").operationThreaded(true)).actionGet();
assertThat("cycle #" + i, getResult.getSourceAsString(), equalTo(source("1", "test").string()));
assertThat(getResult.getIndex(), equalTo(getConcreteIndexName()));
}

logger.info("Get [type1/1] with script");
for (int i = 0; i < 5; i++) {
getResult = client().prepareGet("test", "type1", "1").setFields("type1.name").execute().actionGet();
getResult = client().prepareGet("test", "type1", "1").setFields("name").execute().actionGet();
assertThat(getResult.getIndex(), equalTo(getConcreteIndexName()));
assertThat(getResult.isExists(), equalTo(true));
assertThat(getResult.getSourceAsBytes(), nullValue());
assertThat(getResult.getField("type1.name").getValues().get(0).toString(), equalTo("test"));
assertThat(getResult.getField("name").getValues().get(0).toString(), equalTo("test"));
}

logger.info("Get [type1/2] (should be empty)");
Expand Down Expand Up @@ -321,6 +321,6 @@ public void testDeleteRoutingRequired() throws ExecutionException, InterruptedEx
}

private XContentBuilder source(String id, String nameValue) throws IOException {
return XContentFactory.jsonBuilder().startObject().startObject("type1").field("id", id).field("name", nameValue).endObject().endObject();
return XContentFactory.jsonBuilder().startObject().field("id", id).field("name", nameValue).endObject();
}
}
9 changes: 4 additions & 5 deletions src/test/java/org/elasticsearch/index/mapper/simple/SimpleMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ public void testSimpleMapper() throws Exception {
BytesReference json = new BytesArray(copyToBytesFromClasspath("/org/elasticsearch/index/mapper/simple/test1.json"));
Document doc = docMapper.parse("person", "1", json).rootDoc();

assertThat(doc.get(docMapper.mappers().name("first").mapper().names().indexName()), equalTo("shay"));
assertThat(docMapper.mappers().name("first").mapper().names().fullName(), equalTo("name.first"));
assertThat(doc.get(docMapper.mappers().name("name.first").mapper().names().indexName()), equalTo("shay"));
// System.out.println("Document: " + doc);
// System.out.println("Json: " + docMapper.sourceMapper().value(doc));
doc = docMapper.parse(json).rootDoc();
Expand All @@ -74,7 +73,7 @@ public void testParseToJsonAndParse() throws Exception {
BytesReference json = new BytesArray(copyToBytesFromClasspath("/org/elasticsearch/index/mapper/simple/test1.json"));
Document doc = builtDocMapper.parse(json).rootDoc();
assertThat(doc.get(docMapper.uidMapper().names().indexName()), equalTo(Uid.createUid("person", "1")));
assertThat(doc.get(docMapper.mappers().name("first").mapper().names().indexName()), equalTo("shay"));
assertThat(doc.get(docMapper.mappers().name("name.first").mapper().names().indexName()), equalTo("shay"));
// System.out.println("Document: " + doc);
// System.out.println("Json: " + docMapper.sourceMapper().value(doc));
}
Expand All @@ -89,7 +88,7 @@ public void testSimpleParser() throws Exception {
BytesReference json = new BytesArray(copyToBytesFromClasspath("/org/elasticsearch/index/mapper/simple/test1.json"));
Document doc = docMapper.parse(json).rootDoc();
assertThat(doc.get(docMapper.uidMapper().names().indexName()), equalTo(Uid.createUid("person", "1")));
assertThat(doc.get(docMapper.mappers().name("first").mapper().names().indexName()), equalTo("shay"));
assertThat(doc.get(docMapper.mappers().name("name.first").mapper().names().indexName()), equalTo("shay"));
// System.out.println("Document: " + doc);
// System.out.println("Json: " + docMapper.sourceMapper().value(doc));
}
Expand All @@ -101,7 +100,7 @@ public void testSimpleParserNoTypeNoId() throws Exception {
BytesReference json = new BytesArray(copyToBytesFromClasspath("/org/elasticsearch/index/mapper/simple/test1-notype-noid.json"));
Document doc = docMapper.parse("person", "1", json).rootDoc();
assertThat(doc.get(docMapper.uidMapper().names().indexName()), equalTo(Uid.createUid("person", "1")));
assertThat(doc.get(docMapper.mappers().name("first").mapper().names().indexName()), equalTo("shay"));
assertThat(doc.get(docMapper.mappers().name("name.first").mapper().names().indexName()), equalTo("shay"));
// System.out.println("Document: " + doc);
// System.out.println("Json: " + docMapper.sourceMapper().value(doc));
}
Expand Down
Loading

0 comments on commit b524980

Please sign in to comment.