Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-9412

Update Macro Expander for replacement logic

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.x, master (7.0)
    • Component/s: None
    • Security Level: Public (Default Security Level. Issues are Public)
    • Labels:
      None
    • Flags:
      Patch

      Description

      MacroExpander class was updated to allow to return null when replacement parameters are missing. Right now it defaults to a blank space and it isn't easily verifiable that a parameter was missing. Additionally, unit tests were added for this case and the original use cases of Macro Expander.

      1. SOLR-9412.patch
        6 kB
        Christine Poerschke

        Issue Links

          Activity

          Hide
          cpoerschke Christine Poerschke added a comment -

          Hi Jon, thanks for your patch.

          Please find attached some suggested revisions. What do you think?

          MacroExpander:

          • param/member spell correction
          • make the new flag final
          • constructor style tweak

          TestMacroExpander:

          • assertEquals arg order needed swapping
          • split out the tests:
            • testExamples via its random element tests and shows that existing/example behavior is unaffected by absence or presence or value of the new parameter
            • testOnMissingParams tests the expansion behavior differences, with coverage expanded (no pun intended) so that either or both of the params could be missing
          Show
          cpoerschke Christine Poerschke added a comment - Hi Jon, thanks for your patch. Please find attached some suggested revisions. What do you think? MacroExpander: param/member spell correction make the new flag final constructor style tweak TestMacroExpander: assertEquals arg order needed swapping split out the tests: testExamples via its random element tests and shows that existing/example behavior is unaffected by absence or presence or value of the new parameter testOnMissingParams tests the expansion behavior differences, with coverage expanded (no pun intended) so that either or both of the params could be missing
          Hide
          cpoerschke Christine Poerschke added a comment -

          re: the random(ized) elements in the test and LuceneTestCase, Dawid Weiss's talk at Berlin Buzzwords is a good intro to the concept etc.

          Show
          cpoerschke Christine Poerschke added a comment - re: the random(ized) elements in the test and LuceneTestCase, Dawid Weiss 's talk at Berlin Buzzwords is a good intro to the concept etc.
          Hide
          jdorando Jon Dorando added a comment -

          This is really good! Thanks for the suggestions!

          Show
          jdorando Jon Dorando added a comment - This is really good! Thanks for the suggestions!
          Hide
          yseeley@gmail.com Yonik Seeley added a comment -

          +1 to the idea, and to the latest patch.

          Show
          yseeley@gmail.com Yonik Seeley added a comment - +1 to the idea, and to the latest patch.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit c9c2d5537adede62313e51186b75fab08fc6ad1f in lucene-solr's branch refs/heads/master from Christine Poerschke
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c9c2d55 ]

          SOLR-9412: Add failOnMissingParams option to MacroExpander, add TestMacroExpander class. (Jon Dorando, Christine Poerschke)

          Show
          jira-bot ASF subversion and git services added a comment - Commit c9c2d5537adede62313e51186b75fab08fc6ad1f in lucene-solr's branch refs/heads/master from Christine Poerschke [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c9c2d55 ] SOLR-9412 : Add failOnMissingParams option to MacroExpander, add TestMacroExpander class. (Jon Dorando, Christine Poerschke)
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit c2a8814df23ddbb07c6d02fd72e07334aad88692 in lucene-solr's branch refs/heads/branch_6x from Christine Poerschke
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c2a8814 ]

          SOLR-9412: Add failOnMissingParams option to MacroExpander, add TestMacroExpander class. (Jon Dorando, Christine Poerschke)

          Show
          jira-bot ASF subversion and git services added a comment - Commit c2a8814df23ddbb07c6d02fd72e07334aad88692 in lucene-solr's branch refs/heads/branch_6x from Christine Poerschke [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c2a8814 ] SOLR-9412 : Add failOnMissingParams option to MacroExpander, add TestMacroExpander class. (Jon Dorando, Christine Poerschke)
          Hide
          cpoerschke Christine Poerschke added a comment -

          Thanks Jon!

          Show
          cpoerschke Christine Poerschke added a comment - Thanks Jon!

            People

            • Assignee:
              cpoerschke Christine Poerschke
              Reporter:
              jdorando Jon Dorando
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development