-
Notifications
You must be signed in to change notification settings - Fork 525
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
[C#,C++,Java] Generate DTOs for non-perf-sensitive usecases. #957
Conversation
8d3b4c5
to
2c1f6e1
Compare
classpath = project(':sbe-tool').sourceSets.main.runtimeClasspath | ||
systemProperties( | ||
'sbe.output.dir': 'csharp/sbe-generated', | ||
'sbe.target.language': 'uk.co.real_logic.sbe.generation.csharp.CSharpDtos', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is better than the approach for multiple generators in Go in #951.
For that one, we added an extra flag sbe.go.generate.generate.flyweights=true
. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I struggled to choose between the two mechanisms: flag vs. generator. I opted for the generator, as it seemed less intrusive (and less likely to collide with changes in my other PR). However, at that time, I had not noticed the existing pattern for the Go generation. Using a flag avoids spinning up a new JVM and parsing the schema multiple times. Therefore, that might be a better pattern. We should be consistent in any case. I'm happy to use a flag instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency, I've added a system property, sbe.csharp.generate.dtos
, that enables DTO generation when using the CSharp
rather than CSharpDtos
target.
Feedback to consider:
|
d79d661
to
21ae253
Compare
sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/ParserPropertyTest.java
Fixed
Show fixed
Hide fixed
sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/ParserPropertyTest.java
Dismissed
Show dismissed
Hide dismissed
sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/CSharpDtosPropertyTest.java
Fixed
Show fixed
Hide fixed
f7bc822
to
b9e603f
Compare
implementation files('build/classes/java/generated') | ||
implementation "org.hamcrest:hamcrest:${hamcrestVersion}" | ||
implementation "org.mockito:mockito-core:${mockitoVersion}" | ||
implementation "org.junit.jupiter:junit-jupiter-params:${junitVersion}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that we're using jvm-test-suites
to add new source sets in a Gradle 9 compatible manner.
Here implementation
means testImplementation
in old money.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proof
zach@worky-ii ~/src/real-logic/simple-binary-encoding feature/dtos ⍟6 ↵ 1 ./gradlew sbe-tool:dependencies
Project ':sbe-tool'
annotationProcessor - Annotation processors and their dependencies for source set 'main'.
No dependenciesapi - API dependencies for source set 'main'. (n)
--- org.agrona:agrona:[1.19.2,2.0[ (n)apiElements - API elements for main. (n)
No dependenciescheckstyle - The Checkstyle libraries to be used for this project.
--- com.puppycrawl.tools:checkstyle:9.3
+--- info.picocli:picocli:4.6.2
+--- org.antlr:antlr4-runtime:4.9.3
+--- commons-beanutils:commons-beanutils:1.9.4
| --- commons-collections:commons-collections:3.2.2
+--- com.google.guava:guava:31.0.1-jre
| +--- com.google.guava:failureaccess:1.0.1
| +--- com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava
| +--- com.google.code.findbugs:jsr305:3.0.2
| +--- org.checkerframework:checker-qual:3.12.0
| +--- com.google.errorprone:error_prone_annotations:2.7.1
| --- com.google.j2objc:j2objc-annotations:1.3
+--- org.reflections:reflections:0.10.2
| +--- org.javassist:javassist:3.28.0-GA
| --- com.google.code.findbugs:jsr305:3.0.2
--- net.sf.saxon:Saxon-HE:10.6compileClasspath - Compile classpath for source set 'main'.
--- org.agrona:agrona:{strictly [1.19.2,2.0[; prefer 1.19.2} -> 1.19.2compileOnly - Compile only dependencies for source set 'main'. (n)
No dependenciescompileOnlyApi - Compile only API dependencies for source set 'main'. (n)
No dependenciesdefault - Configuration for default artifacts. (n)
No dependenciesgeneratedAnnotationProcessor - Annotation processors and their dependencies for source set 'generated'.
No dependenciesgeneratedCompileClasspath - Compile classpath for source set 'generated'.
No dependenciesgeneratedCompileOnly - Compile only dependencies for source set 'generated'. (n)
No dependenciesgeneratedImplementation - Implementation only dependencies for source set 'generated'. (n)
No dependenciesgeneratedRuntimeClasspath - Runtime classpath of source set 'generated'.
No dependenciesgeneratedRuntimeOnly - Runtime only dependencies for source set 'generated'. (n)
No dependenciesimplementation - Implementation only dependencies for source set 'main'. (n)
No dependenciesjavadocElements - javadoc elements for main. (n)
No dependenciesmainSourceElements - List of source directories contained in the Main SourceSet. (n)
No dependenciespropertyTestAnnotationProcessor - Annotation processors and their dependencies for source set 'property test'.
No dependenciespropertyTestCompileClasspath - Compile classpath for source set 'property test'.
+--- project :sbe-tool ()
+--- net.jqwik:jqwik:1.8.0
| +--- org.apiguardian:apiguardian-api:1.1.2
| +--- net.jqwik:jqwik-api:1.8.0
| | +--- org.apiguardian:apiguardian-api:1.1.2
| | +--- org.opentest4j:opentest4j:1.3.0
| | --- org.junit.platform:junit-platform-commons:1.10.0
| | +--- org.junit:junit-bom:5.10.0
| | | +--- org.junit.jupiter:junit-jupiter:5.10.0 (c)
| | | +--- org.junit.jupiter:junit-jupiter-api:5.10.0 (c)
| | | +--- org.junit.jupiter:junit-jupiter-params:5.10.0 (c)
| | | --- org.junit.platform:junit-platform-commons:1.10.0 (c)
| | --- org.apiguardian:apiguardian-api:1.1.2
| +--- net.jqwik:jqwik-web:1.8.0
| | +--- org.apiguardian:apiguardian-api:1.1.2
| | +--- net.jqwik:jqwik-api:1.8.0 ()
| | --- org.opentest4j:opentest4j:1.3.0
| --- net.jqwik:jqwik-time:1.8.0
| +--- org.apiguardian:apiguardian-api:1.1.2
| +--- net.jqwik:jqwik-api:1.8.0 ()
| --- org.opentest4j:opentest4j:1.3.0
+--- org.json:json:20230618
+--- org.junit.jupiter:junit-jupiter:5.10.0
| +--- org.junit:junit-bom:5.10.0 ()
| +--- org.junit.jupiter:junit-jupiter-api:5.10.0
| | +--- org.junit:junit-bom:5.10.0 ()
| | +--- org.opentest4j:opentest4j:1.3.0
| | +--- org.junit.platform:junit-platform-commons:1.10.0 ()
| | --- org.apiguardian:apiguardian-api:1.1.2
| --- org.junit.jupiter:junit-jupiter-params:5.10.0
| +--- org.junit:junit-bom:5.10.0 ()
| +--- org.junit.jupiter:junit-jupiter-api:5.10.0 ()
| --- org.apiguardian:apiguardian-api:1.1.2
--- org.agrona:agrona:{strictly [1.19.2,2.0[; prefer 1.19.2} -> 1.19.2propertyTestCompileOnly - Compile only dependencies for source set 'property test'. (n)
No dependenciespropertyTestImplementation - Implementation only dependencies for source set 'property test'. (n)
+--- project sbe-tool (n)
+--- net.jqwik:jqwik:1.8.0 (n)
--- org.json:json:20230618 (n)propertyTestRuntimeClasspath - Runtime classpath of source set 'property test'.
+--- project :sbe-tool ()
+--- net.jqwik:jqwik:1.8.0
| +--- org.apiguardian:apiguardian-api:1.1.2
| +--- net.jqwik:jqwik-api:1.8.0
| | +--- org.apiguardian:apiguardian-api:1.1.2
| | +--- org.opentest4j:opentest4j:1.3.0
| | --- org.junit.platform:junit-platform-commons:1.10.0
| | --- org.junit:junit-bom:5.10.0
| | +--- org.junit.jupiter:junit-jupiter:5.10.0 (c)
| | +--- org.junit.jupiter:junit-jupiter-api:5.10.0 (c)
| | +--- org.junit.jupiter:junit-jupiter-engine:5.10.0 (c)
| | +--- org.junit.jupiter:junit-jupiter-params:5.10.0 (c)
| | +--- org.junit.platform:junit-platform-commons:1.10.0 (c)
| | +--- org.junit.platform:junit-platform-engine:1.10.0 (c)
| | --- org.junit.platform:junit-platform-launcher:1.10.0 (c)
| +--- net.jqwik:jqwik-web:1.8.0
| | +--- org.apiguardian:apiguardian-api:1.1.2
| | +--- net.jqwik:jqwik-api:1.8.0 ()
| | --- org.opentest4j:opentest4j:1.3.0
| +--- net.jqwik:jqwik-time:1.8.0
| | +--- org.apiguardian:apiguardian-api:1.1.2
| | +--- net.jqwik:jqwik-api:1.8.0 ()
| | --- org.opentest4j:opentest4j:1.3.0
| --- net.jqwik:jqwik-engine:1.8.0
| +--- org.junit.platform:junit-platform-engine:1.10.0
| | +--- org.junit:junit-bom:5.10.0 ()
| | +--- org.opentest4j:opentest4j:1.3.0
| | --- org.junit.platform:junit-platform-commons:1.10.0 ()
| +--- org.apiguardian:apiguardian-api:1.1.2
| +--- net.jqwik:jqwik-api:1.8.0 ()
| +--- org.opentest4j:opentest4j:1.3.0
| --- org.junit.platform:junit-platform-commons:1.10.0 ()
+--- org.json:json:20230618
+--- org.junit.jupiter:junit-jupiter:5.10.0
| +--- org.junit:junit-bom:5.10.0 ()
| +--- org.junit.jupiter:junit-jupiter-api:5.10.0
| | +--- org.junit:junit-bom:5.10.0 ()
| | +--- org.opentest4j:opentest4j:1.3.0
| | --- org.junit.platform:junit-platform-commons:1.10.0 ()
| +--- org.junit.jupiter:junit-jupiter-params:5.10.0
| | +--- org.junit:junit-bom:5.10.0 ()
| | --- org.junit.jupiter:junit-jupiter-api:5.10.0 ()
| --- org.junit.jupiter:junit-jupiter-engine:5.10.0
| +--- org.junit:junit-bom:5.10.0 ()
| +--- org.junit.platform:junit-platform-engine:1.10.0 ()
| --- org.junit.jupiter:junit-jupiter-api:5.10.0 ()
+--- org.junit.platform:junit-platform-launcher -> 1.10.0
| +--- org.junit:junit-bom:5.10.0 ()
| --- org.junit.platform:junit-platform-engine:1.10.0 (*)
--- org.agrona:agrona:{strictly [1.19.2,2.0[; prefer 1.19.2} -> 1.19.2propertyTestRuntimeOnly - Runtime only dependencies for source set 'property test'. (n)
No dependenciesruntimeClasspath - Runtime classpath of source set 'main'.
--- org.agrona:agrona:{strictly [1.19.2,2.0[; prefer 1.19.2} -> 1.19.2runtimeElements - Elements of runtime for main. (n)
No dependenciesruntimeOnly - Runtime only dependencies for source set 'main'. (n)
No dependenciessignatures (n)
No dependenciessourcesElements - sources elements for main. (n)
No dependenciestestAnnotationProcessor - Annotation processors and their dependencies for source set 'test'.
No dependenciestestCompileClasspath - Compile classpath for source set 'test'.
+--- org.agrona:agrona:{strictly [1.19.2,2.0[; prefer 1.19.2} -> 1.19.2
+--- org.hamcrest:hamcrest:2.2
+--- org.mockito:mockito-core:4.11.0
| +--- net.bytebuddy:byte-buddy:1.12.19
| --- net.bytebuddy:byte-buddy-agent:1.12.19
+--- org.junit.jupiter:junit-jupiter-params:5.10.0
| +--- org.junit:junit-bom:5.10.0
| | +--- org.junit.jupiter:junit-jupiter:5.10.0 (c)
| | +--- org.junit.jupiter:junit-jupiter-api:5.10.0 (c)
| | +--- org.junit.jupiter:junit-jupiter-params:5.10.0 (c)
| | --- org.junit.platform:junit-platform-commons:1.10.0 (c)
| +--- org.junit.jupiter:junit-jupiter-api:5.10.0
| | +--- org.junit:junit-bom:5.10.0 ()
| | +--- org.opentest4j:opentest4j:1.3.0
| | +--- org.junit.platform:junit-platform-commons:1.10.0
| | | +--- org.junit:junit-bom:5.10.0 ()
| | | --- org.apiguardian:apiguardian-api:1.1.2
| | --- org.apiguardian:apiguardian-api:1.1.2
| --- org.apiguardian:apiguardian-api:1.1.2
--- org.junit.jupiter:junit-jupiter:5.10.0
+--- org.junit:junit-bom:5.10.0 ()
+--- org.junit.jupiter:junit-jupiter-api:5.10.0 ()
--- org.junit.jupiter:junit-jupiter-params:5.10.0 (*)testCompileOnly - Compile only dependencies for source set 'test'. (n)
No dependenciestestImplementation - Implementation only dependencies for source set 'test'. (n)
+--- unspecified (n)
+--- org.hamcrest:hamcrest:2.2 (n)
+--- org.mockito:mockito-core:4.11.0 (n)
--- org.junit.jupiter:junit-jupiter-params:5.10.0 (n)testRuntimeClasspath - Runtime classpath of source set 'test'.
+--- org.agrona:agrona:{strictly [1.19.2,2.0[; prefer 1.19.2} -> 1.19.2
+--- org.hamcrest:hamcrest:2.2
+--- org.mockito:mockito-core:4.11.0
| +--- net.bytebuddy:byte-buddy:1.12.19
| +--- net.bytebuddy:byte-buddy-agent:1.12.19
| --- org.objenesis:objenesis:3.3
+--- org.junit.jupiter:junit-jupiter-params:5.10.0
| +--- org.junit:junit-bom:5.10.0
| | +--- org.junit.jupiter:junit-jupiter:5.10.0 (c)
| | +--- org.junit.jupiter:junit-jupiter-api:5.10.0 (c)
| | +--- org.junit.jupiter:junit-jupiter-engine:5.10.0 (c)
| | +--- org.junit.jupiter:junit-jupiter-params:5.10.0 (c)
| | +--- org.junit.platform:junit-platform-launcher:1.10.0 (c)
| | +--- org.junit.platform:junit-platform-commons:1.10.0 (c)
| | --- org.junit.platform:junit-platform-engine:1.10.0 (c)
| --- org.junit.jupiter:junit-jupiter-api:5.10.0
| +--- org.junit:junit-bom:5.10.0 ()
| +--- org.opentest4j:opentest4j:1.3.0
| --- org.junit.platform:junit-platform-commons:1.10.0
| --- org.junit:junit-bom:5.10.0 ()
+--- org.junit.jupiter:junit-jupiter:5.10.0
| +--- org.junit:junit-bom:5.10.0 ()
| +--- org.junit.jupiter:junit-jupiter-api:5.10.0 ()
| +--- org.junit.jupiter:junit-jupiter-params:5.10.0 ()
| --- org.junit.jupiter:junit-jupiter-engine:5.10.0
| +--- org.junit:junit-bom:5.10.0 ()
| +--- org.junit.platform:junit-platform-engine:1.10.0
| | +--- org.junit:junit-bom:5.10.0 ()
| | +--- org.opentest4j:opentest4j:1.3.0
| | --- org.junit.platform:junit-platform-commons:1.10.0 ()
| --- org.junit.jupiter:junit-jupiter-api:5.10.0 ()
--- org.junit.platform:junit-platform-launcher -> 1.10.0
+--- org.junit:junit-bom:5.10.0 ()
--- org.junit.platform:junit-platform-engine:1.10.0 (*)testRuntimeOnly - Runtime only dependencies for source set 'test'. (n)
No dependencies(c) - A dependency constraint, not a dependency. The dependency affected by the constraint occurs elsewhere in the tree.
(*) - Indicates repeated occurrences of a transitive dependency subtree. Gradle expands transitive dependency subtrees only once per project; repeat occurrences only display the root of the subtree, followed by this annotation.(n) - A dependency or dependency configuration that cannot be resolved.
A web-based, searchable dependency report is available by adding the --scan option.
build.gradle
Outdated
implementation project() | ||
implementation "net.jqwik:jqwik:${jqwikVersion}" | ||
implementation "org.json:json:${jsonVersion}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that we're using jvm-test-suites
to add new source sets in a Gradle 9 compatible manner.
Here implementation
means propertyTestImplementation
in old money.
120b30d
to
da32de0
Compare
@kieranelby, please can someone kick the tyres on your side and let me know if you'd prefer different representations? To use it, you can supply |
sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/csharp/CSharpDtoGenerator.java
Outdated
Show resolved
Hide resolved
16d914f
to
5eba82c
Compare
The aim of these changes are to avoid multiple representations in DTOs and to support read-only views over data. The new validation includes: - Checking null values "idiomatic null values" rather than the reserved null value, to prevent against multiple kinds of null in DTOs. - Checking primitive field values are at least `minValue` and at most `maxValue`. Note that this validation is not applied to fixed-size arrays as the specification says, "Data range attributes minValue and maxValue do not apply", under the "Fixed-length data" section. Records are now immutable. Record expressions, i.e., using `with`, are supported and will apply validation, as we have customised the `init` property accessor. I have not included support for encoding null composite values from DTOs; however, in theory, this could be added later.
Previously, you had to pass the `CSharpDtos` FQN as the `TargetCodeGenerator` when running sbe-tool; however, that means the XML schema was parsed multiple times, as DTOs depend on flyweights, which seems wasteful. Therefore, I have introduced a system property, `sbe.csharp.generate.dtos` that also controls the generation of DTOs when targetting `CSharp`.
These tests showed some deficiencies in the DTO generation. For example, added variable-length data was not represented as nullable nor properly handled in `EncodeInto(...)`. During this work, I noticed composite "field" tokens (i.e., types) take their `token.version()` from their containing message/group field. I had to adjust some code that was using `token.version() > 0` to determine whether a field had been added, as this only works with message/group fields.
…tion. The SBE spec doesn't explicitly allow variable-length data or groups to be added to existing message schemas, i.e., only block-level fields may be added; however in practice the encoders/decoders for some languages do support the addition of var data fields. Previously, I had represented these "added" var data fields as optional strings or byte arrays, but having chatted with Martin, we think it is better to mimick the existing decoder representation, i.e., use empty strings/arrays to represent missing elements. In this commit, I've also fixed some instances where I was checking `token.version() > 0` where I should have been checking `token.version() > sinceVersionOfParentContainer`. Ideally, I would have liked to remove such checks entirely, but the C# and C++ codecs do not give sensible responses when decoding older versions in some cases, i.e., you _must_ check the presence of the field before accessing it.
This commit addresses some feedback from Martin: - Makes encode and decode methods static - Improves naming: - `EncodeInto(codec)` -> `EncodeWith` - `DecodeFrom(codec)` -> `DecodeWith` - Introduces version that works directly with buffer
In some applications performance is not cricital. Some users would like to use SBE across their whole "estate", but don't want the "sharp edges" associated with using flyweight codecs, e.g., accidental escape. In this commit, I've added a first cut of DTO generation for C++ and a simple test based on the Car Example. The DTOs support encoding and decoding via the generated codecs using `DtoT::encode(CodecT& codec, const DtoT& dto)` and `DtoT::decode(CodecT& codec, Dto& dto)` methods. Generation can be enabled specifying the target code generator class, `uk.co.real_logic.sbe.generation.cpp.CppDtos`, or by passing a system property `-Dsbe.cpp.generate.dtos=true`.
Changes: - `encode` -> `encodeWith` - `decode` -> `decodeWith`
As we use the generated codecs to create a string representation of our DTOs, we don't use Agrona buffers in C++, and there is no concept of resizing, it is necessary to size a temporary buffer during the construction of the string data. Previously, we were letting the user supply this value, which wasn't a very friendly API. Now, we use the `computeLength` methods on the codec to determine how big of a temporary buffer we need. Perhaps the methods will also be useful for avoiding a buffer copy when used in conjunction with Aeron. For example, a developer could use `dto.computeEncodedLength()` to initialise a buffer claim rather than copying via the `offer(...)` API.
I had incorrectly assumed that the `Flyweight::computeLength` method took _encoded lengths_ of groups etc., but actually it takes a complicated structure of group counts and variable lengths. As it was hard to build this list, I've opted for a simpler approach: do the length calculation within the generated DTO message and its groups. In this commit, I've also added some convenience methods for converting between DTOs and "byte arrays".
It is more-idiomatic to represent variable-length data using `std::string` even when there is no character encoding specified, the the `std::string` API provides useful utilities regardless.
The main change in this commit is to add property-based testing around the C++ DTOs. We check the same property as the C# tests, i.e., that decoding and re-encoding via a DTO preserves the original bytes. There are some other smaller changes in this commit: 1. We now only generate arbitrary schemas where enums have at least one case, as this is required by the spec. 2. We now log details about how we encoded the `input.dat` file used in property-based tests, which can help diagnose problems in the test.
Previously, when generating arbitrary encoded values the generator would not necessarily extend the buffer to the size necessary to hold the message, e.g., if optional fields were left unset at the end of the message block. We now call `checkLimit` to "reserve" space for the full block length at the message and group level. This fixes an exception seen in the slow tests: ``` java.lang.IndexOutOfBoundsException: index=0 length=129 capacity=128 at org.agrona.AbstractMutableDirectBuffer.boundsCheck0(AbstractMutableDirectBuffer.java:1719) at org.agrona.AbstractMutableDirectBuffer.getBytes(AbstractMutableDirectBuffer.java:464) at org.agrona.io.DirectBufferInputStream.read(DirectBufferInputStream.java:175) at uk.co.real_logic.sbe.properties.DtosPropertyTest.writeInputFile(DtosPropertyTest.java:199) at uk.co.real_logic.sbe.properties.DtosPropertyTest.cppDtoEncodeShouldBeTheInverseOfDtoDecode(DtosPropertyTest.java:147) at java.lang.reflect.Method.invoke(Method.java:498) at net.jqwik.engine.execution.CheckedPropertyFactory.lambda$createRawFunction$1(CheckedPropertyFactory.java:84) at net.jqwik.engine.execution.CheckedPropertyFactory.lambda$createRawFunction$2(CheckedPropertyFactory.java:91) at net.jqwik.engine.properties.CheckedFunction.execute(CheckedFunction.java:17) at net.jqwik.api.lifecycle.AroundTryHook.lambda$static$0(AroundTryHook.java:57) at net.jqwik.engine.execution.lifecycle.HookSupport.lambda$wrap$2(HookSupport.java:48) at net.jqwik.engine.hooks.lifecycle.TryLifecycleMethodsHook.aroundTry(TryLifecycleMethodsHook.java:57) at net.jqwik.engine.execution.lifecycle.HookSupport.lambda$wrap$3(HookSupport.java:53) at net.jqwik.engine.execution.lifecycle.HookSupport.lambda$wrap$2(HookSupport.java:48) at net.jqwik.engine.hooks.lifecycle.BeforeTryMembersHook.aroundTry(BeforeTryMembersHook.java:69) at net.jqwik.engine.execution.lifecycle.HookSupport.lambda$wrap$3(HookSupport.java:53) at net.jqwik.engine.execution.CheckedPropertyFactory.lambda$createTryExecutor$0(CheckedPropertyFactory.java:60) at net.jqwik.engine.execution.lifecycle.AroundTryLifecycle.execute(AroundTryLifecycle.java:23) at net.jqwik.engine.properties.GenericProperty.testPredicate(GenericProperty.java:166) at net.jqwik.engine.properties.GenericProperty.check(GenericProperty.java:68) at net.jqwik.engine.execution.CheckedProperty.check(CheckedProperty.java:67) at net.jqwik.engine.execution.PropertyMethodExecutor.executeProperty(PropertyMethodExecutor.java:90) at net.jqwik.engine.execution.PropertyMethodExecutor.executeMethod(PropertyMethodExecutor.java:69) at net.jqwik.engine.execution.PropertyMethodExecutor.lambda$execute$0(PropertyMethodExecutor.java:49) at net.jqwik.api.lifecycle.AroundPropertyHook.lambda$static$0(AroundPropertyHook.java:46) at net.jqwik.engine.execution.lifecycle.HookSupport.lambda$wrap$0(HookSupport.java:26) at net.jqwik.api.lifecycle.PropertyExecutor.executeAndFinally(PropertyExecutor.java:39) at net.jqwik.engine.hooks.lifecycle.PropertyLifecycleMethodsHook.aroundProperty(PropertyLifecycleMethodsHook.java:56) at net.jqwik.engine.execution.lifecycle.HookSupport.lambda$wrap$1(HookSupport.java:31) at net.jqwik.engine.execution.lifecycle.HookSupport.lambda$wrap$0(HookSupport.java:26) at net.jqwik.engine.hooks.statistics.StatisticsHook.aroundProperty(StatisticsHook.java:37) at net.jqwik.engine.execution.lifecycle.HookSupport.lambda$wrap$1(HookSupport.java:31) at net.jqwik.engine.execution.lifecycle.HookSupport.lambda$wrap$0(HookSupport.java:26) at net.jqwik.engine.hooks.lifecycle.AutoCloseableHook.aroundProperty(AutoCloseableHook.java:13) at net.jqwik.engine.execution.lifecycle.HookSupport.lambda$wrap$1(HookSupport.java:31) at net.jqwik.engine.execution.PropertyMethodExecutor.execute(PropertyMethodExecutor.java:47) at net.jqwik.engine.execution.PropertyTaskCreator.executeTestMethod(PropertyTaskCreator.java:166) at net.jqwik.engine.execution.PropertyTaskCreator.lambda$createTask$1(PropertyTaskCreator.java:51) at net.jqwik.engine.execution.lifecycle.CurrentDomainContext.runWithContext(CurrentDomainContext.java:28) at net.jqwik.engine.execution.PropertyTaskCreator.lambda$createTask$2(PropertyTaskCreator.java:50) at net.jqwik.engine.execution.pipeline.ExecutionTask$1.lambda$execute$0(ExecutionTask.java:31) at net.jqwik.engine.execution.lifecycle.CurrentTestDescriptor.runWithDescriptor(CurrentTestDescriptor.java:17) at net.jqwik.engine.execution.pipeline.ExecutionTask$1.execute(ExecutionTask.java:31) at net.jqwik.engine.execution.pipeline.ExecutionPipeline.runToTermination(ExecutionPipeline.java:82) at net.jqwik.engine.execution.JqwikExecutor.execute(JqwikExecutor.java:46) at net.jqwik.engine.JqwikTestEngine.executeTests(JqwikTestEngine.java:70) at net.jqwik.engine.JqwikTestEngine.execute(JqwikTestEngine.java:53) ```
Recently JUnit was upgraded. Rebasing revealed a dependency conflict.
This commit adds support for generating SBE DTOs in Java. Unlike the C# and C++ support, there isn't a natural candidate for the idiomatic representation of optional fields, e.g., `Optional<T>` is only meant for use as a return type. Therefore, the mapping between Java types and the encoding is simpler than in these other languages. In a future commit, I will extend the property-based tests to cover Java DTOs.
The added test (non-exhaustively) checks the property: ``` ∀ msg ∈ MessageSchemas, ∀ encoding ∈ EncodingsOf(msg), encoding = dtoEncode(dtoDecode(encoding)) ``` The added test shows some compilation failures, which need to be resolved before enabling it.
Most not present conditions were already elided via `generateFieldNotPresentCondition(inComposite=true, ...)` but the condition within the wrap method was not. Here's an example of a failing schema found with the DTO property-based tests: ``` <?xml version="1.0" encoding="UTF-8" standalone="no"?> <sbe:messageSchema id="42" package="uk.co.real_logic.sbe.properties" version="1" xmlns:sbe="http://fixprotocol.io/2016/sbe"> <types> <composite name="messageHeader"> <type name="blockLength" primitiveType="uint16"/> <type name="templateId" primitiveType="uint16"/> <type name="schemaId" primitiveType="uint16"/> <type name="version" primitiveType="uint16"/> </composite> <composite name="groupSizeEncoding"> <type name="blockLength" primitiveType="uint16"/> <type name="numInGroup" primitiveType="uint16"/> </composite> <composite name="Type1"> <type length="2" name="member0OfType0" presence="required" primitiveType="uint8"/> </composite> </types> <sbe:message id="1" name="TestMessage"> <field id="0" name="member0" presence="optional" sinceVersion="1" type="Type1"/> </sbe:message> </sbe:messageSchema> ``` And generated code (within a composite without a `parentMessage`): ``` public void wrapMember0OfType0(final DirectBuffer wrapBuffer) { if (parentMessage.actingVersion < 1) { wrapBuffer.wrap(buffer, offset, 0); return; } wrapBuffer.wrap(buffer, offset + 0, 2); } ```
The main issue was around determining when a field might not be present. We were checking `token.version() > 0`, which isn't accurate in composites or groups. In this commit, I've also started to use the `Footnotes` API in JQWIK, which allows useful debug information to be captured _only_ for failing attempts. It then prints out the shrunken footnotes upon a failure.
@ZachBray Thanks for this awesome job. Consider the following setup: <sbe:messageSchema xmlns:sbe="http://fixprotocol.io/2016/sbe"
xmlns:xi="http://www.w3.org/2001/XInclude"
package="messages.sbe">
<xi:include href="sbe-common-types.xml"/>
and inside <types package="messages.sbe.common"> In this case the DTOs will miss all the type imports from |
Thanks for the report, @ratcashdev. I think this may have recently been fixed. Have you tried 1.33.2? |
I confirm, 1.33.2 solves this for JAVA at least. |
@ratcashdev You've added support for package overrides for Java in #904. It looks like C# and C++ do not have support for this feature yet. That support needs to be added and then DTO generation could be adjusted as well. |
Overview
In some applications, performance is not critical. Some users would like to use SBE across their whole "estate" but don't want the "sharp edges" associated with flyweight codecs, e.g., usage not aligning with data lifetimes.
In this PR, I've added DTO generation for C#, C++, and Java.
I'm using property-based tests to gain confidence that the DTOs are working correctly. In particular, I'm checking the following property (albeit not exhaustively):
I.e., for any message schema
dtoEncode
is the inverse ofdtoDecode
and the "round trip" preserves all information in the original encoding.These tests run periodically rather than on every commit; however, I've tested out the CI job using a PR hook here.
Implementation notes
The DTOs support encoding and decoding via the generated codecs using
static void EncodeWith(CodecT codec, DtoT dto)
andstatic DtoT DecodeWith(CodecT codec)
methods.C# Representations
Messages and composites are represented as immutable records.
init
accessors are provided so that record expressions may be used, e.g.,x with { Y = Z }
.ToString()
does not show what is inside groups etc.; therefore, we provideToSbeString()
as well.Groups are represented as
IReadOnlyList<GroupT>
Added/optional primitives are represented as nullable types.
null
indicates the value is not filled. The reserved null value defined explicitly in the schema or implicitly by the SBE specification is not permitted for use within the DTOs, as this would lead to multiple representations ofnull
in consuming application code. Both constructors andinit
accessors validate that values are in the allowed range.Added fixed-length data is represented through nullable reference types, e.g.,
string?
andIReadOnlyList<byte>?
. Missing data, e.g., due to the encoding version, is represented asnull
.Missing, added variable-length data is represented as an empty string or array, similarly to the codecs.
Enums and bitsets use the existing codec representations, i.e., generated enums.
0
.Other changes
6.0
(LTS) rather than3.x
for CI build and testssbe-dll
still targets the (quite ancient, ~2017).NET Standard 2.0
but no longer the (very ancient, ~2012).NET Framework 4.5
..NET Framework 4.5
to a minimum of.NET Framework 4.6.1
float
anddouble
are their minimum +ve representable values rather than minimum -ve values.