Skip to content

Commit 248eae9

Browse files
ZachBrayZachBray
committed
[C#] Extend tests to cover extended schemas.
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.
1 parent bc17a46 commit 248eae9

File tree

8 files changed

+184
-38
lines changed

8 files changed

+184
-38
lines changed

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/csharp/CSharpDtoGenerator.java

Lines changed: 65 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ public void generate() throws IOException
9393
collectVarData(messageBody, offset, varData);
9494
generateVarData(sb, ctorArgs, varData, BASE_INDENT + INDENT);
9595

96-
generateDecodeFrom(sb, className, codecClassName, fields, groups, varData, BASE_INDENT + INDENT);
97-
generateEncodeInto(sb, codecClassName, fields, groups, varData, BASE_INDENT + INDENT);
96+
generateMessageDecodeFrom(sb, className, codecClassName, fields, groups, varData, BASE_INDENT + INDENT);
97+
generateMessageEncodeInto(sb, codecClassName, fields, groups, varData, BASE_INDENT + INDENT);
9898
generateDisplay(sb, codecClassName, "WrapForEncode", null, BASE_INDENT + INDENT);
9999

100100
removeTrailingComma(ctorArgs);
@@ -119,6 +119,12 @@ public void generate() throws IOException
119119
}
120120
}
121121

