Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes #842 - allow LogHandlers to break conversion throwing an exception #844

Merged
merged 2 commits into from
Aug 17, 2019
Merged

fixes #842 - allow LogHandlers to break conversion throwing an exception #844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
39b78fc
allow LogHandlers to break conversion throwing an exception
abelsromero Aug 15, 2019
f521d2b
update CHANGELOG
abelsromero Aug 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions CHANGELOG.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ Improvement::

* Set logger name on logged log records (@lread) (#834)

Bug Fixes::

* Remove exception protection from LogHandler in `JRubyAsciidoctor` to align behaviour with `AbstractConverter` (@abelsromero) (#844)

Build::

* Upgrade to Gradle 5.5.1 (@Fiouz) (#747)
Expand All @@ -33,7 +37,7 @@ Improvements::
* Upgrade to Asciidoctor PDF 1.5.0-alpha.18
* Added an experimental API to write Syntax Highlighters in Java (#826)

Documentation:
Documentation::

* Clarify that an InlineMacroProcessor should return a PhraseNode and that Strings are deprecated. (@jmini) (#823)

Expand All @@ -49,6 +53,7 @@ Improvements::

Bug Fixes::

{nbsp}

== 2.0.0-RC.3 (2019-04-18)

Expand All @@ -63,8 +68,7 @@ Improvements::

Bug Fixes::

* Fix logger registration when creating AsciidoctorJ instance with Asciidoctor.Factory.create(@ahus1) (#790)

* Fix logger registration when creating AsciidoctorJ instance with Asciidoctor.Factory.create (@ahus1) (#790)

== 2.0.0-RC.2 (2019-04-09)

Expand All @@ -81,7 +85,6 @@ Bug Fixes::

* Fix logger implementation (#786)


== 2.0.0-RC.1 (2019-03-27)

Enhancements::
Expand All @@ -102,7 +105,7 @@ Bug Fixes::
* Don't return null from List.blocks() and DefinitionList.blocks() (@jensnerche) (#761)
* Move org.asciidoctor.spi.ProcessorFactory to org.asciidoctor.extension (@jensnerche) (#762)

Documentation:
Documentation::

* Update documentation for running AsciidoctorJ in OSGi (@twasyl) (#765)

Expand All @@ -120,7 +123,6 @@ Bug Fixes::

* Fix registration of extension as instances (#754)


Documentation::

* Add extension migration guide
17 changes: 6 additions & 11 deletions asciidoctorj-core/src/main/java/org/asciidoctor/jruby/internal/JRubyAsciidoctor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,19 @@
import org.asciidoctor.converter.JavaConverterRegistry;
import org.asciidoctor.extension.ExtensionGroup;
import org.asciidoctor.extension.JavaExtensionRegistry;
import org.asciidoctor.extension.RubyExtensionRegistry;
import org.asciidoctor.jruby.AsciidoctorJRuby;
import org.asciidoctor.jruby.ast.impl.NodeConverter;
import org.asciidoctor.jruby.syntaxhighlighter.internal.SyntaxHighlighterRegistryExecutor;
import org.asciidoctor.log.LogHandler;
import org.asciidoctor.log.LogRecord;
import org.asciidoctor.jruby.DirectoryWalker;
import org.asciidoctor.extension.RubyExtensionRegistry;
import org.asciidoctor.jruby.ast.impl.DocumentHeaderImpl;
import org.asciidoctor.jruby.ast.impl.NodeConverter;
import org.asciidoctor.jruby.converter.internal.ConverterRegistryExecutor;
import org.asciidoctor.jruby.extension.internal.ExtensionRegistryExecutor;
import org.asciidoctor.jruby.log.internal.JULLogHandler;
import org.asciidoctor.jruby.log.internal.JavaLogger;
import org.asciidoctor.jruby.log.internal.LogHandlerRegistryExecutor;
import org.asciidoctor.jruby.syntaxhighlighter.internal.SyntaxHighlighterRegistryExecutor;
import org.asciidoctor.log.LogHandler;
import org.asciidoctor.log.LogRecord;
import org.asciidoctor.syntaxhighlighter.SyntaxHighlighterRegistry;
import org.jruby.*;
import org.jruby.exceptions.RaiseException;
Expand All @@ -29,7 +29,6 @@

import java.io.*;
import java.util.*;
import java.util.logging.Level;
import java.util.logging.Logger;

public class JRubyAsciidoctor implements AsciidoctorJRuby, LogHandler {
Expand Down Expand Up @@ -482,11 +481,7 @@ private RubyClass getExtensionGroupClass() {
@Override
public void log(LogRecord logRecord) {
for (LogHandler logHandler : logHandlers) {
try {
logHandler.log(logRecord);
} catch (Exception e) {
LOGGER.log(Level.WARNING, "Unexpected exception while logging Asciidoctor log entry", e);
}
logHandler.log(logRecord);
}
}
}
138 changes: 77 additions & 61 deletions asciidoctorj-core/src/test/java/org/asciidoctor/WhenAsciidoctorLogsToConsole.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,7 @@
import java.util.stream.Collectors;

import static org.asciidoctor.OptionsBuilder.options;
import static org.hamcrest.Matchers.both;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.*;
import static org.hamcrest.core.IsNull.nullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
Expand Down Expand Up @@ -67,20 +63,20 @@ public void shouldRedirectToJUL() throws Exception {

File inputFile = classpath.getResource("documentwithnotexistingfile.adoc");
String renderContent = asciidoctor.convertFile(inputFile,
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());

File expectedFile = new File(inputFile.getParent(), "documentwithnotexistingfile.html");
expectedFile.delete();

assertEquals(4, memoryLogHandler.getLogRecords().size());
assertThat(memoryLogHandler.getLogRecords().get(0).getMessage(),
both(containsString("include file not found"))
.and(containsString("documentwithnotexistingfile.adoc: line 3")));
both(containsString("include file not found"))
.and(containsString("documentwithnotexistingfile.adoc: line 3")));
}

private MemoryLogHandler registerMemoryLogHandler() {
Expand All @@ -99,12 +95,12 @@ public void shouldNotifyLogHandler() throws Exception {

File inputFile = classpath.getResource("documentwithnotexistingfile.adoc");
String renderContent = asciidoctor.convertFile(inputFile,
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());

File expectedFile = new File(inputFile.getParent(), "documentwithnotexistingfile.html");
expectedFile.delete();
Expand All @@ -116,7 +112,7 @@ public void shouldNotifyLogHandler() throws Exception {
assertThat(cursor.getFile(), is(inputFile.getName()));
assertThat(cursor.getLineNumber(), is(3));

for (LogRecord logRecord: logRecords) {
for (LogRecord logRecord : logRecords) {
assertThat(logRecord.getCursor(), not(nullValue()));
assertThat(logRecord.getCursor().getFile(), not(nullValue()));
assertThat(logRecord.getCursor().getDir(), not(nullValue()));
Expand All @@ -133,13 +129,13 @@ public void shouldLogInvalidRefs() throws Exception {

File inputFile = classpath.getResource("documentwithinvalidrefs.adoc");
String renderContent = asciidoctor.convertFile(inputFile,
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.toFile(false)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.toFile(false)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());

assertThat(logRecords, hasSize(1));
assertThat(logRecords.get(0).getMessage(), containsString("invalid reference: invalidref"));
Expand All @@ -159,12 +155,12 @@ public void shouldOnlyNotifyFromRegisteredAsciidoctor() throws Exception {
// Now render via second instance and check that there is no notification
File inputFile = classpath.getResource("documentwithnotexistingfile.adoc");
String renderContent1 = secondInstance.convertFile(inputFile,
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());

File expectedFile1 = new File(inputFile.getParent(), "documentwithnotexistingfile.html");
expectedFile1.delete();
Expand All @@ -173,12 +169,12 @@ public void shouldOnlyNotifyFromRegisteredAsciidoctor() throws Exception {

// Now render via first instance and check that notifications appeared.
String renderContent = asciidoctor.convertFile(inputFile,
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());

File expectedFile2 = new File(inputFile.getParent(), "documentwithnotexistingfile.html");
expectedFile2.delete();
Expand All @@ -201,12 +197,12 @@ public void shouldNoLongerNotifyAfterUnregisterOnlyNotifyFromRegisteredAsciidoct

File inputFile = classpath.getResource("documentwithnotexistingfile.adoc");
String renderContent = asciidoctor.convertFile(inputFile,
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());

File expectedFile = new File(inputFile.getParent(), "documentwithnotexistingfile.html");
expectedFile.delete();
Expand All @@ -217,12 +213,12 @@ public void shouldNoLongerNotifyAfterUnregisterOnlyNotifyFromRegisteredAsciidoct
asciidoctor.unregisterLogHandler(logHandler);

asciidoctor.convertFile(inputFile,
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());

File expectedFile2 = new File(inputFile.getParent(), "documentwithnotexistingfile.html");
expectedFile2.delete();
Expand All @@ -235,12 +231,12 @@ public void shouldNotifyLogHandlerService() throws Exception {

File inputFile = classpath.getResource("documentwithnotexistingfile.adoc");
String renderContent = asciidoctor.convertFile(inputFile,
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());
options()
.inPlace(true)
.safe(SafeMode.SERVER)
.attributes(
AttributesBuilder.attributes().allowUriRead(true))
.asMap());

File expectedFile = new File(inputFile.getParent(), "documentwithnotexistingfile.html");
expectedFile.delete();
Expand All @@ -256,7 +252,7 @@ public void shouldNotifyLogHandlerService() throws Exception {
assertThat(cursor.getFile(), is(inputFile.getName()));
assertThat(cursor.getLineNumber(), is(3));

for (LogRecord logRecord: logRecords) {
for (LogRecord logRecord : logRecords) {
assertThat(logRecord.getCursor(), not(Matchers.nullValue()));
assertThat(logRecord.getCursor().getFile(), not(Matchers.nullValue()));
assertThat(logRecord.getCursor().getDir(), not(Matchers.nullValue()));
Expand All @@ -270,8 +266,8 @@ public static class LoggingProcessor extends BlockProcessor {
public Object process(StructuralNode parent, Reader reader, Map<String, Object> attributes) {
log(new LogRecord(Severity.INFO, parent.getSourceLocation(), "Hello Log"));
final List<String> strings = reader.readLines().stream()
.map(String::toUpperCase)
.collect(Collectors.toList());
.map(String::toUpperCase)
.collect(Collectors.toList());

return createBlock(parent, "paragraph", strings);
}
Expand All @@ -286,7 +282,7 @@ public void a_extension_should_be_able_to_log() throws Exception {
asciidoctor.javaExtensionRegistry().block(LoggingProcessor.class);

String renderContent = asciidoctor.convert("= Test\n\n== Something different\n\n[big]\nHello World",
options().option("sourcemap", "true").asMap());
options().option("sourcemap", "true").asMap());

assertEquals(1, logRecords.size());
assertThat(logRecords.get(0).getMessage(), is("Hello Log"));
Expand All @@ -296,6 +292,26 @@ public void a_extension_should_be_able_to_log() throws Exception {
assertThat(renderContent, containsString("HELLO WORLD"));
}

@Test
public void should_fail_convert_when_logHandler_throws_an_exception() {
// given
final String errorMessage = "Conversion aborted by LogHandler";
asciidoctor.registerLogHandler(logRecord -> {
throw new RuntimeException(errorMessage);
});
// when
try {
asciidoctor.convert(
"= Test\n\n== Something different\n\n[big]\nHello World",
options().option("sourcemap", "true").asMap());
} catch (Throwable t) {
// then
assertThat(t.getMessage(), containsString("Failed to load AsciiDoc document"));
final Throwable cause = t.getCause();
assertThat(cause.getClass(), equalTo(RuntimeException.class));
assertThat(cause.getMessage(), is(errorMessage));
}
}

}