Solr
  1. Solr
  2. SOLR-8339

SolrDocument and SolrInputDocument should have a common interface

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.5, 6.0
    • Component/s: None
    • Labels:
      None

      Description

      Currently, both share a Map interface (SOLR-928). However, there are many common methods like createField(), setField() etc. that should perhaps go into an interface/abstract class.

      1. SOLR-8339.patch
        8 kB
        Ishan Chattopadhyaya
      2. SOLR-8339.patch
        8 kB
        Ishan Chattopadhyaya
      3. SOLR-8339.patch
        8 kB
        Ishan Chattopadhyaya
      4. SOLR-8339.patch
        6 kB
        Ishan Chattopadhyaya

        Issue Links

          Activity

          Hide
          Ishan Chattopadhyaya added a comment - - edited

          Is there a historic reason for not having a common interface/abstract class for SolrDocument and SolrInputDocument other than a Map?
          Since having this right now might break backcompat, does it make sense to do this for 6.0?
          Right now, the motivation for doing this is SOLR-8220, but not strictly needed.

          Show
          Ishan Chattopadhyaya added a comment - - edited Is there a historic reason for not having a common interface/abstract class for SolrDocument and SolrInputDocument other than a Map? Since having this right now might break backcompat, does it make sense to do this for 6.0? Right now, the motivation for doing this is SOLR-8220 , but not strictly needed.
          Hide
          Ishan Chattopadhyaya added a comment -

          It seems there is no backcompat implications. Spoke to Noble Paul offline, and mentioned that there is no backcompat issue with this change as long as any existing method is not deleted or altered.

          Here's a patch adding a SolrDocumentBase as an abstract class which SolrDocument and SolrInputDocument now extend from.

          Show
          Ishan Chattopadhyaya added a comment - It seems there is no backcompat implications. Spoke to Noble Paul offline, and mentioned that there is no backcompat issue with this change as long as any existing method is not deleted or altered. Here's a patch adding a SolrDocumentBase as an abstract class which SolrDocument and SolrInputDocument now extend from.
          Hide
          Ishan Chattopadhyaya added a comment -

          Updated patch, includes the child document methods common to both the classes as well.

          Show
          Ishan Chattopadhyaya added a comment - Updated patch, includes the child document methods common to both the classes as well.
          Hide
          Mikhail Khludnev added a comment -

          Looked on. Patch seems fine!

          Show
          Mikhail Khludnev added a comment - Looked on. Patch seems fine!
          Hide
          Ishan Chattopadhyaya added a comment -

          Removed javadocs references using @see annotation. Some how ant was not happy. Those links seemed not all that useful anyway.

          Show
          Ishan Chattopadhyaya added a comment - Removed javadocs references using @see annotation. Some how ant was not happy. Those links seemed not all that useful anyway.
          Hide
          Ahmet Arslan added a comment -

          With this change, can we remove org.apache.solr.client.solrj.util.ClientUtils#toSolrInputDocument method? And org.apache.solr.client.solrj.util.ClientUtils#toSolrDocument ?

          Show
          Ahmet Arslan added a comment - With this change, can we remove org.apache.solr.client.solrj.util.ClientUtils#toSolrInputDocument method? And org.apache.solr.client.solrj.util.ClientUtils#toSolrDocument ?
          Hide
          Shalin Shekhar Mangar added a comment -

          With this change, can we remove org.apache.solr.client.solrj.util.ClientUtils#toSolrInputDocument method? And org.apache.solr.client.solrj.util.ClientUtils#toSolrDocument ?

          We can deprecate those in 5.x and remove in trunk.

          Ishan Chattopadhyaya - SolrDocument used to be serializable but you have removed that in your patch. Same for SolrInputDocument. That is a compat-break. Can you put that back?

          Show
          Shalin Shekhar Mangar added a comment - With this change, can we remove org.apache.solr.client.solrj.util.ClientUtils#toSolrInputDocument method? And org.apache.solr.client.solrj.util.ClientUtils#toSolrDocument ? We can deprecate those in 5.x and remove in trunk. Ishan Chattopadhyaya - SolrDocument used to be serializable but you have removed that in your patch. Same for SolrInputDocument. That is a compat-break. Can you put that back?
          Hide
          Ishan Chattopadhyaya added a comment -

          Thanks Shalin Shekhar Mangar. Added the Serializable to the base class.

          Show
          Ishan Chattopadhyaya added a comment - Thanks Shalin Shekhar Mangar . Added the Serializable to the base class.
          Hide
          ASF subversion and git services added a comment -

          Commit 1717654 from shalin@apache.org in branch 'dev/trunk'
          [ https://svn.apache.org/r1717654 ]

          SOLR-8339: Refactor SolrDocument and SolrInputDocument to have a common base abstract class called SolrDocumentBase. Deprecated methods toSolrInputDocument and toSolrDocument in ClientUtils.

          Show
          ASF subversion and git services added a comment - Commit 1717654 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1717654 ] SOLR-8339 : Refactor SolrDocument and SolrInputDocument to have a common base abstract class called SolrDocumentBase. Deprecated methods toSolrInputDocument and toSolrDocument in ClientUtils.
          Hide
          ASF subversion and git services added a comment -

          Commit 1717657 from shalin@apache.org in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1717657 ]

          SOLR-8339: Refactor SolrDocument and SolrInputDocument to have a common base abstract class called SolrDocumentBase. Deprecated methods toSolrInputDocument and toSolrDocument in ClientUtils.

          Show
          ASF subversion and git services added a comment - Commit 1717657 from shalin@apache.org in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1717657 ] SOLR-8339 : Refactor SolrDocument and SolrInputDocument to have a common base abstract class called SolrDocumentBase. Deprecated methods toSolrInputDocument and toSolrDocument in ClientUtils.
          Hide
          Shalin Shekhar Mangar added a comment -

          Thanks Ishan!

          Show
          Shalin Shekhar Mangar added a comment - Thanks Ishan!

            People

            • Assignee:
              Shalin Shekhar Mangar
              Reporter:
              Ishan Chattopadhyaya
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development