122+
private enum DefinitionKind
123+
{
124+
Composite,
125+
Message
126+
}
127+
122128
private void generateGroups(
123129
final StringBuilder sb,
124130
final StringBuilder ctorArgs,
@@ -174,9 +180,9 @@ private void generateGroups(
174180

175181
generateDecodeListFrom(
176182
groupRecordBody, groupClassName, qualifiedCodecClassName, indent + INDENT);
177-
generateDecodeFrom(
183+
generateMessageDecodeFrom(
178184
groupRecordBody, groupClassName, qualifiedCodecClassName, fields, groups, varData, indent + INDENT);
179-
generateEncodeInto(
185+
generateMessageEncodeInto(
180186
groupRecordBody, qualifiedCodecClassName, fields, groups, varData, indent + INDENT);
181187

182188
removeTrailingComma(groupCtorArgs);
@@ -210,7 +216,8 @@ private void generateCompositeDecodeFrom(
210216
{
211217
final Token token = tokens.get(i);
212218

213-
generateFieldDecodeFrom(sb, token, token, codecClassName, indent + INDENT + INDENT);
219+
generateFieldDecodeFrom(
220+
sb, DefinitionKind.Composite, token, token, codecClassName, indent + INDENT + INDENT);
214221

215222
i += tokens.get(i).componentTokenCount();
216223
}
@@ -270,7 +277,7 @@ private void generateDecodeListFrom(
270277
.append(indent).append("}\n");
271278
}
272279

273-
private void generateDecodeFrom(
280+
private void generateMessageDecodeFrom(
274281
final StringBuilder sb,
275282
final String dtoClassName,
276283
final String codecClassName,
@@ -285,7 +292,7 @@ private void generateDecodeFrom(
285292
.append(indent).append("{\n");
286293

287294
sb.append(indent).append(INDENT).append("return new ").append(dtoClassName).append("(\n");
288-
generateFieldsDecodeFrom(sb, fields, codecClassName, indent + INDENT + INDENT);
295+
generateMessageFieldsDecodeFrom(sb, fields, codecClassName, indent + INDENT + INDENT);
289296
generateGroupsDecodeFrom(sb, groups, indent + INDENT + INDENT);
290297
generateVarDataDecodeFrom(sb, varData, indent + INDENT + INDENT);
291298
removeTrailingComma(sb);
@@ -294,7 +301,7 @@ private void generateDecodeFrom(
294301
sb.append(indent).append("}\n");
295302
}
296303

297-
private void generateFieldsDecodeFrom(
304+
private void generateMessageFieldsDecodeFrom(
298305
final StringBuilder sb,
299306
final List<Token> tokens,
300307
final String codecClassName,
@@ -307,35 +314,37 @@ private void generateFieldsDecodeFrom(
307314
{
308315
final Token encodingToken = tokens.get(i + 1);
309316

310-
generateFieldDecodeFrom(sb, signalToken, encodingToken, codecClassName, indent);
317+
generateFieldDecodeFrom(sb, DefinitionKind.Message, signalToken, encodingToken, codecClassName, indent);
311318
}
312319
}
313320
}
314321

315322
private void generateFieldDecodeFrom(
316323
final StringBuilder sb,
324+
final DefinitionKind rootKind,
317325
final Token fieldToken,
318326
final Token typeToken,
319-
final String codecClassName, final String indent)
327+
final String codecClassName,
328+
final String indent)
320329
{
321330
switch (typeToken.signal())
322331
{
323332
case ENCODING:
324-
generatePrimitiveDecodeFrom(sb, fieldToken, typeToken, codecClassName, indent);
333+
generatePrimitiveDecodeFrom(sb, rootKind, fieldToken, typeToken, codecClassName, indent);
325334
break;
326335

327336
case BEGIN_SET:
328-
generatePropertyDecodeFrom(sb, fieldToken, "0", null, indent);
337+
generatePropertyDecodeFrom(sb, rootKind, fieldToken, "0", null, indent);
329338
break;
330339

331340
case BEGIN_ENUM:
332341
final String enumName = formatClassName(typeToken.applicableTypeName());
333342
final String nullValue = formatNamespace(ir.packageName()) + "." + enumName + ".NULL_VALUE";
334-
generatePropertyDecodeFrom(sb, fieldToken, nullValue, null, indent);
343+
generatePropertyDecodeFrom(sb, rootKind, fieldToken, nullValue, null, indent);
335344
break;
336345

337346
case BEGIN_COMPOSITE:
338-
generateComplexDecodeFrom(sb, fieldToken, typeToken, indent);
347+
generateComplexDecodeFrom(sb, rootKind, fieldToken, typeToken, indent);
339348
break;
340349

341350
default:
@@ -345,6 +354,7 @@ private void generateFieldDecodeFrom(
345354

346355
private void generatePrimitiveDecodeFrom(
347356
final StringBuilder sb,
357+
final DefinitionKind rootKind,
348358
final Token fieldToken,
349359
final Token typeToken,
350360
final String codecClassName,
@@ -360,16 +370,17 @@ private void generatePrimitiveDecodeFrom(
360370
if (arrayLength == 1)
361371
{
362372
final String codecNullValue = codecClassName + "." + formatPropertyName(fieldToken.name()) + "NullValue";
363-
generatePropertyDecodeFrom(sb, fieldToken, "null", codecNullValue, indent);
373+
generatePropertyDecodeFrom(sb, rootKind, fieldToken, "null", codecNullValue, indent);
364374
}
365375
else if (arrayLength > 1)
366376
{
367-
generateArrayDecodeFrom(sb, fieldToken, typeToken, indent);
377+
generateArrayDecodeFrom(sb, rootKind, fieldToken, typeToken, indent);
368378
}
369379
}
370380

371381
private void generateArrayDecodeFrom(
372382
final StringBuilder sb,
383+
final DefinitionKind rootKind,
373384
final Token fieldToken,
374385
final Token typeToken,
375386
final String indent)
@@ -386,6 +397,7 @@ private void generateArrayDecodeFrom(
386397
{
387398
generateRecordPropertyAssignment(
388399
sb,
400+
rootKind,
389401
fieldToken,
390402
indent,
391403
"codec.Get" + formattedPropertyName + "()",
@@ -397,6 +409,7 @@ private void generateArrayDecodeFrom(
397409
{
398410
generateRecordPropertyAssignment(
399411
sb,
412+
rootKind,
400413
fieldToken,
401414
indent,
402415
"codec." + formattedPropertyName + "AsSpan().ToArray()",
@@ -408,6 +421,7 @@ private void generateArrayDecodeFrom(
408421

409422
private void generatePropertyDecodeFrom(
410423
final StringBuilder sb,
424+
final DefinitionKind rootKind,
411425
final Token fieldToken,
412426
final String dtoNullValue,
413427
final String codecNullValue,
@@ -423,6 +437,7 @@ private void generatePropertyDecodeFrom(
423437

424438
generateRecordPropertyAssignment(
425439
sb,
440+
rootKind,
426441
fieldToken,
427442
indent,
428443
"codec." + formattedPropertyName,
@@ -433,6 +448,7 @@ private void generatePropertyDecodeFrom(
433448

434449
private void generateComplexDecodeFrom(
435450
final StringBuilder sb,
451+
final DefinitionKind rootKind,
436452
final Token fieldToken,
437453
final Token typeToken,
438454
final String indent)
@@ -443,6 +459,7 @@ private void generateComplexDecodeFrom(
443459

444460
generateRecordPropertyAssignment(
445461
sb,
462+
rootKind,
446463
fieldToken,
447464
indent,
448465
dtoClassName + ".DecodeFrom(codec." + formattedPropertyName + ")",
@@ -469,6 +486,7 @@ private void generateGroupsDecodeFrom(
469486

470487
generateRecordPropertyAssignment(
471488
sb,
489+
DefinitionKind.Message,
472490
groupToken,
473491
indent,
474492
groupDtoClassName + ".DecodeListFrom(codec." + formattedPropertyName + ")",
@@ -511,6 +529,7 @@ private void generateVarDataDecodeFrom(
511529

512530
generateRecordPropertyAssignment(
513531
sb,
532+
DefinitionKind.Message,
514533
token,
515534
indent,
516535
"codec." + accessor + "()",
@@ -523,6 +542,7 @@ private void generateVarDataDecodeFrom(
523542

524543
private void generateRecordPropertyAssignment(
525544
final StringBuilder sb,
545+
final DefinitionKind rootKind,
526546
final Token token,
527547
final String indent,
528548
final String presentExpression,
@@ -534,8 +554,20 @@ private void generateRecordPropertyAssignment(
534554

535555
sb.append(indent).append(formattedPropertyName).append(": ");
536556

537-
if (token.version() > 0)
557+
// N.B., in the IR, the composite information is "embedded" into each message/group field.
558+
// When we are looking at a composite "field" (i.e., a type) the `token.version()` method
559+
// returns the "sinceVersion" of the containing field. Therefore, we cannot decide presence
560+
// based on `token.version()` when dealing with composites.
561+
if (rootKind.equals(DefinitionKind.Message) && token.version() > 0)
538562
{
563+
if (token.signal() != Signal.BEGIN_VAR_DATA && !token.isOptionalEncoding())
564+
{
565+
throw new IllegalStateException(
566+
"Expected added field " + propertyName +
567+
" (sinceVersion=" + token.version() + ") to have optional presence."
568+
);
569+
}
570+
539571
sb.append("codec.").append(formattedPropertyName).append("InActingVersion()");
540572

541573
if (null != nullCodecValueOrNull)
@@ -553,7 +585,7 @@ private void generateRecordPropertyAssignment(
553585
}
554586
}
555587

556-
private void generateEncodeInto(
588+
private void generateMessageEncodeInto(
557589
final StringBuilder sb,
558590
final String codecClassName,
559591
final List<Token> fields,
@@ -674,7 +706,7 @@ private String nullableConvertedExpression(
674706
final String expression,
675707
final String nullValue)
676708
{
677-
return fieldToken.version() > 0 || fieldToken.isOptionalEncoding() ?
709+
return fieldToken.isOptionalEncoding() ?
678710
expression + " ?? " + nullValue :
679711
expression;
680712
}
@@ -791,9 +823,19 @@ private void generateVarDataEncodeInto(
791823
if (token.signal() == Signal.BEGIN_VAR_DATA)
792824
{
793825
final String propertyName = token.name();
826+
final String formattedPropertyName = formatPropertyName(propertyName);
827+
828+
if (token.version() > 0)
829+
{
830+
sb.append(indent).append("if (null == ").append(formattedPropertyName).append(")\n")
831+
.append(indent).append("{\n")
832+
.append(indent).append(INDENT).append("throw new System.InvalidOperationException(\"")
833+
.append(formattedPropertyName).append(" must not be null.\");\n")
834+
.append(indent).append("}\n\n");
835+
}
794836

795-
sb.append(indent).append("codec.Set").append(formatPropertyName(propertyName))
796-
.append("(").append(formatPropertyName(propertyName)).append(");\n");
837+
sb.append(indent).append("codec.Set").append(formattedPropertyName)
838+
.append("(").append(formattedPropertyName).append(");\n");
797839
}
798840
}
799841
}
@@ -1181,7 +1223,8 @@ private void generateVarData(
11811223
final String propertyName = token.name();
11821224
final Token varDataToken = Generators.findFirst("varData", tokens, i);
11831225
final String characterEncoding = varDataToken.encoding().characterEncoding();
1184-
final String dtoType = characterEncoding == null ? "byte[]" : "string";
1226+
final String nullableSuffix = token.version() > 0 ? "?" : "";
1227+
final String dtoType = (characterEncoding == null ? "byte[]" : "string") + nullableSuffix;
11851228

11861229
final String formattedPropertyName = formatPropertyName(propertyName);
11871230

‎sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/CSharpDtosPropertyTest.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,17 @@ void dtoEncodeShouldBeTheInverseOfDtoDecode(
8989
"SCHEMA:\n" + encodedMessage.schema());
9090
}
9191

92+
final byte[] errorBytes = Files.readAllBytes(stderr);
93+
if (errorBytes.length != 0)
94+
{
95+
throw new AssertionError(
96+
"Process wrote to stderr.\n\n" +
97+
"STDOUT:\n" + new String(Files.readAllBytes(stdout)) + "\n\n" +
98+
"STDERR:\n" + new String(errorBytes) + "\n\n" +
99+
"SCHEMA:\n" + encodedMessage.schema() + "\n\n"
100+
);
101+
}
102+
92103
final byte[] inputBytes = new byte[encodedMessage.length()];
93104
encodedMessage.buffer().getBytes(0, inputBytes);
94105
final byte[] outputBytes = Files.readAllBytes(tempDir.resolve("output.dat"));

0 commit comments

Comments
 (0)