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.

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 reading of macro table appends. #979

Merged
merged 1 commit into from
Oct 21, 2024
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
1 change: 1 addition & 0 deletions src/main/java/com/amazon/ion/impl/IonCursorBinary.java
Original file line number Diff line number Diff line change
Expand Up @@ -2417,6 +2417,7 @@ private boolean slowReadHeader(final int typeIdByte, final boolean isAnnotated,
}
if (minorVersion == 1) {
if (valueTid.isMacroInvocation) {
setCheckpointAfterValueHeader();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this, the reader will in certain cases attempt to read the same byte repetitively, not making any forward progress.

return true;
}
if (valueTid.isNull && valueTid.length > 0) {
Expand Down
9 changes: 6 additions & 3 deletions src/main/java/com/amazon/ion/impl/macro/EncodingContext.kt
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package com.amazon.ion.impl.macro

/**
* When we implement modules, this will likely need to be replaced.
* For now, it is a placeholder for what is to come and a container for the macro table.
*/
class EncodingContext(
val macroTable: Map<MacroRef, Macro>
) {
class EncodingContext(macroTable: Map<MacroRef, Macro>) {

val macroTable = macroTable.toMutableMap()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This performs a copy, allowing the caller to mutate the map passed in as an argument without modifying the underlying encoding context. This map is mutable because users may add new macros to the table.


companion object {
// TODO: Replace this with a DEFAULT encoding context that includes system macros.
@JvmStatic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1321,6 +1321,67 @@ public void multipleListsWithinSymbolTableDeclaration(InputType inputType, Strea
}
}

@ParameterizedTest(name = "{0},{1}")
@MethodSource("allCombinations")
public void emptyMacroAppendToEmptyTable(InputType inputType, StreamType streamType) throws Exception {
ByteArrayOutputStream out = new ByteArrayOutputStream();
IonRawWriter_1_1 writer = streamType.newWriter(out);
writer.writeIVM();

startEncodingDirective(writer);
startMacroTable(writer);
writer.writeSymbol(SystemSymbols_1_1.ION_ENCODING);
endMacroTable(writer);
endEncodingDirective(writer);

byte[] data = getBytes(writer, out);
try (IonReader reader = inputType.newReader(data)) {
assertNull(reader.next());
}
}

@ParameterizedTest(name = "{0},{1}")
@MethodSource("allCombinations")
public void emptyMacroAppendToNonEmptyTable(InputType inputType, StreamType streamType) throws Exception {
ByteArrayOutputStream out = new ByteArrayOutputStream();
IonRawWriter_1_1 writer = streamType.newWriter(out);
writer.writeIVM();

SortedMap<String, Macro> macroTable = new TreeMap<>();
macroTable.put("foo", new TemplateMacro(
Collections.singletonList(new Macro.Parameter("foo", Macro.ParameterEncoding.Tagged, Macro.ParameterCardinality.ExactlyOne)),
Collections.singletonList(new Expression.VariableRef(0))
));
Map<String, Integer> symbols = Collections.emptyMap();

startEncodingDirective(writer); {
startMacroTable(writer); {
startMacro(writer, symbols, "foo"); {
writeMacroSignature(writer, symbols, "x");
writeVariableExpansion(writer, symbols, "x");
} endMacro(writer);
} endMacroTable(writer);
} endEncodingDirective(writer);


startEncodingDirective(writer); {
startMacroTable(writer); {
writer.writeSymbol(SystemSymbols_1_1.ION_ENCODING);
} endMacroTable(writer);
writeEncodingDirectiveSymbolTable(writer, true, "bar");
} endEncodingDirective(writer);

writer.stepInEExp(0, true, macroTable.get("foo")); {
writer.writeSymbol(1);
} writer.stepOut();

byte[] data = getBytes(writer, out);
try (IonReader reader = inputType.newReader(data)) {
assertEquals(IonType.SYMBOL, reader.next());
assertEquals("bar", reader.stringValue());
}
}

// TODO cover every Ion type
// TODO annotations in macro definition (using 'annotate' system macro)
// TODO test error conditions
Expand Down
Loading