Solr
  1. Solr
  2. SOLR-1129

SolrJ cannot bind dynamic fields to beans

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3
    • Fix Version/s: 1.4
    • Component/s: clients - java
    • Labels:
      None

      Description

      SolrJ does not support binding of dynamic fields to bean fields

      The field declaration could be as follows

      
      @Field("*_s")
      public String anyString;
      
      1. SOLR-1129.patch
        12 kB
        Noble Paul
      2. SOLR-1129.patch
        13 kB
        Noble Paul
      3. SOLR-1129.patch
        14 kB
        Noble Paul
      4. SOLR-1129.patch
        11 kB
        Avlesh Singh
      5. SOLR-1129.patch
        11 kB
        Avlesh Singh
      6. SOLR-1129.patch
        6 kB
        Avlesh Singh

        Issue Links

          Activity

          Hide
          Avlesh Singh added a comment -

          Thanks for creating this ticket, Noble.

          I have an issue with the approach you have mentioned. IMO, the client should specify the "field" to which the value belongs to.
          I would rather prefer a format like this -

          @Field("*_s")
          public Map<String, Object> foo;
          

          where the keys are dynamicField name's. For multivalued fields, a user might specify an Object[] as value.

          Show
          Avlesh Singh added a comment - Thanks for creating this ticket, Noble. I have an issue with the approach you have mentioned. IMO, the client should specify the "field" to which the value belongs to. I would rather prefer a format like this - @Field( "*_s" ) public Map< String , Object > foo; where the keys are dynamicField name's. For multivalued fields, a user might specify an Object[] as value.
          Hide
          Noble Paul added a comment -

          I see your point . it can be extended to support this

          Show
          Noble Paul added a comment - I see your point . it can be extended to support this
          Hide
          Avlesh Singh added a comment -

          Adding a patch for this enhancement.
          Existing test cases pass. I'll add new test cases in TestDocumentObjectBinder shortly.

          This is the approach I took.

          1. Dynamic fields can be annotated in the following manner
            @Field("*_random")
            public Map<String, Integer> random;
            
            @Field("supplier_*")
            public Map<String, String[]> suppliers;
            
            @Field("oem_*")
            public Map<String, List<String>> oem;
            
          2. Based on the value type of this Map, I populate all matching fields in the response with their corresponding value(s).
          3. I added one more clause to the DocField's storeType method to take care of fields of type Map. Using getActualTypeArguments for this ParameterizedType value, I populate the existing properties in the DocField viz name, field, type, isArray, isList etc.
          4. I added a private method, getFieldValueByFieldName(SolrDocument sdoc, String fieldName) in the DocField class, which is now being called by the inject method for all fields. For regular index fields, the method behavior has not changed. However, for a dynamic field (as annotated by the client), this method prepares a Map of matching fieldNames and their corresponding value(s). This in turn is used by the inject method to call the client bean's corresponding setter method.

          Please note that this fix is for version 1.3.

          Cheers
          Avlesh

          Show
          Avlesh Singh added a comment - Adding a patch for this enhancement. Existing test cases pass. I'll add new test cases in TestDocumentObjectBinder shortly. This is the approach I took. Dynamic fields can be annotated in the following manner @Field( "*_random" ) public Map< String , Integer > random; @Field( "supplier_*" ) public Map< String , String []> suppliers; @Field( "oem_*" ) public Map< String , List< String >> oem; Based on the value type of this Map , I populate all matching fields in the response with their corresponding value(s). I added one more clause to the DocField 's storeType method to take care of fields of type Map . Using getActualTypeArguments for this ParameterizedType value, I populate the existing properties in the DocField viz name, field, type, isArray, isList etc. I added a private method, getFieldValueByFieldName(SolrDocument sdoc, String fieldName) in the DocField class, which is now being called by the inject method for all fields. For regular index fields, the method behavior has not changed. However, for a dynamic field (as annotated by the client), this method prepares a Map of matching fieldNames and their corresponding value(s). This in turn is used by the inject method to call the client bean's corresponding setter method. Please note that this fix is for version 1.3. Cheers Avlesh
          Hide
          Noble Paul added a comment -

          few comments

          • The patch does not apply on trunk. fix for an older version is not possible
          • I am not sure if the following cases are handled
            @Field("supplier_*")
            public List<String> suppliers;
            
            @Field("oem_*")
            public String[] oem;
            
          • We may not need to support LinkedHashMap as the type of the field . I don't think Solr preserves order
          • No injection time matching . field.matches("^"fieldName"$") is expensive. Create and cache the Pattern object well in advance.
          Show
          Noble Paul added a comment - few comments The patch does not apply on trunk. fix for an older version is not possible I am not sure if the following cases are handled @Field( "supplier_*" ) public List< String > suppliers; @Field( "oem_*" ) public String [] oem; We may not need to support LinkedHashMap as the type of the field . I don't think Solr preserves order No injection time matching . field.matches("^" fieldName "$") is expensive. Create and cache the Pattern object well in advance.
          Hide
          Avlesh Singh added a comment -

          All you points are taken, Noble.
          I'll make the modifications in trunk and get back asap.

          Show
          Avlesh Singh added a comment - All you points are taken, Noble. I'll make the modifications in trunk and get back asap.
          Hide
          Shalin Shekhar Mangar added a comment -

          Marking for 1.4 as this is a serious deficiency.

          Show
          Shalin Shekhar Mangar added a comment - Marking for 1.4 as this is a serious deficiency.
          Hide
          Noble Paul added a comment -

          we released 1.3 w/o it. Still don't have a final patch . So, I feel it is not a must for 1.4. But if it goes in it is well and good

          Show
          Noble Paul added a comment - we released 1.3 w/o it. Still don't have a final patch . So, I feel it is not a must for 1.4. But if it goes in it is well and good
          Hide
          Avlesh Singh added a comment -

          Sorry for the delay. Will submit a patch latest by tomorrow.

          Show
          Avlesh Singh added a comment - Sorry for the delay. Will submit a patch latest by tomorrow.
          Hide
          Avlesh Singh added a comment -

          All the changes as discussed earlier, have been implemented.
          SolrJ can now be used to bind dynamic fields using any one of these

          @Field("*_random")
          public Map<String, Integer> random;
          
          @Field("categories_*")
          public Map<String, String[]> categories;
          
          @Field("oem_*")
          public Map<String, List<String>> oem;
          
          @Field("supplier_*")
          public List<String> allSuppliers;
          
          @Field("supplier_*")
          public String supplier;
          
          @Field("supplier_*")
          public void setSuppliers(String[] allSuppliers){}
          

          The attached patch also contains a test case for the above mentioned usages.

          Cheers
          Avlesh

          Show
          Avlesh Singh added a comment - All the changes as discussed earlier, have been implemented. SolrJ can now be used to bind dynamic fields using any one of these @Field( "*_random" ) public Map< String , Integer > random; @Field( "categories_*" ) public Map< String , String []> categories; @Field( "oem_*" ) public Map< String , List< String >> oem; @Field( "supplier_*" ) public List< String > allSuppliers; @Field( "supplier_*" ) public String supplier; @Field( "supplier_*" ) public void setSuppliers( String [] allSuppliers){} The attached patch also contains a test case for the above mentioned usages. Cheers Avlesh
          Hide
          Avlesh Singh added a comment -

          Just realized that the usage of field.matches("^"fieldName"$") has not been changed to use the cached pattern. Made the necessary change. Please use the latest patch.

          Show
          Avlesh Singh added a comment - Just realized that the usage of field.matches("^"fieldName"$") has not been changed to use the cached pattern. Made the necessary change. Please use the latest patch.
          Hide
          Noble Paul added a comment -

          My attempt at simplifying the patch .
          pleas take a look and let me know if any usecase is missing

          Show
          Noble Paul added a comment - My attempt at simplifying the patch . pleas take a look and let me know if any usecase is missing
          Hide
          Noble Paul added a comment -

          w/o the SOPs

          Show
          Noble Paul added a comment - w/o the SOPs
          Hide
          Noble Paul added a comment -

          should this go into 1.4 ?

          Show
          Noble Paul added a comment - should this go into 1.4 ?
          Hide
          Avlesh Singh added a comment -

          Good to go, Noble. I have tried and tested the patch, it looks good.

          Show
          Avlesh Singh added a comment - Good to go, Noble. I have tried and tested the patch, it looks good.
          Hide
          Shalin Shekhar Mangar added a comment -

          should this go into 1.4 ?

          +1

          Show
          Shalin Shekhar Mangar added a comment - should this go into 1.4 ? +1
          Hide
          Noble Paul added a comment -

          updated to the trunk . I plan to commit this shortly

          Show
          Noble Paul added a comment - updated to the trunk . I plan to commit this shortly
          Hide
          Noble Paul added a comment -

          committed r794144

          thanks Avlesh

          Show
          Noble Paul added a comment - committed r794144 thanks Avlesh
          Hide
          Anders Hessellund Jensen added a comment -

          The patch only addresses binding of dynamic fields in one direction, that is, converting documents to beans. The other direction, converting beans to documents, does not work. Attempting to index a bean with dynamic fields using the SolrServer.addBean() method does fails to correctly index the dynamic fields.

          Show
          Anders Hessellund Jensen added a comment - The patch only addresses binding of dynamic fields in one direction, that is, converting documents to beans. The other direction, converting beans to documents, does not work. Attempting to index a bean with dynamic fields using the SolrServer.addBean() method does fails to correctly index the dynamic fields.
          Hide
          Avlesh Singh added a comment -

          There is already as issue for the above Anders. SOLR-1357 is waiting for a patch.

          Show
          Avlesh Singh added a comment - There is already as issue for the above Anders. SOLR-1357 is waiting for a patch.
          Hide
          Grant Ingersoll added a comment -

          Bulk close for Solr 1.4

          Show
          Grant Ingersoll added a comment - Bulk close for Solr 1.4

            People

            • Assignee:
              Noble Paul
              Reporter:
              Noble Paul
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development