Solr
  1. Solr
  2. SOLR-748

FacetComponent helper classes are package restricted

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3
    • Fix Version/s: 1.4
    • Component/s: search
    • Labels:
      None

      Description

      Original discussion:
      http://www.nabble.com/Package-Access-Issues---Extending-FacetComponent-to19148122.html

      The FacetComponent class uses several helper classes that currently have package-restricted access. This makes it impossible to extend the FacetComponent without rewriting most of its functionality.

      A proposed solution is to make those classes public and make their public member variables accessibly only through get and set functions (i.e. make them private).

      1. 748.patch
        28 kB
        Wojtek Piaseczny
      2. SOLR-748.patch
        20 kB
        Wojtek Piaseczny
      3. SOLR-748.patch
        36 kB
        Wojtek Piaseczny
      4. SOLR-748.patch
        27 kB
        Wojtek Piaseczny
      5. SOLR-748.patch
        26 kB
        Wojtek Piaseczny

        Activity

        Hide
        Wojtek Piaseczny added a comment -

        Implements the proposed solution.

        Show
        Wojtek Piaseczny added a comment - Implements the proposed solution.
        Hide
        Wojtek Piaseczny added a comment -

        A more detailed list of changes:

        ShardFacetCount, DistribFieldFacet, and FacetInfo classes become final public. Their member variables become private, and are accessible (get & set) through public accessors.

        FieldFacet becomes a public class. Its member variables become protected, and are accessible (get & set) through public accessors.

        ResponseBuilder's private member variable _facetInfo renamed to facetInfo and made public.

        FacetComponent uses public accessors to access class members.

        Show
        Wojtek Piaseczny added a comment - A more detailed list of changes: ShardFacetCount, DistribFieldFacet, and FacetInfo classes become final public. Their member variables become private, and are accessible (get & set) through public accessors. FieldFacet becomes a public class. Its member variables become protected, and are accessible (get & set) through public accessors. ResponseBuilder's private member variable _facetInfo renamed to facetInfo and made public. FacetComponent uses public accessors to access class members.
        Hide
        Wojtek Piaseczny added a comment -

        SOLR-755 broke the previous patch. This patch contains no new functionality.

        Show
        Wojtek Piaseczny added a comment - SOLR-755 broke the previous patch. This patch contains no new functionality.
        Hide
        Wojtek Piaseczny added a comment -

        Are there any objections to committing this patch?

        I realize I didn't mention exactly what I'm using this for. I want to override the finishStage method of the FacetComponent class in my own facet component. To use values calculated by the default behavior of the FacetComponent class, I need several classes to be public (the ones listed above).

        Show
        Wojtek Piaseczny added a comment - Are there any objections to committing this patch? I realize I didn't mention exactly what I'm using this for. I want to override the finishStage method of the FacetComponent class in my own facet component. To use values calculated by the default behavior of the FacetComponent class, I need several classes to be public (the ones listed above).
        Hide
        Erik Hatcher added a comment -

        I'll take this one.

        Show
        Erik Hatcher added a comment - I'll take this one.
        Hide
        Wojtek Piaseczny added a comment -

        Updated patch for Jan 26th trunk.

        Show
        Wojtek Piaseczny added a comment - Updated patch for Jan 26th trunk.
        Hide
        Wojtek Piaseczny added a comment -

        Previous patch didn't apply cleanly.

        Show
        Wojtek Piaseczny added a comment - Previous patch didn't apply cleanly.
        Hide
        Wojtek Piaseczny added a comment -

        Updated patch for April 24th trunk code.

        Show
        Wojtek Piaseczny added a comment - Updated patch for April 24th trunk code.
        Hide
        Shalin Shekhar Mangar added a comment -

        Wojtek, how about we just make the following changes:

        1. Inner classes as public static
        2. The class attributes as public
        3. Add an experimental API note to each of these classes

        I don't prefer keeping top-level classes which are used in one place only. The getter/setters seem to be just noise. They may give us some flexibility in the future but in this case that might not be needed?

        What do you think?

        Show
        Shalin Shekhar Mangar added a comment - Wojtek, how about we just make the following changes: Inner classes as public static The class attributes as public Add an experimental API note to each of these classes I don't prefer keeping top-level classes which are used in one place only. The getter/setters seem to be just noise. They may give us some flexibility in the future but in this case that might not be needed? What do you think?
        Hide
        Wojtek Piaseczny added a comment -

        Shalin,

        1. Makes sense.
        2. Using public access seems dangerous, but is consistent with a lot of the code in this project, so it's probably the right solution.
        3. Makes sense.

        I'll post a patch shortly.

        (Also, thanks for taking this issue!)

        Show
        Wojtek Piaseczny added a comment - Shalin, 1. Makes sense. 2. Using public access seems dangerous, but is consistent with a lot of the code in this project, so it's probably the right solution. 3. Makes sense. I'll post a patch shortly. (Also, thanks for taking this issue!)
        Hide
        Wojtek Piaseczny added a comment - - edited

        Patch implements Shalin's suggested changes.

        Show
        Wojtek Piaseczny added a comment - - edited Patch implements Shalin's suggested changes.
        Hide
        Shalin Shekhar Mangar added a comment -

        Committed revision 770431.

        Thanks Wojtek!

        Show
        Shalin Shekhar Mangar added a comment - Committed revision 770431. Thanks Wojtek!
        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:
            Shalin Shekhar Mangar
            Reporter:
            Wojtek Piaseczny
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development