Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
public final class ImageWriterOptions {
public static class Builder {
private MetadataVersion metadataVersion;
private MetadataVersion requestedMetadataVersion;
private Consumer<UnwritableMetadataException> lossHandler = e -> {
throw e;
};
Expand All @@ -42,6 +43,7 @@ public Builder(MetadataImage image) {
}

public Builder setMetadataVersion(MetadataVersion metadataVersion) {
setRequestedMetadataVersion(metadataVersion);
if (metadataVersion.isLessThan(MetadataVersion.MINIMUM_BOOTSTRAP_VERSION)) {
// When writing an image, all versions less than 3.3-IV0 are treated as 3.0-IV1.
// This is because those versions don't support FeatureLevelRecord.
Expand All @@ -58,37 +60,48 @@ public Builder setRawMetadataVersion(MetadataVersion metadataVersion) {
return this;
}

public void setRequestedMetadataVersion(MetadataVersion orgMetadataVersion) {
this.requestedMetadataVersion = orgMetadataVersion;
}

public MetadataVersion metadataVersion() {
return metadataVersion;
}

public MetadataVersion requestedMetadataVersion() {
return requestedMetadataVersion;
}

public Builder setLossHandler(Consumer<UnwritableMetadataException> lossHandler) {
this.lossHandler = lossHandler;
return this;
}

public ImageWriterOptions build() {
return new ImageWriterOptions(metadataVersion, lossHandler);
return new ImageWriterOptions(metadataVersion, lossHandler, requestedMetadataVersion);
}
}

private final MetadataVersion metadataVersion;
private final MetadataVersion requestedMetadataVersion;
private final Consumer<UnwritableMetadataException> lossHandler;

private ImageWriterOptions(
MetadataVersion metadataVersion,
Consumer<UnwritableMetadataException> lossHandler
Consumer<UnwritableMetadataException> lossHandler,
MetadataVersion orgMetadataVersion
) {
this.metadataVersion = metadataVersion;
this.lossHandler = lossHandler;
this.requestedMetadataVersion = orgMetadataVersion;
}

public MetadataVersion metadataVersion() {
return metadataVersion;
}

public void handleLoss(String loss) {
lossHandler.accept(new UnwritableMetadataException(metadataVersion, loss));
lossHandler.accept(new UnwritableMetadataException(requestedMetadataVersion, loss));
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ private static void writeWithExpectedLosses(
MetadataImage image = delta.apply(MetadataProvenance.EMPTY);
RecordListWriter writer = new RecordListWriter();
image.write(writer, new ImageWriterOptions.Builder().
setRawMetadataVersion(metadataVersion).
setMetadataVersion(metadataVersion).
setLossHandler(lossConsumer).
build());
assertEquals(expectedLosses, lossConsumer.losses, "Failed to get expected metadata losses.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;

import java.io.ByteArrayOutputStream;
import java.io.PrintStream;
import java.util.function.Consumer;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

Expand All @@ -44,9 +48,33 @@ public void testSetMetadataVersion() {
setMetadataVersion(version);
if (i < MetadataVersion.MINIMUM_BOOTSTRAP_VERSION.ordinal()) {
assertEquals(MetadataVersion.MINIMUM_KRAFT_VERSION, options.metadataVersion());
assertEquals(version, options.requestedMetadataVersion());
} else {
assertEquals(version, options.metadataVersion());
}
}
}

@Test
public void testHandleLoss() {
PrintStream originalOut = System.out;
String expectedMessage = "stuff";
Consumer<UnwritableMetadataException> customLossHandler = e -> System.out.println(e.getMessage());

for (int i = MetadataVersion.MINIMUM_KRAFT_VERSION.ordinal();
i < MetadataVersion.VERSIONS.length;
i++) {
ByteArrayOutputStream outContent = new ByteArrayOutputStream();
MetadataVersion version = MetadataVersion.VERSIONS[i];
ImageWriterOptions options = new ImageWriterOptions.Builder()
.setMetadataVersion(version)
.setLossHandler(customLossHandler)
.build();
System.setOut(new PrintStream(outContent));
options.handleLoss(expectedMessage);
System.setOut(originalOut);
String formattedMessage = String.format("Metadata has been lost because the following could not be represented in metadata version %s: %s", version, expectedMessage);
assertEquals(formattedMessage, outContent.toString().trim());
}
Comment on lines +64 to +71

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can simplify this test as this without relying System.out:

for (int i = MetadataVersion.MINIMUM_KRAFT_VERSION.ordinal();
             i < MetadataVersion.VERSIONS.length;
             i++) {
            
            MetadataVersion version = MetadataVersion.VERSIONS[i];
            String formattedMessage = String.format("Metadata has been lost because the following could not be represented in metadata version %s: %s", version, expectedMessage);
            Consumer<UnwritableMetadataException> customLossHandler = e -> {
                assertEquals(formattedMessage, e.getMessage());
            };
            ImageWriterOptions options = new ImageWriterOptions.Builder()
                    .setMetadataVersion(version)
                    .setLossHandler(customLossHandler)
                    .build();
            options.handleLoss(expectedMessage);
}

WDYT?

@Owen-CH-Leung Owen-CH-Leung Jul 17, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@showuon Thanks! Yes it's a lot cleaner. I've adopted your suggestion.

}
}