Issue #232 (HL7 2.5.1: Failure to parse ORDER group repetitions in OML_O33)

[ghost]

• Added unit test that exposes the failure to parse ORDER group repetitions.
• Added a ParserConfiguration class which holds the "non-greedy mode" setting.
• Updated MessageIterator to handle non-greedy parsing when using PipeParser.
• Updated PipeParser to hold ParserConfiguration.

[ghost]

@milkshakeuk Thanks for taking the time to review my pull request :)

[milkshakeuk]

@milkshakeuk Thanks for taking the time to review my pull request :)

@IanGralinski-Invetech no problem!

[milkshakeuk]

@IanGralinski-Invetech is there any way we can port these 2 greedy tests from hapi just for the extra coverage.

https://github.com/hapifhir/hapi-hl7v2/blob/3333e3aeae60afb7493f6570456e6280c0e16c0b/hapi-test/src/test/java/ca/uhn/hl7v2/parser/NewPipeParserTest.java#L158

https://github.com/hapifhir/hapi-hl7v2/blob/3333e3aeae60afb7493f6570456e6280c0e16c0b/hapi-test/src/test/java/ca/uhn/hl7v2/parser/NewPipeParserTest.java#L217

If not no pressure I can give it ago.

[github-actions[bot]]

Unit Test Results

       5 files     105 suites   16s :stopwatch:    989 tests    983 :heavy_check_mark:   6 :zzz: 0 :x: 1 901 runs  1 890 :heavy_check_mark: 11 :zzz: 0 :x:

Results for commit aee3cfe3.

[ghost]

@IanGralinski-Invetech is there any way we can port these 2 greedy tests from hapi just for the extra coverage.

https://github.com/hapifhir/hapi-hl7v2/blob/3333e3aeae60afb7493f6570456e6280c0e16c0b/hapi-test/src/test/java/ca/uhn/hl7v2/parser/NewPipeParserTest.java#L158

https://github.com/hapifhir/hapi-hl7v2/blob/3333e3aeae60afb7493f6570456e6280c0e16c0b/hapi-test/src/test/java/ca/uhn/hl7v2/parser/NewPipeParserTest.java#L217

If not no pressure I can give it ago.

I can give it a go, though it might still be a day or two until I can jump back on this.

[milkshakeuk]

@IanGralinski-Invetech I was thinking (as you do before you fall to sleep) and I think we should change the name of ParserConfiguration to Hl7ParserOptions or maybe event just ParserOptions ... as this seems to follow the modern dotnet style, plus if we ever build any convenient service registration for use with IServiceCollection it would fit with that pattern of having an options delegate for things like globally settings the parser options.

e.g.

services.AddControllers()
    .AddJsonOptions(options => options.JsonSerializerOptions.PropertyNamingPolicy = new SnakeCaseNamingPolicy());

or

builder.Services.Configure<JsonOptions>(opt =>
{
    opt.SerializerOptions.PropertyNamingPolicy = new SnakeCaseNamingPolicy());
});

What do you think? Happy to discuss.

[ghost]

@milkshakeuk I have made an attempt at porting the 2 greedy tests from hapi. The first was relatively straightforward to translate to C#, while the second I'm not 100% sure that I've translated the intent of the test. Does it look correct to you, or have I missed something?

I have also renamed ParserConfiguration to ParserOptions as you suggested :)

[github-actions[bot]]

Unit Test Results

       5 files     105 suites   16s :stopwatch:    991 tests    985 :heavy_check_mark:   6 :zzz: 0 :x: 1 905 runs  1 894 :heavy_check_mark: 11 :zzz: 0 :x:

Results for commit ecbf65b5.

[github-actions[bot]]

Unit Test Results

       5 files     105 suites   16s :stopwatch:    991 tests    985 :heavy_check_mark:   6 :zzz: 0 :x: 1 905 runs  1 894 :heavy_check_mark: 11 :zzz: 0 :x:

Results for commit 2b04bcb0.

[milkshakeuk]

@milkshakeuk I have made an attempt at porting the 2 greedy tests from hapi. The first was relatively straightforward to translate to C#, while the second I'm not 100% sure that I've translated the intent of the test. Does it look correct to you, or have I missed something?

@IanGralinski-Invetech I don't think it loses its intent, I suspect the empty if blocks in the hapi tests might be just there for when they get example messages for other message types greedy is applicable.

[github-actions[bot]]

Unit Test Results

       5 files     105 suites   16s :stopwatch:    991 tests    985 :heavy_check_mark:   6 :zzz: 0 :x: 1 905 runs  1 894 :heavy_check_mark: 11 :zzz: 0 :x:

Results for commit a2473794.

[milkshakeuk]

@PhantomGrazzler I have updated the wiki with details on all the ParserOptions.