Skip to content

Fix to JsonWriter to not flush underlying outputStream when error during serialisation #40

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.

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

Merged
merged 1 commit into from
Dec 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions blackbox-test/src/main/java/org/example/customer/IErrOnRead.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.example.customer;

import io.avaje.jsonb.Json;

import java.util.UUID;

@Json
public record IErrOnRead(UUID id, String firstName, String lastName) {

public String lastName() {
throw new IllegalArgumentException("error reading lastName");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package org.example.customer;

import io.avaje.jsonb.JsonException;
import io.avaje.jsonb.JsonType;
import io.avaje.jsonb.Jsonb;
import org.junit.jupiter.api.Test;

import java.io.ByteArrayOutputStream;
import java.util.UUID;

import static org.assertj.core.api.Assertions.assertThat;

class IErrOnReadTest {

@Test
void toJson_withError() {
IErrOnRead bean = new IErrOnRead(UUID.randomUUID(), "first", "last");
Jsonb jsonb = Jsonb.builder().build();
JsonType<IErrOnRead> type = jsonb.type(IErrOnRead.class);

ByteArrayOutputStream baos = new ByteArrayOutputStream();
try {
type.toJson(bean, baos);
} catch (JsonException expectedForTest) {
assertThat(expectedForTest.getCause()).hasMessage("error reading lastName");
}
assertThat(baos.toByteArray()).describedAs("no flush to outputStream expected").hasSize(0);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ final class JacksonWriter implements JsonWriter {
this.serializeEmpty = serializeEmpty;
}

@Override
public void markIncomplete() {
// on close don't flush or close the underlying OutputStream
generator.disable(JsonGenerator.Feature.AUTO_CLOSE_TARGET);
generator.disable(JsonGenerator.Feature.FLUSH_PASSED_TO_STREAM);
generator.disable(JsonGenerator.Feature.AUTO_CLOSE_JSON_CONTENT);
}

@Override
public void close() {
try {
Expand Down
7 changes: 7 additions & 0 deletions jsonb/src/main/java/io/avaje/jsonb/JsonWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -228,4 +228,11 @@ public interface JsonWriter extends Closeable, Flushable {
*/
void close();

/**
* Mark the generated json as not completed due to an error.
* <p>
* This typically means not to flush or close an underlying OutputStream which
* allows it to be reset to then write some error response content instead.
*/
void markIncomplete();
}
11 changes: 8 additions & 3 deletions jsonb/src/main/java/io/avaje/jsonb/core/DJsonType.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,20 +75,25 @@ public final byte[] toJsonBytes(T value) {

@Override
public final void toJson(T value, JsonWriter writer) {
adapter.toJson(writer, value);
try {
adapter.toJson(writer, value);
} catch (RuntimeException e) {
writer.markIncomplete();
throw new JsonException(e);
}
}

@Override
public final void toJson(T value, Writer writer) {
try (JsonWriter jsonWriter = jsonb.writer(writer)) {
adapter.toJson(jsonWriter, value);
toJson(value, jsonWriter);
}
}

@Override
public final void toJson(T value, OutputStream outputStream) {
try (JsonWriter writer = jsonb.writer(outputStream)) {
adapter.toJson(writer, value);
toJson(value, writer);
}
close(outputStream);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,4 +186,8 @@ public void close() {
delegate.close();
}

@Override
public void markIncomplete() {
delegate.markIncomplete();
}
}
8 changes: 8 additions & 0 deletions jsonb/src/main/java/io/avaje/jsonb/stream/JGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ final class JGenerator implements JsonGenerator {
private final ArrayStack<JsonNames> nameStack = new ArrayStack<>();
private JsonNames currentNames;
private boolean allNames;
private boolean incomplete;

JGenerator() {
this(512);
Expand All @@ -83,6 +84,7 @@ JsonGenerator prepare(OutputStream targetStream) {
pretty = false;
nameStack.clear();
allNames = false;
incomplete = false;
return this;
}

Expand Down Expand Up @@ -381,6 +383,11 @@ public byte[] toByteArray() {
return Arrays.copyOf(buffer, position);
}

@Override
public void markIncomplete() {
incomplete = true;
}

@Override
public void flush() {
if (target != null && position != 0) {
Expand All @@ -395,6 +402,7 @@ public void flush() {

@Override
public void close() {
if (incomplete) return;
flush();
if (target != null) {
try {
Expand Down
4 changes: 4 additions & 0 deletions jsonb/src/main/java/io/avaje/jsonb/stream/JsonGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,8 @@ interface JsonGenerator extends Closeable, Flushable {
*/
byte[] toByteArray();

/**
* Mark that json generation was not completed due to an error.
*/
void markIncomplete();
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ public <T> T unwrap(Class<T> underlying) {
return (T)generator;
}

@Override
public void markIncomplete() {
generator.markIncomplete();
}

@Override
public void close() {
generator.close();
Expand Down
21 changes: 21 additions & 0 deletions jsonb/src/test/java/io/avaje/jsonb/stream/DieselAdapterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,27 @@ void write_to_OutputStream() {
assertThat(os1.toString()).isEqualTo("{\"one\":\"hi\"}");
}

@Test
void write_markIncomplete_withError() {
ByteArrayOutputStream os = new ByteArrayOutputStream();
try {
try (JsonWriter jw = adapter.writer(os)) {
jw.beginObject();
jw.name("one");
jw.value("I will be incomplete");
jw.markIncomplete();
throwAnError();
}
} catch (Exception e) {
// ignore
}
assertThat(os.toString()).isEqualTo("");
}

private void throwAnError() {
throw new IllegalStateException("foo");
}

private void writeHello(JsonWriter jw, String message) {
jw.beginObject();
jw.name("one");
Expand Down