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

Refactor XMLWriter startTag to allow arbitrary attributes to be written

Details

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 1.4
    • 1.5, 3.1, 4.0-ALPHA
    • None
    • None
    • My MacBook laptop.

    Description

      There are certain cases in which a user would like to write arbitrary attributes as part of the XML output for a field tag. Case in point: I'd like to declare tags in the SOLR output that are e.g., georss namespace, like georss:point. Other users may want to declare myns:mytag tags, which should be perfectly legal as SOLR goes. This isn't currently possible with the XMLWriter implementation, which curiously only allows the attribute "name" to be included in the XML tags.

      Coincidentally, users of XMLWriter aren't allowed to modify the <response outer XML tag to include those arbitrary namespaces (which was my original thought as a workaround for this). This wouldn't matter anyways, because by the time the user got to the FieldType#writeXML method, the header for the XML would have been written anyways.

      I've developed a workaround, and in doing so, allowed something that should have probably been allowed in the first place: allow a user to write arbitrary attributes (including xmlns:myns="myuri") as part of the XMLWriter#startTag function. I've kept the existing #startTag, but replaced its innards with versions of startTag that include startTagWithNamespaces, and startTagNoAttrs.

      Attachments

        1. SOLR-1592.Mattmann.112209_02.patch.txt
          4 kB
          Chris A. Mattmann
        2. SOLR-1592.Mattmann.112209.patch.txt
          3 kB
          Chris A. Mattmann
        3. SOLR-1592.patch
          2 kB
          Noble Paul
        4. SOLR-1592.patch
          0.9 kB
          Noble Paul

        Issue Links

          Activity

            here's a cleaner version of the patch with a few more methods. I can add javadocs to them to explain them better, but I think this is a lot more flexible than the current methods that exist (see the patch I'm about to attach to SOLR-1586 as evidence).

            chrismattmann Chris A. Mattmann added a comment - here's a cleaner version of the patch with a few more methods. I can add javadocs to them to explain them better, but I think this is a lot more flexible than the current methods that exist (see the patch I'm about to attach to SOLR-1586 as evidence).
            noble.paul Noble Paul added a comment -

            isn't this good enough?

            noble.paul Noble Paul added a comment - isn't this good enough?
            noble.paul Noble Paul added a comment -

            added a method to write cdata also

            noble.paul Noble Paul added a comment - added a method to write cdata also

            Hey Noble:

            I like your latest patch. My +1 on it – it meets my use case (and what I'm doing with SOLR-1586). I'll attach a new patch for SOLR-1586 that uses this code if you are OK to commit it.

            Cheers,
            Chris

            chrismattmann Chris A. Mattmann added a comment - Hey Noble: I like your latest patch. My +1 on it – it meets my use case (and what I'm doing with SOLR-1586 ). I'll attach a new patch for SOLR-1586 that uses this code if you are OK to commit it. Cheers, Chris
            noble.paul Noble Paul added a comment -

            committed r884411
            Thanks Mattman

            noble.paul Noble Paul added a comment - committed r884411 Thanks Mattman

            Thanks, Noble! This works for me. +1 to resolve...

            I'll attach a patch to SOLR-1586 that leverages this new code...

            chrismattmann Chris A. Mattmann added a comment - Thanks, Noble! This works for me. +1 to resolve... I'll attach a patch to SOLR-1586 that leverages this new code...
            lancenorskog Lance Norskog added a comment -

            Please add unit tests and test against the XSLTResponseWriter. In particular please check namespace handling with XSL.

            Do the other responsewriters care about attributes?

            lancenorskog Lance Norskog added a comment - Please add unit tests and test against the XSLTResponseWriter. In particular please check namespace handling with XSL. Do the other responsewriters care about attributes?
            noble.paul Noble Paul added a comment -

            lease add unit tests and test against the XSLTResponseWriter. ?

            What has this got to do with XSLTResponseWriter? The methods added are like helper methods which can be used by custom response writers.

            noble.paul Noble Paul added a comment - lease add unit tests and test against the XSLTResponseWriter. ? What has this got to do with XSLTResponseWriter? The methods added are like helper methods which can be used by custom response writers.

            Hi Lance:

            I'd have to agree with Noble on this – the methods that were added were simple helper methods – we didn't change any core code nor did we expose any features that needed regression checks?

            Cheers,
            Chris

            chrismattmann Chris A. Mattmann added a comment - Hi Lance: I'd have to agree with Noble on this – the methods that were added were simple helper methods – we didn't change any core code nor did we expose any features that needed regression checks? Cheers, Chris
            lancenorskog Lance Norskog added a comment -

            Ok, never mind.

            lancenorskog Lance Norskog added a comment - Ok, never mind.
            hossman Chris M. Hostetter added a comment - Correcting Fix Version based on CHANGES.txt, see this thread for more details... http://mail-archives.apache.org/mod_mbox/lucene-dev/201005.mbox/%3Calpine.DEB.1.10.1005251052040.24672@radix.cryptio.net%3E

            Bulk close for 3.1.0 release

            gsingers Grant Ingersoll added a comment - Bulk close for 3.1.0 release

            People

              noble.paul Noble Paul
              chrismattmann Chris A. Mattmann
              Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: