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

3.0.0, including structure interpreter API #1

Open
wants to merge 74 commits into
base: main
Choose a base branch
from

Conversation

lukebemish
Copy link
Member

@lukebemish lukebemish commented Jul 5, 2024

A new system for expressing data structures in a way that can be "interpreted" to various formats -- that is, Codecs, StreamCodecs, JSON schema, (eventually) config screens, etc.

Made of the following components:

  • Structure<A>, represents a structure for representing an A
  • Interpreter<T>, maps Structure<A> -> T<A> (representing the type function through K1/App)

Structures can represent:

  • Lists, through Structure#listOf
  • Records, through Structure#record and RecordStructure.Builder. Record builders can be combined, so long as keys are unique, similar to MapCodecs, and follow a similar design as KeyedRecordCodecBuilder.
  • flatXmap/xmaped structures, through the appropriate methods on Structure
  • keyed structures, which represent an Interpreter-specific implementation, linked to a given instance-comparable Key -- think primitives
  • annotated structures, which provide data an Interpreter may use to modify the complete structure -- think comments.

Current interpreters include:

  • Codecs through CodecInterpreter
  • MapCodecs through MapCodecInterpreter, which delegates to a CodecInterpreter for all nested non-map stuff
  • StreamCodecs through StreamCodecInterpreter
  • JSON schemas, through JsonSchemaInterpreter
  • Config screens, through ConfigScreenInterpreter
  • Default values, through IdentityInterpreter

TODO items:

  • Implement structures for remaining common types:
    • ItemStack
    • key-dispatched maps
    • Holders
  • Implement an equivalent of keyed structures for things that take in some parameters and produce an Interpreter-specific object -- see, resource keys or the like.
  • Investigate a config screen interpreter
  • Allow attaching keys to either structures or interpreters: so, interpreters get a key on their Mu, and a keyed structure can include stuff keyed on that to fall back to, if interpreters don't have something.
  • Figure out dispatch in JSON schemas
  • regex matching requirements in JSON schemas -- resource locations, etc
  • Either codecs or similar forms

For an example of usage, see https://github.com/lukebemishprojects/codecextras/blob/feature/structures/src/test/java/dev/lukebemish/codecextras/test/structured/TestStructured.java

@lukebemish lukebemish changed the title Structure interpreter API 3.0.0, including structure interpreter API Sep 10, 2024
@Gaming32
Copy link

I discovered an issue. Annotations on simple things like strings will "leak" to same-typed fields later on in the structure. I've tracked this down to the schema not being copied after calling schemaValue in annotate.

@lukebemish
Copy link
Member Author

Could have sworn I'd fixed all those cases when I change up how JsonSchemaInterpreter worked the last time to handle reusable stuff; must have missed that usage. Thanks!

@Gaming32
Copy link

Gaming32 commented Sep 23, 2024

Another issue I've found is that JsonSchemaInterpreter doesn't support any of the _IN_RANGE parametric keys.

The converter I created for INT_IN_RANGE is this:

new ParametricKeyedValue<>() {
    @Override
    public <T> @NotNull App<JsonSchemaInterpreter.Holder.Mu, App<Const.Mu<Integer>, T>> convert(
        @NotNull App<Const.Mu<Range<@NotNull Integer>>, T> parameter
    ) {
        final var range = Const.unbox(parameter);
        final var result = JsonSchemaInterpreter.INTEGER.get();
        result.addProperty("minimum", range.min());
        result.addProperty("maximum", range.max());
        return new JsonSchemaInterpreter.Holder<>(result);
    }
}

@Gaming32
Copy link

One thing I noticed is that the places that deal with definitions use HashMap instead of LinkedHashMap. Using LinkedHashMap would guarantee that the output is deterministic.

@Gaming32
Copy link

Gaming32 commented Oct 4, 2024

I have found that StructuredMapCodec doesn't support partial results. If a specific field codec returns a partial result, the StructuredMapCodec should use that partial result, but instead it just errors out entirely.

@lukebemish
Copy link
Member Author

lukebemish commented Oct 4, 2024

Hmm. Yeah not sure the (Map)CodecInterpreter currently handles partials any special way -- can you share a repro case? I'll turn it into a test and get it working, if it's possible to do so.

@Gaming32
Copy link

Gaming32 commented Oct 5, 2024

Here's a repro. The first assertEquals passes, but the second fails.

final var codec = RecordCodecBuilder.<List<String>>create(
    instance -> instance.group(
        Codec.STRING.listOf().fieldOf("example").forGetter(Function.identity())
    ).apply(instance, Function.identity())
);
final var structure = Structure.<List<String>>record(i -> {
    final var example = i.add("example", Structure.STRING.listOf(), Function.identity());
    return container -> example.apply(container);
});

final var json = JsonParser.parseString("{\"example\": [\"abc\", 123, \"def\"]}");
final var expected = Optional.of(List.of("abc", "def"));

assertEquals(expected, codec.parse(JsonOps.INSTANCE, json).resultOrPartial());
assertEquals(
    expected,
    CodecInterpreter.create()
        .interpret(structure)
        .getOrThrow()
        .parse(JsonOps.INSTANCE, json)
        .resultOrPartial()
);

@lukebemish
Copy link
Member Author

Alright I see what you mean; I'll look into support for partials.

@Gaming32
Copy link

Gaming32 commented Oct 7, 2024

Side note, just in case someone wants to print a Key for debugging purposes, Key.toString should probably include its name (currently it's just the default identity toString).

@lukebemish
Copy link
Member Author

Oof yeah thanks

@lukebemish
Copy link
Member Author

Alright, 1.21.2 is a thing now so I'm going to try to get more work on this done soon -- I have a lot of work to do IRL, so there may be a bit of a delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants