Skip to content

Commit

Permalink
Prohibit indexing a document with parent for a type that doesn't have…
Browse files Browse the repository at this point in the history 
… a `_parent` field configured and prohibit adding a _parent field to an existing mapping.

Closes #3848 #3849
  • Loading branch information
@martijnvg
martijnvg committed Oct 15, 2013
1 parent 89de3ab commit cc9ab11
Show file tree
Hide file tree
Showing 14 changed files with 159 additions and 34 deletions.
8 changes: 8 additions & 0 deletions src/main/java/org/elasticsearch/action/index/IndexRequest.java
View file
Expand Up @@ -578,6 +578,14 @@ public void process(MetaData metaData, String aliasOrIndex, @Nullable MappingMet
if (mappingMd.routing().required() && routing == null) {
throw new RoutingMissingException(index, type, id);
}

if (parent != null && !mappingMd.hasParentField()) {
throw new ElasticSearchIllegalArgumentException("Can't specify parent if no parent field has been configured");
}
} else {
if (parent != null) {
throw new ElasticSearchIllegalArgumentException("Can't specify parent if no parent field has been configured");
}
}

// generate id if not already provided and id generation is allowed
Expand Down
26 changes: 24 additions & 2 deletions src/main/java/org/elasticsearch/cluster/metadata/MappingMetaData.java
View file
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.cluster.metadata;

