Solr
  1. Solr
  2. SOLR-2331

Refactor CoreContainer's SolrXML serialization code and improve testing

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0-ALPHA
    • Component/s: multicore
    • Labels:
      None

      Description

      CoreContainer has enough code in it - I'd like to factor out the solr.xml serialization code into SolrXMLSerializer or something - which should make testing it much easier and lightweight.

        Issue Links

          Activity

          Hide
          Mark Miller added a comment -

          Here is a first pass - more to come.

          Show
          Mark Miller added a comment - Here is a first pass - more to come.
          Hide
          Chris Male added a comment -

          Hey Mark,

          +1 to this. Is it possible to go one step further and pull out the load functionality too? Then they both can go into a class which manages the reading and writing. It'd be a nice step towards abstracting away from the solr.xml file being the only way to configure a CoreContainer.

          Show
          Chris Male added a comment - Hey Mark, +1 to this. Is it possible to go one step further and pull out the load functionality too? Then they both can go into a class which manages the reading and writing. It'd be a nice step towards abstracting away from the solr.xml file being the only way to configure a CoreContainer.
          Hide
          Mark Miller added a comment -

          one step further and pull out the load functionality

          Yeah, I think the idea is good. I think some of the 'load' code could also be pulled up into the SolrXMLSerializer.

          If no one objects, I'd like to commit this part soon though - then tackle the load side later.

          Show
          Mark Miller added a comment - one step further and pull out the load functionality Yeah, I think the idea is good. I think some of the 'load' code could also be pulled up into the SolrXMLSerializer. If no one objects, I'd like to commit this part soon though - then tackle the load side later.
          Hide
          Steve Rowe added a comment -

          I reopened SOLR-2500 because TestSolrProperties is failing consistently on Windows 7/Oracle JDK 1.6.0_21 for me, but it appears that this is the issue that introduced the problem.

          I've tracked the issue down to the anonymous FileInputStream created in order to print out the contents of the persisted core configuration to STDOUT – the following line was uncommented when Mark committed the patch on this issue:

          206:  System.out.println(IOUtils.toString(new FileInputStream(new File(solrXml.getParent(), "solr-persist.xml"))));
          

          This patch de-anonymizes the FileInputStream and closes it after the file contents are printed out.

          I plan to commit this later tonight.

          Show
          Steve Rowe added a comment - I reopened SOLR-2500 because TestSolrProperties is failing consistently on Windows 7/Oracle JDK 1.6.0_21 for me, but it appears that this is the issue that introduced the problem. I've tracked the issue down to the anonymous FileInputStream created in order to print out the contents of the persisted core configuration to STDOUT – the following line was uncommented when Mark committed the patch on this issue: 206: System .out.println(IOUtils.toString( new FileInputStream( new File(solrXml.getParent(), "solr-persist.xml" )))); This patch de-anonymizes the FileInputStream and closes it after the file contents are printed out. I plan to commit this later tonight.
          Hide
          Steve Rowe added a comment -

          This version of the patch wraps the persisted core config printing to STDOUT in an if (VERBOSE) block.

          Committing shortly.

          Show
          Steve Rowe added a comment - This version of the patch wraps the persisted core config printing to STDOUT in an if (VERBOSE) block. Committing shortly.
          Hide
          Steve Rowe added a comment -

          This patch de-anonymizes the FileInputStream and closes it after the file contents are printed out [and] wraps the persisted core config printing to STDOUT in an if (VERBOSE) block.

          Committed in r1144088.

          Show
          Steve Rowe added a comment - This patch de-anonymizes the FileInputStream and closes it after the file contents are printed out [and] wraps the persisted core config printing to STDOUT in an if (VERBOSE) block. Committed in r1144088.
          Hide
          Bill Bell added a comment -

          Commit? +1

          Show
          Bill Bell added a comment - Commit? +1
          Hide
          Mark Miller added a comment - - edited

          Thanks Steve - calling this one done - CoreContainer needs more refactoring, but it can come in further issues. Noble has a great one going to factor out zookeeper parts as well.

          Show
          Mark Miller added a comment - - edited Thanks Steve - calling this one done - CoreContainer needs more refactoring, but it can come in further issues. Noble has a great one going to factor out zookeeper parts as well.
          Hide
          Yury Kats added a comment -

          Looks like this introduced a regression in solr.xml persistence.
          See SOLR-2691.

          Show
          Yury Kats added a comment - Looks like this introduced a regression in solr.xml persistence. See SOLR-2691 .

            People

            • Assignee:
              Mark Miller
              Reporter:
              Mark Miller
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development