Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/410#discussion_r200230404
  
    --- Diff: 
solr/core/src/java/org/apache/solr/update/processor/NestedUpdateProcessorFactory.java
 ---
    @@ -0,0 +1,120 @@
    +/*
    + * 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.update.processor;
    +
    +import java.io.IOException;
    +
    +import org.apache.solr.common.SolrException;
    +import org.apache.solr.common.SolrInputDocument;
    +import org.apache.solr.common.SolrInputField;
    +import org.apache.solr.request.SolrQueryRequest;
    +import org.apache.solr.response.SolrQueryResponse;
    +import org.apache.solr.schema.IndexSchema;
    +import org.apache.solr.update.AddUpdateCommand;
    +
    +public class NestedUpdateProcessorFactory extends 
UpdateRequestProcessorFactory {
    +
    +  public UpdateRequestProcessor getInstance(SolrQueryRequest req, 
SolrQueryResponse rsp, UpdateRequestProcessor next ) {
    +    boolean storeParent = shouldStoreDocParent(req.getSchema());
    +    boolean storePath = shouldStoreDocPath(req.getSchema());
    +    if(!(storeParent || storePath)) {
    +      return next;
    +    }
    +    return new NestedUpdateProcessor(req, rsp, 
shouldStoreDocParent(req.getSchema()), shouldStoreDocPath(req.getSchema()), 
next);
    +  }
    +
    +  private static boolean shouldStoreDocParent(IndexSchema schema) {
    +    return schema.getFields().containsKey(IndexSchema.PARENT_FIELD_NAME);
    +  }
    +
    +  private static boolean shouldStoreDocPath(IndexSchema schema) {
    +    return schema.getFields().containsKey(IndexSchema.PATH_FIELD_NAME);
    +  }
    +}
    +
    +class NestedUpdateProcessor extends UpdateRequestProcessor {
    +  private static final String PATH_SEP_CHAR = "/";
    +  private boolean storePath;
    +  private boolean storeParent;
    +  private String uniqueKeyFieldName;
    +
    +
    +  protected NestedUpdateProcessor(SolrQueryRequest req, SolrQueryResponse 
rsp, boolean storeParent, boolean storePath, UpdateRequestProcessor next) {
    +    super(next);
    +    this.storeParent = storeParent;
    +    this.storePath = storePath;
    +    this.uniqueKeyFieldName = 
req.getSchema().getUniqueKeyField().getName();
    +  }
    +
    +  @Override
    +  public void processAdd(AddUpdateCommand cmd) throws IOException {
    +    SolrInputDocument doc = cmd.getSolrInputDocument();
    +    processDocChildren(doc, null);
    +    super.processAdd(cmd);
    +  }
    +
    +  private void processDocChildren(SolrInputDocument doc, String fullPath) {
    +    int childNum = 0;
    +    for(SolrInputField field: doc.values()) {
    +      for(Object val: field) {
    +        if(!(val instanceof SolrInputDocument)) {
    +          // either all collection items are child docs or none are.
    +          break;
    +        }
    +        final String fieldName = field.getName();
    +
    +        if(fieldName.contains(PATH_SEP_CHAR)) {
    +          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, 
"Field name: '" + fieldName
    +              + "' contains: '" + PATH_SEP_CHAR + "' , which is reserved 
for the nested URP");
    +        }
    +        final String jointPath = fullPath == null ? fieldName: fullPath + 
PATH_SEP_CHAR + fieldName;
    +        SolrInputDocument cDoc = (SolrInputDocument) val;
    +        if(!cDoc.containsKey(uniqueKeyFieldName)) {
    +          String parentDocId = 
doc.getField(uniqueKeyFieldName).getFirstValue().toString();
    +          cDoc.setField(uniqueKeyFieldName, 
generateChildUniqueId(parentDocId, fieldName, childNum));
    +        }
    +        processChildDoc((SolrInputDocument) val, doc, jointPath);
    +        ++childNum;
    +      }
    +    }
    +  }
    +
    +  private void processChildDoc(SolrInputDocument sdoc, SolrInputDocument 
parent, String fullPath) {
    +    if(storePath) {
    +      setPathField(sdoc, fullPath);
    +    }
    +    if (storeParent) {
    +      setParentKey(sdoc, parent);
    +    }
    +    processDocChildren(sdoc, fullPath);
    +  }
    +
    +  private String generateChildUniqueId(String parentId, String childKey, 
int childNum) {
    +    // combines parentId with the child's key and childNum. e.g. 
"10/footnote/1"
    +    return String.join(PATH_SEP_CHAR, parentId, childKey, 
Integer.toString(childNum));
    --- End diff --
    
    Glad you fixed the comment.
    Another dubious use of of String.join
    What did you think of my suggestion to use some other separator char (not 
PATH_SEP_CHAR) over here.  My first suggestion was a pound symbol.  Though it 
wouldn't show when in a URL... maybe a comma.  But we could stay with a '/'; I 
just thought it might be nice to make a distinction between the separator 
between parent/child and the separator to append the child sequence/num.
    We ought to surface this to the JIRA as it's likely to get input.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to