import org.elasticsearch.ElasticSearchIllegalStateException;
import org.elasticsearch.Version;
import org.elasticsearch.action.TimestampParsingException;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
Expand Down Expand Up @@ -258,13 +259,15 @@ public int hashCode() {
private Id id;
private Routing routing;
private Timestamp timestamp;
private boolean hasParentField;

public MappingMetaData(DocumentMapper docMapper) {
this.type = docMapper.type();
this.source = docMapper.mappingSource();
this.id = new Id(docMapper.idFieldMapper().path());
this.routing = new Routing(docMapper.routingFieldMapper().required(), docMapper.routingFieldMapper().path());
this.timestamp = new Timestamp(docMapper.timestampFieldMapper().enabled(), docMapper.timestampFieldMapper().path(), docMapper.timestampFieldMapper().dateTimeFormatter().format());
this.hasParentField = docMapper.parentFieldMapper().active();
}

public MappingMetaData(CompressedString mapping) throws IOException {
Expand Down Expand Up @@ -344,14 +347,20 @@ private void initMappers(Map<String, Object> withoutType) {
} else {
this.timestamp = Timestamp.EMPTY;
}
if (withoutType.containsKey("_parent")) {
this.hasParentField = true;
} else {
this.hasParentField = false;
}
}

public MappingMetaData(String type, CompressedString source, Id id, Routing routing, Timestamp timestamp) {
public MappingMetaData(String type, CompressedString source, Id id, Routing routing, Timestamp timestamp, boolean hasParentField) {
this.type = type;
this.source = source;
this.id = id;
this.routing = routing;
this.timestamp = timestamp;
this.hasParentField = hasParentField;
}

void updateDefaultMapping(MappingMetaData defaultMapping) {
Expand All @@ -374,6 +383,10 @@ public CompressedString source() {
return this.source;
}

public boolean hasParentField() {
return hasParentField;
}

/**
* Converts the serialized compressed form of the mappings into a parsed map.
*/
Expand Down Expand Up @@ -516,6 +529,9 @@ public static void writeTo(MappingMetaData mappingMd, StreamOutput out) throws I
out.writeBoolean(false);
}
out.writeString(mappingMd.timestamp().format());
if (out.getVersion().onOrAfter(Version.V_0_90_6)) {
out.writeBoolean(mappingMd.hasParentField());
}
}

@Override
Expand Down Expand Up @@ -553,7 +569,13 @@ public static MappingMetaData readFrom(StreamInput in) throws IOException {
Routing routing = new Routing(in.readBoolean(), in.readBoolean() ? in.readString() : null);
// timestamp
Timestamp timestamp = new Timestamp(in.readBoolean(), in.readBoolean() ? in.readString() : null, in.readString());
return new MappingMetaData(type, source, id, routing, timestamp);
final boolean hasParentField;
if (in.getVersion().onOrAfter(Version.V_0_90_6)) {
hasParentField = in.readBoolean();
} else {
hasParentField = true; // We assume here that the type has a parent field, which is confirm with the behaviour of <= 0.90.5
}
return new MappingMetaData(type, source, id, routing, timestamp, hasParentField);
}

public static class ParseContext {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/elasticsearch/index/cache/id/simple/SimpleIdCache.java
View file
Expand Up @@ -123,7 +123,7 @@ public void refresh(List<AtomicReaderContext> atomicReaderContexts) throws IOExc
BytesRef spare = new BytesRef();
for (String type : indexService.mapperService().types()) {
ParentFieldMapper parentFieldMapper = indexService.mapperService().documentMapper(type).parentFieldMapper();
if (parentFieldMapper != null) {
if (parentFieldMapper.active()) {
parentTypes.add(new HashedBytesArray(Strings.toUTF8Bytes(parentFieldMapper.type(), spare)));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/elasticsearch/index/get/ShardGetService.java
View file
Expand Up @@ -226,7 +226,7 @@ public GetResult innerGet(String type, String id, String[] gFields, boolean real
Object value = null;
if (field.equals(RoutingFieldMapper.NAME) && docMapper.routingFieldMapper().fieldType().stored()) {
value = source.routing;
} else if (field.equals(ParentFieldMapper.NAME) && docMapper.parentFieldMapper() != null && docMapper.parentFieldMapper().fieldType().stored()) {
} else if (field.equals(ParentFieldMapper.NAME) && docMapper.parentFieldMapper().active() && docMapper.parentFieldMapper().fieldType().stored()) {
value = source.parent;
} else if (field.equals(TimestampFieldMapper.NAME) && docMapper.timestampFieldMapper().fieldType().stored()) {
value = source.timestamp;
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java
View file
Expand Up @@ -177,7 +177,7 @@ public Builder(String index, @Nullable Settings indexSettings, RootObjectMapper.
this.rootMappers.put(TimestampFieldMapper.class, new TimestampFieldMapper());
this.rootMappers.put(TTLFieldMapper.class, new TTLFieldMapper());
this.rootMappers.put(VersionFieldMapper.class, new VersionFieldMapper());
// don't add parent field, by default its "null"
this.rootMappers.put(ParentFieldMapper.class, new ParentFieldMapper());
}

public Builder meta(ImmutableMap<String, Object> meta) {
Expand Down Expand Up @@ -306,7 +306,7 @@ public DocumentMapper(String index, @Nullable Settings indexSettings, DocumentMa

this.typeFilter = typeMapper().termFilter(type, null);

if (rootMapper(ParentFieldMapper.class) != null) {
if (rootMapper(ParentFieldMapper.class).active()) {
// mark the routing field mapper as required
rootMapper(RoutingFieldMapper.class).markAsRequired();
}
Expand Down Expand Up @@ -631,8 +631,9 @@ public void traverse(ObjectMapperListener listener) {

public synchronized MergeResult merge(DocumentMapper mergeWith, MergeFlags mergeFlags) {
MergeContext mergeContext = new MergeContext(this, mergeFlags);
rootObjectMapper.merge(mergeWith.rootObjectMapper, mergeContext);
assert rootMappers.size() == mergeWith.rootMappers.size();

rootObjectMapper.merge(mergeWith.rootObjectMapper, mergeContext);
for (Map.Entry<Class<? extends RootMapper>, RootMapper> entry : rootMappers.entrySet()) {
// root mappers included in root object will get merge in the rootObjectMapper
if (entry.getValue().includeInObject()) {
Expand Down
36 changes: 33 additions & 3 deletions src/main/java/org/elasticsearch/index/mapper/internal/ParentFieldMapper.java
View file
Expand Up @@ -130,6 +130,13 @@ protected ParentFieldMapper(String name, String indexName, String type, Postings
this.typeAsBytes = new BytesRef(type);
}

public ParentFieldMapper() {
super(new Names(Defaults.NAME, Defaults.NAME, Defaults.NAME, Defaults.NAME), Defaults.BOOST, new FieldType(Defaults.FIELD_TYPE),
Lucene.KEYWORD_ANALYZER, Lucene.KEYWORD_ANALYZER, null, null, null, null, null);
type = null;
typeAsBytes = null;
}

public String type() {
return type;
}
Expand Down Expand Up @@ -169,6 +176,10 @@ public boolean includeInObject() {

@Override
protected void parseCreateField(ParseContext context, List<Field> fields) throws IOException {
if (!active()) {
return;
}

if (context.parser().currentName() != null && context.parser().currentName().equals(Defaults.NAME)) {
// we are in the parsing of _parent phase
String parentId = context.parser().text();
Expand Down Expand Up @@ -253,7 +264,7 @@ public Filter termFilter(Object value, @Nullable QueryParseContext context) {

List<String> types = new ArrayList<String>(context.mapperService().types().size());
for (DocumentMapper documentMapper : context.mapperService()) {
if (documentMapper.parentFieldMapper() == null) {
if (!documentMapper.parentFieldMapper().active()) {
types.add(documentMapper.type());
}
}
Expand Down Expand Up @@ -284,7 +295,7 @@ public Filter termsFilter(List values, @Nullable QueryParseContext context) {

List<String> types = new ArrayList<String>(context.mapperService().types().size());
for (DocumentMapper documentMapper : context.mapperService()) {
if (documentMapper.parentFieldMapper() == null) {
if (!documentMapper.parentFieldMapper().active()) {
types.add(documentMapper.type());
}
}
Expand Down Expand Up @@ -319,6 +330,10 @@ protected String contentType() {

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
if (!active()) {
return builder;
}

builder.startObject(CONTENT_TYPE);
builder.field("type", type);
builder.endObject();
Expand All @@ -327,6 +342,21 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws

@Override
public void merge(Mapper mergeWith, MergeContext mergeContext) throws MergeMappingException {
// do nothing here, no merging, but also no exception
ParentFieldMapper other = (ParentFieldMapper) mergeWith;
if (active() == other.active()) {
return;
}

if (active() != other.active() || !type.equals(other.type)) {
mergeContext.addConflict("The _parent field can't be added or updated");
}
}

/**
* @return Whether the _parent field is actually used.
*/
public boolean active() {
return type != null;
}

}
2 changes: 1 addition & 1 deletion src/main/java/org/elasticsearch/index/query/HasChildFilterParser.java
View file
Expand Up @@ -121,7 +121,7 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
if (childDocMapper == null) {
throw new QueryParsingException(parseContext.index(), "No mapping for for type [" + childType + "]");
}
if (childDocMapper.parentFieldMapper() == null) {
if (!childDocMapper.parentFieldMapper().active()) {
throw new QueryParsingException(parseContext.index(), "Type [" + childType + "] does not have parent mapping");
}
String parentType = childDocMapper.parentFieldMapper().type();
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/elasticsearch/index/query/HasChildQueryParser.java
View file
Expand Up @@ -123,7 +123,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
if (childDocMapper == null) {
throw new QueryParsingException(parseContext.index(), "[has_child] No mapping for for type [" + childType + "]");
}
if (childDocMapper.parentFieldMapper() == null) {
if (!childDocMapper.parentFieldMapper().active()) {
throw new QueryParsingException(parseContext.index(), "[has_child] Type [" + childType + "] does not have parent mapping");
}
String parentType = childDocMapper.parentFieldMapper().type();
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/elasticsearch/index/query/HasParentFilterParser.java
View file
Expand Up @@ -135,7 +135,7 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
parentTypes.add(parentType);
for (DocumentMapper documentMapper : parseContext.mapperService()) {
ParentFieldMapper parentFieldMapper = documentMapper.parentFieldMapper();
if (parentFieldMapper != null) {
if (parentFieldMapper.active()) {
DocumentMapper parentTypeDocumentMapper = parseContext.mapperService().documentMapper(parentFieldMapper.type());
if (parentTypeDocumentMapper == null) {
// Only add this, if this parentFieldMapper (also a parent) isn't a child of another parent.
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/elasticsearch/index/query/HasParentQueryParser.java
View file
Expand Up @@ -134,7 +134,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
parentTypes.add(parentType);
for (DocumentMapper documentMapper : parseContext.mapperService()) {
ParentFieldMapper parentFieldMapper = documentMapper.parentFieldMapper();
if (parentFieldMapper != null) {
if (parentFieldMapper.active()) {
DocumentMapper parentTypeDocumentMapper = parseContext.mapperService().documentMapper(parentFieldMapper.type());
if (parentTypeDocumentMapper == null) {
// Only add this, if this parentFieldMapper (also a parent) isn't a child of another parent.
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/elasticsearch/index/query/TopChildrenQueryParser.java
View file
Expand Up @@ -120,7 +120,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
if (childDocMapper == null) {
throw new QueryParsingException(parseContext.index(), "No mapping for for type [" + childType + "]");
}
if (childDocMapper.parentFieldMapper() == null) {
if (!childDocMapper.parentFieldMapper().active()) {
throw new QueryParsingException(parseContext.index(), "Type [" + childType + "] does not have parent mapping");
}
String parentType = childDocMapper.parentFieldMapper().type();
Expand Down

0 comments on commit cc9ab11

Please sign in to comment.