diff --git a/pom.xml b/pom.xml index c19df9c..a4b3898 100644 --- a/pom.xml +++ b/pom.xml @@ -58,6 +58,12 @@ junit-jupiter test + + org.mockito + mockito-core + 5.11.0 + test + diff --git a/src/main/java/com/github/olivergondza/saxeed/Saxeed.java b/src/main/java/com/github/olivergondza/saxeed/Saxeed.java index 2847165..a1537c6 100644 --- a/src/main/java/com/github/olivergondza/saxeed/Saxeed.java +++ b/src/main/java/com/github/olivergondza/saxeed/Saxeed.java @@ -30,12 +30,11 @@ public class Saxeed { private SAXParser saxParser; - private XMLOutputFactory xmlOutputFactory; private InputSource input; private final Map transformations = new LinkedHashMap<>(); public Saxeed() { - setXmlOutputFactory(null); // Initialize with defaults + } public Saxeed setSaxParser(SAXParser saxParser) { @@ -44,14 +43,6 @@ public Saxeed setSaxParser(SAXParser saxParser) { return this; } - public Saxeed setXmlOutputFactory(XMLOutputFactory xmlOutputFactory) { - this.xmlOutputFactory = xmlOutputFactory != null - ? xmlOutputFactory - : XMLOutputFactory.newInstance() - ; - return this; - } - public Saxeed setInput(Path path) { input = new InputSource(path.toFile().toURI().toASCIIString()); return this; @@ -167,7 +158,7 @@ private MultiplexingHandler getSaxHandler() { Target target = trans.getValue(); TransformationBuilder builder = trans.getKey(); - return target.getHandler(builder, xmlOutputFactory); + return builder.build(this, target); }).collect(Collectors.toList()); return new MultiplexingHandler(handlers); } diff --git a/src/main/java/com/github/olivergondza/saxeed/Target.java b/src/main/java/com/github/olivergondza/saxeed/Target.java index 549c542..8a6b629 100644 --- a/src/main/java/com/github/olivergondza/saxeed/Target.java +++ b/src/main/java/com/github/olivergondza/saxeed/Target.java @@ -1,8 +1,8 @@ package com.github.olivergondza.saxeed; import com.github.olivergondza.saxeed.ex.FailedWriting; -import com.github.olivergondza.saxeed.internal.TransformationHandler; +import javax.xml.stream.FactoryConfigurationError; import javax.xml.stream.XMLOutputFactory; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamWriter; @@ -16,22 +16,29 @@ /** * Target to write the resulting content into. */ -public interface Target { +public abstract class Target implements AutoCloseable { + + private AutoCloseable close = null; /** * Identify the target for the ease of debugging. * * Mostly used in error messages to identify the file/stream/etc. */ - String getName(); + public abstract String getName(); - TransformationHandler getHandler(TransformationBuilder builder, XMLOutputFactory xmlOutputFactory); + /** + * Create new XMLStreamWriter for the transformation. + * + * If it is desirable to close any resource allocated, register it by {@link #registerClosable(AutoCloseable)}. + */ + public abstract XMLStreamWriter getWriter(Saxeed saxeed); - default XMLStreamWriter create(XMLOutputFactory xmlOutputFactory, OutputStream os) { + public static XMLStreamWriter createXmlStreamWriter(OutputStream os) { try { - return xmlOutputFactory.createXMLStreamWriter(os); - } catch (XMLStreamException e) { - throw new FailedWriting("Unable to create XMLStreamWriter for " + getName(), e); + return XMLOutputFactory.newInstance().createXMLStreamWriter(os); + } catch (FactoryConfigurationError | XMLStreamException e) { + throw new FailedWriting("Unable to create XMLStreamWriter from " + objectId(os), e); } } @@ -40,13 +47,39 @@ private static String objectId(Object obj) { return obj.getClass().getName() + "@" + Integer.toHexString(obj.hashCode()); } + /** + * Register single resource for closing on {@link #close()}. + */ + protected void registerClosable(AutoCloseable close) { + if (this.close != null) throw new IllegalStateException("Unclosed resource already present: " + this.close); + + this.close = close; + } + + /** + * Close method is called on target every time a transformation using a target is completed. + * + * It closes whatever resource was registered using {@link #registerClosable(AutoCloseable)}, or nothing if not set. + */ + @Override + public void close() throws FailedWriting { + if (close != null) { + try { + close.close(); + close = null; + } catch (Exception e) { + throw new FailedWriting("Failed closing target " + getName(), e); + } + } + } + /** * Target for a file. * * The content is flush/closed once the transformation is over. It means that using same File target repeatedly will * overwrite its content. */ - class FileTarget implements Target { + static class FileTarget extends Target { private final File file; public FileTarget(File file) { @@ -63,12 +96,13 @@ public String getName() { } @Override - public TransformationHandler getHandler(TransformationBuilder builder, XMLOutputFactory xmlOutputFactory) { + public XMLStreamWriter getWriter(Saxeed saxeed) { OutputStream os = getOutputStream(); - return builder.build(create(xmlOutputFactory, os), os); + registerClosable(os); + return createXmlStreamWriter(os); } - private OutputStream getOutputStream() throws FailedWriting { + protected /*for testing*/ OutputStream getOutputStream() throws FailedWriting { try { return new BufferedOutputStream(new FileOutputStream(file)); } catch (FileNotFoundException e) { @@ -82,7 +116,7 @@ private OutputStream getOutputStream() throws FailedWriting { * * The stream is NOT closed. */ - class OutputStreamTarget implements Target { + static class OutputStreamTarget extends Target { private final OutputStream os; public OutputStreamTarget(OutputStream os) { @@ -95,12 +129,12 @@ public String getName() { } @Override - public TransformationHandler getHandler(TransformationBuilder builder, XMLOutputFactory xmlOutputFactory) { - return builder.build(create(xmlOutputFactory, os), null); + public XMLStreamWriter getWriter(Saxeed saxeed) { + return createXmlStreamWriter(os); } } - class DevNullTarget extends OutputStreamTarget { + static class DevNullTarget extends OutputStreamTarget { private static final OutputStream outputStream = new OutputStream() { @Override @@ -119,7 +153,7 @@ public String getName() { } } - class XmlStreamWriterTarget implements Target { + static class XmlStreamWriterTarget extends Target { private final XMLStreamWriter writer; @@ -133,8 +167,8 @@ public String getName() { } @Override - public TransformationHandler getHandler(TransformationBuilder builder, XMLOutputFactory __) { - return builder.build(writer, null); + public XMLStreamWriter getWriter(Saxeed saxeed) { + return writer; } } } diff --git a/src/main/java/com/github/olivergondza/saxeed/TransformationBuilder.java b/src/main/java/com/github/olivergondza/saxeed/TransformationBuilder.java index a5eab3f..cb13551 100644 --- a/src/main/java/com/github/olivergondza/saxeed/TransformationBuilder.java +++ b/src/main/java/com/github/olivergondza/saxeed/TransformationBuilder.java @@ -2,7 +2,6 @@ import com.github.olivergondza.saxeed.internal.TransformationHandler; -import javax.xml.stream.XMLStreamWriter; import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; @@ -37,7 +36,7 @@ public TransformationBuilder add(Subscribed subs, Collection vi return this; } - public TransformationHandler build(XMLStreamWriter writer, AutoCloseable closeAction) { - return new TransformationHandler(visitors, writer, closeAction); + public TransformationHandler build(Saxeed saxeed, Target target) { + return new TransformationHandler(saxeed, target, visitors); } } diff --git a/src/main/java/com/github/olivergondza/saxeed/internal/TransformationHandler.java b/src/main/java/com/github/olivergondza/saxeed/internal/TransformationHandler.java index b034454..cd18ddd 100644 --- a/src/main/java/com/github/olivergondza/saxeed/internal/TransformationHandler.java +++ b/src/main/java/com/github/olivergondza/saxeed/internal/TransformationHandler.java @@ -1,7 +1,9 @@ package com.github.olivergondza.saxeed.internal; +import com.github.olivergondza.saxeed.Saxeed; import com.github.olivergondza.saxeed.Subscribed; import com.github.olivergondza.saxeed.TagName; +import com.github.olivergondza.saxeed.Target; import com.github.olivergondza.saxeed.UpdatingVisitor; import com.github.olivergondza.saxeed.ex.FailedTransforming; import com.github.olivergondza.saxeed.ex.FailedWriting; @@ -36,8 +38,7 @@ public class TransformationHandler extends DefaultHandler implements AutoCloseab private final Map> visitorCache = new HashMap<>(); private final XMLStreamWriter writer; - // A resource to close once done. Can be null - private final AutoCloseable closeAction; + private final Target target; private TagImpl currentTag; private final CharChunk currentChars = new CharChunk(); @@ -49,13 +50,12 @@ public class TransformationHandler extends DefaultHandler implements AutoCloseab private final Map writtenBookmarks = new HashMap<>(); public TransformationHandler( - LinkedHashMap visitors, - XMLStreamWriter writer, - AutoCloseable closeAction + Saxeed saxeed, + Target target, LinkedHashMap visitors ) { this.visitors = visitors; - this.writer = writer; - this.closeAction = closeAction; + this.target = target; + this.writer = target.getWriter(saxeed); } @Override @@ -328,19 +328,8 @@ public void endDocument() { @Override public void close() throws FailedWriting { try { - // Make sure the content is written to XMLStreamWriter, no matter if closing the target or not writer.flush(); - - // Closing XMLStreamWriter never close the target, but it will be un-writable afterward. - // So closing writer iff the target should be closed. - if (closeAction != null) { - // Hackish: Need to close 2 resources, and need to preserve both the eventual exceptions from close() - exactly what - // an empty try-with-resources would do. But, the XMLStreamWriter does not implement AutoClosable, so a compromise - // approach is needed. - try (closeAction) { - writer.close(); - } - } + target.close(); } catch (Exception e) { throw new FailedWriting("Failed closing stream", e); } diff --git a/src/test/java/com/github/olivergondza/saxeed/ClosingTest.java b/src/test/java/com/github/olivergondza/saxeed/ClosingTest.java index e8b7792..b608580 100644 --- a/src/test/java/com/github/olivergondza/saxeed/ClosingTest.java +++ b/src/test/java/com/github/olivergondza/saxeed/ClosingTest.java @@ -1,31 +1,38 @@ package com.github.olivergondza.saxeed; -import com.github.olivergondza.saxeed.internal.TransformationHandler; +import com.github.olivergondza.saxeed.ex.FailedWriting; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; -import javax.xml.stream.XMLOutputFactory; import javax.xml.stream.XMLStreamWriter; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.io.PrintStream; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.List; import java.util.function.Supplier; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Verify targets are (not) closed as documented. */ class ClosingTest { - private static final class ClosingTarget implements Target, AutoCloseable { + private static final class ClosingTarget extends Target { - private final boolean close; private final List closes = new ArrayList<>(); + private final AutoCloseable closeTracker; public ClosingTarget(boolean close) { - this.close = close; + closeTracker = close + ? (() -> closes.add(new Throwable())) + : null + ; } @Override @@ -34,14 +41,9 @@ public String getName() { } @Override - public TransformationHandler getHandler(TransformationBuilder builder, XMLOutputFactory xmlOutputFactory) { - XMLStreamWriter writer = create(xmlOutputFactory, new ByteArrayOutputStream()); - return builder.build(writer, close ? this : null); - } - - @Override - public void close() throws Exception { - this.closes.add(new Throwable()); + public XMLStreamWriter getWriter(Saxeed saxeed) { + registerClosable(closeTracker); + return createXmlStreamWriter(new ByteArrayOutputStream()); } public void assertClosedTimes(int expected) { @@ -63,30 +65,29 @@ public void assertClosedTimes(int expected) { } @Test - void close() { - ClosingTarget closeMe = new ClosingTarget(true); - new Saxeed().setInputString("") - .addTransformation(new TransformationBuilder(), closeMe) - .transform(); - - closeMe.assertClosedTimes(1); - - closeMe = new ClosingTarget(true); - new Saxeed().setInputString("") - .addTransformation(new TransformationBuilder(), closeMe) - .addTransformation(new TransformationBuilder(), closeMe) - .addTransformation(new TransformationBuilder(), closeMe) - .transform(); - - closeMe.assertClosedTimes(3); - } + void closeFileTarget() throws Exception { + final boolean[] closed = {false}; + Path out = Files.createTempFile("saxeed", getClass().getName()); + Target.FileTarget target = new Target.FileTarget(out) { + @Override + protected OutputStream getOutputStream() throws FailedWriting { + return new OutputStream() { + @Override + public void write(int b) throws IOException { + + } + + @Override + public void close() throws IOException { + closed[0] = true; + } + }; + } + }; - @Test - void doNotClose() { - ClosingTarget doNotCloseMe = new ClosingTarget(false); - new Saxeed().setInputString("").addTransformation(new TransformationBuilder(), doNotCloseMe).transform(); + new Saxeed().setInputString("").addTransformation(new TransformationBuilder(), target).transform(); - doNotCloseMe.assertClosedTimes(0); + assertTrue(closed[0]); } @Test @@ -105,4 +106,20 @@ public void close() { assertEquals("", baos.toString()); } + + @Test + void doNotCloseXmlStreamWriterTarget() throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + XMLStreamWriter writer = Mockito.spy(Target.createXmlStreamWriter(baos)); + Mockito.doThrow(new AssertionError("Stream closed")) + .when(writer).close(); + + Target.XmlStreamWriterTarget target = new Target.XmlStreamWriterTarget(writer); + + writer.writeStartElement("open"); + new Saxeed().setInputString("").addTransformation(new TransformationBuilder(), target).transform(); + writer.writeEndElement(); + + assertEquals("", baos.toString()); + } } diff --git a/src/test/java/com/github/olivergondza/saxeed/MergeTest.java b/src/test/java/com/github/olivergondza/saxeed/MergeTest.java index c781ea2..83c4d81 100644 --- a/src/test/java/com/github/olivergondza/saxeed/MergeTest.java +++ b/src/test/java/com/github/olivergondza/saxeed/MergeTest.java @@ -1,30 +1,43 @@ package com.github.olivergondza.saxeed; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; import javax.xml.stream.XMLOutputFactory; import javax.xml.stream.XMLStreamWriter; import java.io.ByteArrayOutputStream; +import java.io.IOException; import static org.junit.jupiter.api.Assertions.assertEquals; class MergeTest { @Test void merge() throws Exception { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - XMLStreamWriter writer = XMLOutputFactory.newFactory().createXMLStreamWriter(baos); + ByteArrayOutputStream os = Mockito.spy(new ByteArrayOutputStream()); + XMLStreamWriter writer = Mockito.spy(Target.createXmlStreamWriter(os)); + try (os) { - writer.writeStartElement("root"); + writer.writeStartElement("root"); - new Saxeed().setInputString("").addTransformation(new TransformationBuilder(), writer).transform(); + new Saxeed().setInputString("").addTransformation(new TransformationBuilder(), writer).transform(); + Mockito.verify(writer, Mockito.never()).close(); + Mockito.verify(writer, Mockito.times(1)).flush(); - writer.writeStartElement("middle"); - writer.writeEndElement(); + writer.writeStartElement("middle"); + writer.writeEndElement(); - new Saxeed().setInputString("").addTransformation(new TransformationBuilder(), writer).transform(); + new Saxeed().setInputString("").addTransformation(new TransformationBuilder(), writer).transform(); + Mockito.verify(writer, Mockito.never()).close(); + Mockito.verify(writer, Mockito.times(2)).flush(); - writer.writeEndElement(); + writer.writeEndElement(); + assertEquals("", os.toString()); - assertEquals("", baos.toString()); + // Saxeed does not close the output stream + Mockito.verify(os, Mockito.never()).close(); + } finally { + // try-with-resources does + Mockito.verify(os, Mockito.times(1)).close(); + } } }