Solr
  1. Solr
  2. SOLR-7484

Refactor SolrDispatchFilter to move all Solr specific implementation to another class

    Details

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

      Description

      Currently almost everything that's done in SDF.doFilter() is sequential. We should refactor it to clean up the code and make things easier to manage.

      1. SOLR-7484.patch
        0.9 kB
        Noble Paul
      2. SOLR-7484.patch
        65 kB
        Anshum Gupta
      3. SOLR-7484.patch
        65 kB
        Anshum Gupta
      4. SOLR-7484.patch
        68 kB
        Anshum Gupta
      5. SOLR-7484.patch
        64 kB
        Anshum Gupta
      6. SOLR-7484.patch
        63 kB
        Anshum Gupta
      7. SOLR-7484.patch
        63 kB
        Noble Paul
      8. SOLR-7484.patch
        63 kB
        Noble Paul
      9. SOLR-7484.patch
        26 kB
        Anshum Gupta
      10. SOLR-7484-override.patch
        2 kB
        Ryan McKinley

        Issue Links

          Activity

          Hide
          Anshum Gupta added a comment -

          First patch. Still working on some cleanup and fix the failing TestCryptoKeys test.

          Show
          Anshum Gupta added a comment - First patch. Still working on some cleanup and fix the failing TestCryptoKeys test.
          Hide
          Noble Paul added a comment - - edited

          I have moved the entire functionality into a class called SolrCall which represents a call to Solr. SolrDispatchFilter is now just reduced to a proxy which just creates a SolrCall and passes the control over there

          Show
          Noble Paul added a comment - - edited I have moved the entire functionality into a class called SolrCall which represents a call to Solr. SolrDispatchFilter is now just reduced to a proxy which just creates a SolrCall and passes the control over there
          Hide
          Noble Paul added a comment -

          more cleanup

          Show
          Noble Paul added a comment - more cleanup
          Hide
          Anshum Gupta added a comment -

          Updated patch with a few changes.

          Show
          Anshum Gupta added a comment - Updated patch with a few changes.
          Hide
          Anshum Gupta added a comment - - edited

          I'm trying to see if there's a way to avoid re-processing during a RETRY but I think that can also come in later.

          Show
          Anshum Gupta added a comment - - edited I'm trying to see if there's a way to avoid re-processing during a RETRY but I think that can also come in later.
          Hide
          Anshum Gupta added a comment -

          Changed the access of most variables in SolrCall to be private and added a getter for path as it's accessed from the SDF.

          Show
          Anshum Gupta added a comment - Changed the access of most variables in SolrCall to be private and added a getter for path as it's accessed from the SDF.
          Hide
          Anshum Gupta added a comment -

          Some more refactoring, it's still WIP but I'll continue to build on this tomorrow morning. Still need to move stuff into smaller methods and document.

          This moves things into a 3 stage process for HttpSolrCall (renamed SolrCall).

          • Construct - Initialize variables
          • Set context - Sets the path, handler, etc. still working on populating it with processed information e.g. collection name etc.
          • {.call()}} - This also calls setContext and then processes the request or returns RETRY/FORWARD/etc. action. to the filter.
          Show
          Anshum Gupta added a comment - Some more refactoring, it's still WIP but I'll continue to build on this tomorrow morning. Still need to move stuff into smaller methods and document. This moves things into a 3 stage process for HttpSolrCall (renamed SolrCall). Construct - Initialize variables Set context - Sets the path, handler, etc. still working on populating it with processed information e.g. collection name etc. {.call()}} - This also calls setContext and then processes the request or returns RETRY/FORWARD/etc. action. to the filter.
          Hide
          Noble Paul added a comment -

          Please separate out the SOLR-7275 changes and let's stay true to the description of the ticket

          Show
          Noble Paul added a comment - Please separate out the SOLR-7275 changes and let's stay true to the description of the ticket
          Hide
          Anshum Gupta added a comment -

          Updated patch, without the SolrRequestContext and a few more methods extracted out. I've just moved the methods out with some comments around the call and haven't really changed much as I wouldn't want to make this an invasive change.
          We can revisit this once after the first commit I think.

          Show
          Anshum Gupta added a comment - Updated patch, without the SolrRequestContext and a few more methods extracted out. I've just moved the methods out with some comments around the call and haven't really changed much as I wouldn't want to make this an invasive change. We can revisit this once after the first commit I think.
          Hide
          Anshum Gupta added a comment -

          Fixes an NPE.

          Show
          Anshum Gupta added a comment - Fixes an NPE.
          Hide
          ASF subversion and git services added a comment -

          Commit 1677644 from Anshum Gupta in branch 'dev/trunk'
          [ https://svn.apache.org/r1677644 ]

          SOLR-7484: Refactor SolrDispatchFilter to extract all Solr specific implementation detail to HttpSolrCall and also extract methods from within the current SDF.doFilter(..) logic making things easier to manage. HttpSolrCall converts the processing to a 3-step process i.e. Construct, Init, and Call so the context of the request would be available after Init and before the actual call operation.

          Show
          ASF subversion and git services added a comment - Commit 1677644 from Anshum Gupta in branch 'dev/trunk' [ https://svn.apache.org/r1677644 ] SOLR-7484 : Refactor SolrDispatchFilter to extract all Solr specific implementation detail to HttpSolrCall and also extract methods from within the current SDF.doFilter(..) logic making things easier to manage. HttpSolrCall converts the processing to a 3-step process i.e. Construct, Init, and Call so the context of the request would be available after Init and before the actual call operation.
          Hide
          ASF subversion and git services added a comment -

          Commit 1677660 from Anshum Gupta in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1677660 ]

          SOLR-7484: Refactor SolrDispatchFilter to extract all Solr specific implementation detail to HttpSolrCall and also extract methods from within the current SDF.doFilter(..) logic making things easier to manage. HttpSolrCall converts the processing to a 3-step process i.e. Construct, Init, and Call so the context of the request would be available after Init and before the actual call operation.(merge from trunk)

          Show
          ASF subversion and git services added a comment - Commit 1677660 from Anshum Gupta in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1677660 ] SOLR-7484 : Refactor SolrDispatchFilter to extract all Solr specific implementation detail to HttpSolrCall and also extract methods from within the current SDF.doFilter(..) logic making things easier to manage. HttpSolrCall converts the processing to a 3-step process i.e. Construct, Init, and Call so the context of the request would be available after Init and before the actual call operation.(merge from trunk)
          Hide
          Noble Paul added a comment -

          should you not remove those redundant cases in switch ?

          Show
          Noble Paul added a comment - should you not remove those redundant cases in switch ?
          Hide
          Anshum Gupta added a comment -

          Sure, I'll commit that patch. Thanks.

          Show
          Anshum Gupta added a comment - Sure, I'll commit that patch. Thanks.
          Hide
          ASF subversion and git services added a comment -

          Commit 1678071 from Anshum Gupta in branch 'dev/trunk'
          [ https://svn.apache.org/r1678071 ]

          SOLR-7484: Cleaning up redundant switch cases from the code

          Show
          ASF subversion and git services added a comment - Commit 1678071 from Anshum Gupta in branch 'dev/trunk' [ https://svn.apache.org/r1678071 ] SOLR-7484 : Cleaning up redundant switch cases from the code
          Hide
          ASF subversion and git services added a comment -

          Commit 1678073 from Anshum Gupta in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1678073 ]

          SOLR-7484: Cleaning up redundant switch cases from the code (merge from trunk)

          Show
          ASF subversion and git services added a comment - Commit 1678073 from Anshum Gupta in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1678073 ] SOLR-7484 : Cleaning up redundant switch cases from the code (merge from trunk)
          Hide
          Ryan McKinley added a comment -

          Here is a simple patch that puts back the ability to override sendError() and execute()

          Previously this was exposed on SolrDispatchFilter, but now we need to make it possible to send a custom HttpSolrCall

          Show
          Ryan McKinley added a comment - Here is a simple patch that puts back the ability to override sendError() and execute() Previously this was exposed on SolrDispatchFilter, but now we need to make it possible to send a custom HttpSolrCall
          Hide
          ASF subversion and git services added a comment -

          Commit 1683024 from Ryan McKinley in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1683024 ]

          SOLR-7484: make it possible to override sendError and execute

          Show
          ASF subversion and git services added a comment - Commit 1683024 from Ryan McKinley in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1683024 ] SOLR-7484 : make it possible to override sendError and execute
          Hide
          ASF subversion and git services added a comment -

          Commit 1683025 from Ryan McKinley in branch 'dev/trunk'
          [ https://svn.apache.org/r1683025 ]

          Merged revision(s) 1683024 from lucene/dev/branches/branch_5x:
          SOLR-7484: make it possible to override sendError and execute
          ........

          Show
          ASF subversion and git services added a comment - Commit 1683025 from Ryan McKinley in branch 'dev/trunk' [ https://svn.apache.org/r1683025 ] Merged revision(s) 1683024 from lucene/dev/branches/branch_5x: SOLR-7484 : make it possible to override sendError and execute ........
          Hide
          Hoss Man added a comment -

          ryan: these changes should be tracked in a distinct jira with it's own entry in CHANGES.txt to keep clear what functionality was included in 5.2 and what came later.

          Show
          Hoss Man added a comment - ryan: these changes should be tracked in a distinct jira with it's own entry in CHANGES.txt to keep clear what functionality was included in 5.2 and what came later.
          Hide
          Ryan McKinley added a comment -

          See SOLR-7623 for an issue to track the functional regression

          (thanks Chris Hostetter (Unused))

          Show
          Ryan McKinley added a comment - See SOLR-7623 for an issue to track the functional regression (thanks Chris Hostetter (Unused) )
          Hide
          Anshum Gupta added a comment -

          Closing it as it's already released.

          Show
          Anshum Gupta added a comment - Closing it as it's already released.

            People

            • Assignee:
              Anshum Gupta
              Reporter:
              Anshum Gupta
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development