From cf343a43855ed0320109cef639ebc9e61c0ecd3e Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Sat, 7 Sep 2024 19:37:26 -0400 Subject: [PATCH 1/2] Test names with dots (plus minor fixes) --- bosk-core/src/main/java/works/bosk/Bosk.java | 3 ++- .../main/java/works/bosk/drivers/DriverConformanceTest.java | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/bosk-core/src/main/java/works/bosk/Bosk.java b/bosk-core/src/main/java/works/bosk/Bosk.java index 500cbe5d..d62eb58a 100644 --- a/bosk-core/src/main/java/works/bosk/Bosk.java +++ b/bosk-core/src/main/java/works/bosk/Bosk.java @@ -106,6 +106,7 @@ public class Bosk implements BoskInfo { * * @see DriverStack */ + @SuppressWarnings("this-escape") public Bosk(String name, Type rootType, DefaultRootFunction defaultRootFunction, DriverFactory driverFactory) { this.name = name; this.localDriver = new LocalDriver(defaultRootFunction); @@ -136,7 +137,7 @@ public Bosk(String name, Type rootType, DefaultRootFunction defaultRootFuncti rawClass(rootType).cast(this.currentRoot); // Ok, we're done initializing - boskInfo.boskRef().set(this); + boskInfo.boskRef().set(this); // @SuppressWarnings("this-escape") } public interface DefaultRootFunction { diff --git a/bosk-testing/src/main/java/works/bosk/drivers/DriverConformanceTest.java b/bosk-testing/src/main/java/works/bosk/drivers/DriverConformanceTest.java index e88270c1..3e7a40f8 100644 --- a/bosk-testing/src/main/java/works/bosk/drivers/DriverConformanceTest.java +++ b/bosk-testing/src/main/java/works/bosk/drivers/DriverConformanceTest.java @@ -389,7 +389,7 @@ void mapValue_works() throws InvalidTypeException { MapValue originalMapValue = MapValue.fromFunction(asList("key.with.dots.1", "key.with.dots.2"), k -> k + "_originalValue"); driver.submitReplacement(mapRef, originalMapValue); assertCorrectBoskContents(); - MapValue newMapValue = originalMapValue.with("key.with.dots.1", "newValue"); + MapValue newMapValue = originalMapValue.with("key.with.dots.1", "_newValue"); driver.submitReplacement(mapRef, newMapValue); assertCorrectBoskContents(); @@ -527,6 +527,7 @@ static Stream childID() { "id.with.dots", "id/with/slashes", "$id$with$dollars$", + "id:with:colons:", AWKWARD_ID, "idWithEmojis\uD83C\uDF33\uD83E\uDDCA" ).map(Identifier::from); @@ -535,7 +536,7 @@ static Stream childID() { /** * Contains all kinds of special characters */ - public static final String AWKWARD_ID = "$id.with%everything/ +\uD83D\uDE09"; + public static final String AWKWARD_ID = "$id.with%everything:/ +\uD83D\uDE09"; @SuppressWarnings("unused") static Stream testEntityField() { From 55263107205ab71ab3a011ab1808531aab1875c5 Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Sun, 8 Sep 2024 06:23:02 -0400 Subject: [PATCH 2/2] Add Jackson linked-map representation --- .../works/bosk/jackson/JacksonPlugin.java | 153 +++++++++++++++--- .../jackson/JacksonPluginConfiguration.java | 62 +++++++ .../works/bosk/jackson/JacksonPluginTest.java | 10 -- .../JacksonRoundTripConformanceTest.java | 15 +- .../works/bosk/AbstractRoundTripTest.java | 18 ++- 5 files changed, 218 insertions(+), 40 deletions(-) create mode 100644 bosk-jackson/src/main/java/works/bosk/jackson/JacksonPluginConfiguration.java diff --git a/bosk-jackson/src/main/java/works/bosk/jackson/JacksonPlugin.java b/bosk-jackson/src/main/java/works/bosk/jackson/JacksonPlugin.java index d9846da9..30a36aee 100644 --- a/bosk-jackson/src/main/java/works/bosk/jackson/JacksonPlugin.java +++ b/bosk-jackson/src/main/java/works/bosk/jackson/JacksonPlugin.java @@ -1,6 +1,5 @@ package works.bosk.jackson; -import com.fasterxml.jackson.core.JacksonException; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.JsonParser; @@ -67,6 +66,7 @@ import static works.bosk.ListingEntry.LISTING_ENTRY; import static works.bosk.ReferenceUtils.rawClass; import static works.bosk.ReferenceUtils.theOnlyConstructorFor; +import static works.bosk.jackson.JacksonPluginConfiguration.defaultConfiguration; /** * Provides JSON serialization/deserialization using Jackson. @@ -74,6 +74,15 @@ */ public final class JacksonPlugin extends SerializationPlugin { private final JacksonCompiler compiler = new JacksonCompiler(this); + private final JacksonPluginConfiguration config; + + public JacksonPlugin() { + this(defaultConfiguration()); + } + + public JacksonPlugin(JacksonPluginConfiguration config) { + this.config = config; + } public BoskJacksonModule moduleFor(BoskInfo boskInfo) { return new BoskJacksonModule() { @@ -334,8 +343,7 @@ private JsonDeserializer> catalogDeserializer(JavaType type, Des @Override @SuppressWarnings({"rawtypes", "unchecked"}) public Catalog deserialize(JsonParser p, DeserializationContext ctxt) throws IOException { - JsonDeserializer valueDeserializer = ctxt.findContextualValueDeserializer(entryType, null); - LinkedHashMap entries = readMapEntries(p, valueDeserializer, ctxt); + LinkedHashMap entries = readMapEntries(p, entryType, ctxt); return Catalog.of(entries.values()); } }; @@ -429,15 +437,13 @@ public SideTable deserialize(JsonParser p, DeserializationContex Reference> domain = null; LinkedHashMap valuesById = null; - JsonDeserializer valueDeserializer = ctxt.findContextualValueDeserializer(valueType, null); - expect(START_OBJECT, p); while (p.nextToken() != END_OBJECT) { p.nextValue(); switch (p.currentName()) { case "valuesById": if (valuesById == null) { - valuesById = readMapEntries(p, valueDeserializer, ctxt); + valuesById = readMapEntries(p, valueType, ctxt); } else { throw new JsonParseException(p, "'valuesById' field appears twice"); } @@ -568,6 +574,13 @@ private abstract static class BoskDeserializer extends JsonDeserializer { } private void writeMapEntries(JsonGenerator gen, Set> entries, SerializerProvider serializers) throws IOException { + switch (config.mapShape()) { + case ARRAY -> writeEntriesAsArray(gen, entries, serializers); + case LINKED_MAP -> writeEntriesAsLinkedMap(gen, entries, serializers); + } + } + + private static void writeEntriesAsArray(JsonGenerator gen, Set> entries, SerializerProvider serializers) throws IOException { gen.writeStartArray(); for (Entry entry: entries) { gen.writeStartObject(); @@ -579,32 +592,128 @@ private void writeMapEntries(JsonGenerator gen, Set> ent gen.writeEndArray(); } + private static void writeEntriesAsLinkedMap(JsonGenerator gen, Set> entries, SerializerProvider serializers) throws IOException { + gen.writeStartObject(); + if (!entries.isEmpty()) { + if (entries.size() == 1) { + var entry = entries.iterator().next(); + gen.writeStringField(FIRST, entry.getKey().toString()); + gen.writeStringField(LAST, entry.getKey().toString()); + writeEntryAsField(gen, Optional.empty(), entry, Optional.empty(), serializers); + } else { + // This will be so much easier with a list + List> list = List.copyOf(entries); + gen.writeStringField(FIRST, list.getFirst().getKey().toString()); + gen.writeStringField(LAST, list.getLast().getKey().toString()); + writeEntryAsField(gen, + Optional.empty(), + list.getFirst(), + Optional.of(list.get(1).getKey()), + serializers); + for (int i = 1; i < list.size()-1; i++) { + writeEntryAsField(gen, + Optional.of(list.get(i-1).getKey()), + list.get(i), + Optional.of(list.get(i+1).getKey()), + serializers); + } + writeEntryAsField(gen, + Optional.of(list.get(list.size()-2).getKey()), + list.getLast(), + Optional.empty(), + serializers); + } + } + gen.writeEndObject(); + } + + private static void writeEntryAsField(JsonGenerator gen, Optional prev, Entry entry, Optional next, SerializerProvider serializers) throws IOException { + gen.writeFieldName(entry.getKey().toString()); + JsonSerializer entryDeserializer = serializers.findContentValueSerializer( + TypeFactory.defaultInstance().constructParametricType(LinkedMapEntry.class, entry.getValue().getClass()), + null); + entryDeserializer.serialize(new LinkedMapEntry<>(prev.map(Object::toString), next.map(Object::toString), entry.getValue()), gen, serializers); + } + /** * Leaves the parser sitting on the END_ARRAY token. You could call nextToken() to continue with parsing. */ - private LinkedHashMap readMapEntries(JsonParser p, JsonDeserializer valueDeserializer, DeserializationContext ctxt) throws IOException { + private LinkedHashMap readMapEntries(JsonParser p, JavaType valueType, DeserializationContext ctxt) throws IOException { + JsonDeserializer valueDeserializer = (JsonDeserializer) ctxt.findContextualValueDeserializer(valueType, null); LinkedHashMap result = new LinkedHashMap<>(); - expect(START_ARRAY, p); - while (p.nextToken() != END_ARRAY) { - expect(START_OBJECT, p); - p.nextValue(); - String fieldName = p.currentName(); - Identifier entryID = Identifier.from(fieldName); - V value; - try (@SuppressWarnings("unused") DeserializationScope scope = entryDeserializationScope(entryID)) { - value = valueDeserializer.deserialize(p, ctxt); + if (p.currentToken() == START_OBJECT) { + JsonDeserializer entryDeserializer = ctxt.findContextualValueDeserializer( + TypeFactory.defaultInstance().constructParametricType(LinkedMapEntry.class, valueType), + null); + HashMap> entries = new HashMap<>(); + String first = null; + String last = null; + while (p.nextToken() != END_OBJECT) { + p.nextValue(); + String fieldName = p.currentName(); + switch (fieldName) { + case FIRST -> first = p.getText(); + case LAST -> last = p.getText(); + default -> { + Identifier entryID = Identifier.from(fieldName); + try (@SuppressWarnings("unused") DeserializationScope scope = entryDeserializationScope(entryID)) { + @SuppressWarnings("unchecked") + LinkedMapEntry entry = (LinkedMapEntry) entryDeserializer.deserialize(p, ctxt); + entries.put(fieldName, entry); +// p.nextToken(); + } + } + } + } + String cur = first; + while (cur != null) { + LinkedMapEntry entry = entries.get(cur); + if (entry == null) { + throw new JsonParseException(p, "No such entry: \"" + cur + "\""); + } + result.put(Identifier.from(cur), entry.value()); + String next = entry.next().orElse(null); + if (next == null && !cur.equals(last)) { + throw new JsonParseException(p, "Entry \" + cur + \" has no next pointer but does not match last = \" + last + \""); + } + // TODO: Verify "prev" pointers + cur = next; } - p.nextToken(); - expect(END_OBJECT, p); + } else { + expect(START_ARRAY, p); + while (p.nextToken() != END_ARRAY) { + expect(START_OBJECT, p); + p.nextValue(); + String fieldName = p.currentName(); + Identifier entryID = Identifier.from(fieldName); + V value; + try (@SuppressWarnings("unused") DeserializationScope scope = entryDeserializationScope(entryID)) { + value = valueDeserializer.deserialize(p, ctxt); + } + p.nextToken(); + expect(END_OBJECT, p); - V oldValue = result.put(entryID, value); - if (oldValue != null) { - throw new JsonParseException(p, "Duplicate sideTable entry '" + fieldName + "'"); + V oldValue = result.put(entryID, value); + if (oldValue != null) { + throw new JsonParseException(p, "Duplicate sideTable entry '" + fieldName + "'"); + } } } return result; } + /** + * Structure of the field values used by the {@link JacksonPluginConfiguration.MapShape#LINKED_MAP LINKED_MAP} format. + * @param prev the key corresponding to the previous map entry, or {@link Optional#empty() empty} if none. + * @param next the key corresponding to the next map entry, or {@link Optional#empty() empty} if none. + * @param value the actual map entry's value + */ + public record LinkedMapEntry( + Optional prev, + Optional next, + V value + ) implements StateTreeNode {} + private static final JavaType ID_LIST_TYPE = TypeFactory.defaultInstance().constructType(new TypeReference< List>() {}); @@ -847,4 +956,6 @@ public static void expect(JsonToken expected, JsonParser p) throws IOException { } } + private static final String FIRST = "-first"; + private static final String LAST = "-last"; } diff --git a/bosk-jackson/src/main/java/works/bosk/jackson/JacksonPluginConfiguration.java b/bosk-jackson/src/main/java/works/bosk/jackson/JacksonPluginConfiguration.java new file mode 100644 index 00000000..9810482c --- /dev/null +++ b/bosk-jackson/src/main/java/works/bosk/jackson/JacksonPluginConfiguration.java @@ -0,0 +1,62 @@ +package works.bosk.jackson; + +public record JacksonPluginConfiguration( + MapShape mapShape +) { + public static JacksonPluginConfiguration defaultConfiguration() { + return new JacksonPluginConfiguration(MapShape.ARRAY); + } + + /** + * How bosk's ordered maps should translate to JSON, where the order of object + * fields is generally not preserved. + */ + public enum MapShape { + /** + * A shape intended for brevity, human readability, and efficient serialization/deserialization. + *

+ * An array of single-field objects, where the field name is the map entry's key + * and the field value is the map entry's value. + */ + ARRAY, + + /** + * A shape intended for efficient incremental modification, + * especially for values stored in a database that supports JSON + * but does not preserve object field order (like Postgresql). + * Inspired by {@link java.util.LinkedHashMap LinkedHashMap}. + *

+ * An object containing the natural keys and values of the map being stored, + * with a few changes: + * + *

    + *
  • + * The object also has fields "{@code -first}" and "{@code -last}" pointing + * at the first and last map entries, so they can be found efficiently. + * (These names have leading dashes to distinguish them from valid + * {@link works.bosk.Identifier Identifier} values.) + *
    • + *
  • + * The object's field values are themselves objects defined by + * {@link works.bosk.jackson.JacksonPlugin.LinkedMapEntry LinkedMapEntry}. + *
    • + *
+ * + * The resulting structure supports the following operations in O(1) time: + *
    + *
  • + * Lookup an entry given its ID + *
    • + *
  • + * Add a new entry at the end. + *
    • + *
  • + * Delete an entry. + *
    • + *
+ * + * It also supports linear time walking of the entries in both forward and reverse order. + */ + LINKED_MAP, + } +} diff --git a/bosk-jackson/src/test/java/works/bosk/jackson/JacksonPluginTest.java b/bosk-jackson/src/test/java/works/bosk/jackson/JacksonPluginTest.java index 38b95fe8..bbb8543a 100644 --- a/bosk-jackson/src/test/java/works/bosk/jackson/JacksonPluginTest.java +++ b/bosk-jackson/src/test/java/works/bosk/jackson/JacksonPluginTest.java @@ -626,11 +626,6 @@ void nonexistentPath_throws() { .readValue("\"/some/nonexistent/path\"")); } - @Test - void catalogFromEmptyMap_throws() { - assertJsonException("{}", Catalog.class, TestEntity.class); - } - @Test void catalogWithContentsArray_throws() { assertJsonException("{ \"contents\": [] }", Catalog.class, TestEntity.class); @@ -681,11 +676,6 @@ void sideTableWithTwoDomains_throws() { assertJsonException("{ \"domain\": \"/entities\", \"domain\": \"/entities\", \"valuesById\": [] }", SideTable.class, TestEntity.class, String.class); } - @Test - void sideTableWithValuesMap_throws() { - assertJsonException("{ \"domain\": \"/entities\", \"valuesById\": {} }", SideTable.class, TestEntity.class, String.class); - } - @Test void sideTableWithTwoValuesFields_throws() { assertJsonException("{ \"domain\": \"/entities\", \"valuesById\": [], \"valuesById\": [] }", SideTable.class, TestEntity.class, String.class); diff --git a/bosk-jackson/src/test/java/works/bosk/jackson/JacksonRoundTripConformanceTest.java b/bosk-jackson/src/test/java/works/bosk/jackson/JacksonRoundTripConformanceTest.java index 283ebb1f..14e16916 100644 --- a/bosk-jackson/src/test/java/works/bosk/jackson/JacksonRoundTripConformanceTest.java +++ b/bosk-jackson/src/test/java/works/bosk/jackson/JacksonRoundTripConformanceTest.java @@ -1,13 +1,20 @@ package works.bosk.jackson; -import org.junit.jupiter.api.BeforeEach; +import java.util.stream.Stream; import works.bosk.drivers.DriverConformanceTest; +import works.bosk.junit.ParametersByName; import static works.bosk.AbstractRoundTripTest.jacksonRoundTripFactory; +import static works.bosk.jackson.JacksonPluginConfiguration.MapShape.LINKED_MAP; public class JacksonRoundTripConformanceTest extends DriverConformanceTest { - @BeforeEach - void setupDriverFactory() { - driverFactory = jacksonRoundTripFactory(); + @ParametersByName + JacksonRoundTripConformanceTest(JacksonPluginConfiguration config) { + driverFactory = jacksonRoundTripFactory(config); + } + + static Stream config() { + return Stream.of(JacksonPluginConfiguration.MapShape.values()) + .map(shape -> new JacksonPluginConfiguration(shape)); } } diff --git a/lib-testing/src/main/java/works/bosk/AbstractRoundTripTest.java b/lib-testing/src/main/java/works/bosk/AbstractRoundTripTest.java index 59b12ea6..07b9b25d 100644 --- a/lib-testing/src/main/java/works/bosk/AbstractRoundTripTest.java +++ b/lib-testing/src/main/java/works/bosk/AbstractRoundTripTest.java @@ -31,10 +31,12 @@ import works.bosk.drivers.mongo.BsonPlugin; import works.bosk.exceptions.InvalidTypeException; import works.bosk.jackson.JacksonPlugin; +import works.bosk.jackson.JacksonPluginConfiguration; import static com.fasterxml.jackson.databind.SerializationFeature.INDENT_OUTPUT; import static java.lang.System.identityHashCode; import static java.util.Collections.newSetFromMap; +import static works.bosk.jackson.JacksonPluginConfiguration.MapShape.LINKED_MAP; public abstract class AbstractRoundTripTest extends AbstractBoskTest { @@ -43,7 +45,8 @@ static Stream> driverFactories() { directFactory(), factoryThatMakesAReference(), - jacksonRoundTripFactory(), + jacksonRoundTripFactory(JacksonPluginConfiguration.defaultConfiguration()), + jacksonRoundTripFactory(new JacksonPluginConfiguration(LINKED_MAP)), bsonRoundTripFactory() ); @@ -60,13 +63,18 @@ public static DriverFactory factoryThatMakesAReference() { }; } - public static DriverFactory jacksonRoundTripFactory() { - return new JacksonRoundTripDriverFactory<>(); + public static DriverFactory jacksonRoundTripFactory(JacksonPluginConfiguration config) { + return new JacksonRoundTripDriverFactory<>(config); } - @RequiredArgsConstructor private static class JacksonRoundTripDriverFactory implements DriverFactory { - private final JacksonPlugin jp = new JacksonPlugin(); + private final JacksonPluginConfiguration config; + private final JacksonPlugin jp; + + private JacksonRoundTripDriverFactory(JacksonPluginConfiguration config) { + this.config = config; + this.jp = new JacksonPlugin(config); + } @Override public BoskDriver build(BoskInfo boskInfo, BoskDriver driver) {