Solr
  1. Solr
  2. SOLR-7298

Fix Collections API SolrJ calls to not add name parameter when not needed

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.0
    • Fix Version/s: 5.1, 6.0
    • Component/s: SolrCloud, SolrJ
    • Labels:
      None

      Description

      Collections API SolrJ calls add the name parameter for a lot of calls incorrectly. It should only be added when required.

      1. collection_api_params.xls
        44 kB
        Anshum Gupta
      2. SOLR-7298.patch
        19 kB
        Shai Erera
      3. SOLR-7298.patch
        5 kB
        Shai Erera

        Activity

        Hide
        Anshum Gupta added a comment -

        This would be useful in looking at and confirming the design/inheritance of classes.

        Show
        Anshum Gupta added a comment - This would be useful in looking at and confirming the design/inheritance of classes.
        Hide
        Shai Erera added a comment -

        Thanks Anshum. I took a look and have some comments:

        • REBALANCELEADERS: I don't see any Request object for this?
        • CREATEALIAS and DELETEALIAS: their 'name' parameter denotes something different than Create/Delete (it's the alias name vs the collection name)
        • CLUSTERSTATUS: also takes a collection and shard parameters, so it can remain under CollectionShardAdminRequest
        • MIGRATE: only needs collection, not shard, so I moved it to extend CollectionAdminRequest.
        Show
        Shai Erera added a comment - Thanks Anshum. I took a look and have some comments: REBALANCELEADERS: I don't see any Request object for this? CREATEALIAS and DELETEALIAS: their 'name' parameter denotes something different than Create/Delete (it's the alias name vs the collection name) CLUSTERSTATUS: also takes a collection and shard parameters, so it can remain under CollectionShardAdminRequest MIGRATE: only needs collection, not shard, so I moved it to extend CollectionAdminRequest.
        Hide
        Anshum Gupta added a comment -

        Thanks Shai. I'll take a look at this.

        Show
        Anshum Gupta added a comment - Thanks Shai. I'll take a look at this.
        Hide
        Shai Erera added a comment -

        Added CollectionAdminRequestRequiredParamsTest which verifies that all requests' default parameters are only their required parameters and none else. If a request changes in the future, the test will break and it will remind us to check documentation etc.

        Also, I moved ClusterStatus to extend CollectionAdminRequest directly, at the expense of adding setter/getter for collection and shard names, as these are optional parameters for this request, yet CollectionShardAdminRequest always adds them. Because they are null (if unset), ModifiableSolrParams eventually does not set them, but I consider this magic and not desired behavior.

        Show
        Shai Erera added a comment - Added CollectionAdminRequestRequiredParamsTest which verifies that all requests' default parameters are only their required parameters and none else. If a request changes in the future, the test will break and it will remind us to check documentation etc. Also, I moved ClusterStatus to extend CollectionAdminRequest directly, at the expense of adding setter/getter for collection and shard names, as these are optional parameters for this request, yet CollectionShardAdminRequest always adds them. Because they are null (if unset), ModifiableSolrParams eventually does not set them, but I consider this magic and not desired behavior.
        Hide
        Anshum Gupta added a comment -

        Thanks Shai. This looks good to me.
        REBALANCELEADERS doesn't have support in SolrJ yet. Seems like it was added without SolrJ support.

        You're right about #2, #3 and #4 too.

        Show
        Anshum Gupta added a comment - Thanks Shai. This looks good to me. REBALANCELEADERS doesn't have support in SolrJ yet. Seems like it was added without SolrJ support. You're right about #2, #3 and #4 too.
        Hide
        ASF subversion and git services added a comment -

        Commit 1669426 from Shai Erera in branch 'dev/trunk'
        [ https://svn.apache.org/r1669426 ]

        SOLR-7298: Fix Collections API SolrJ calls to not add name parameter when not needed

        Show
        ASF subversion and git services added a comment - Commit 1669426 from Shai Erera in branch 'dev/trunk' [ https://svn.apache.org/r1669426 ] SOLR-7298 : Fix Collections API SolrJ calls to not add name parameter when not needed
        Hide
        ASF subversion and git services added a comment -

        Commit 1669428 from Shai Erera in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1669428 ]

        SOLR-7298: Fix Collections API SolrJ calls to not add name parameter when not needed

        Show
        ASF subversion and git services added a comment - Commit 1669428 from Shai Erera in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1669428 ] SOLR-7298 : Fix Collections API SolrJ calls to not add name parameter when not needed
        Hide
        Shai Erera added a comment -

        Committed to trunk and 5x. Thanks Anshum for the review!

        Show
        Shai Erera added a comment - Committed to trunk and 5x. Thanks Anshum for the review!
        Hide
        Timothy Potter added a comment -

        Bulk close after 5.1 release

        Show
        Timothy Potter added a comment - Bulk close after 5.1 release

          People

          • Assignee:
            Shai Erera
            Reporter:
            Anshum Gupta
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development