search

adamecr / Common.DMN.Engine

DMN Engine is a decision engine (rule engine) allowing to execute and evaluate the decisions defined in a DMN model. Its primary target is to evaluate the decision tables that transform the inputs into the output(s) using the decision rules.
MIT License
83 stars 24 forks source link

Made some property-getters within serializable classes public as othe… #1

Closed fgollas closed 4 years ago

fgollas commented 4 years ago

…rwise there was a deserialization issue (NullRefException) when integrating the nuget within a net48 project and deserializing a Model

adamecr commented 4 years ago

Hello @fgollas, these property-getters are intentionally private - the idea is to use them just by XML deserializer, but the parser access them using other properties with some logic:

I'll try the case "nuget package with net48 project" (the test project use the project reference, so there might be a bit different behavior) later on to check for the exception and the root cause. If positive (meaning erroneous), I'll have to think a bit how to "mask" these source XML props

adamecr commented 4 years ago

first quick look - nuget package + netcore21 project works fine - tests passed. However nuget package + net 472 failed as described. Will have a look on the deserialization. Frankly, I didn't work with "full" .net framework for a while :-)

Edit: it seems that in net core, the XmlSerializer may be valid for deserialization only - may not have public getters. For net framework the check is both for readable and writable properties.

I can imagine using IXmlSerializable for AllowedValues class, but I don't like the idea for the others, at these are quite complex.

I'll keep digging, not quite happy with making the props public

fgollas commented 4 years ago

@adamecr: Thank you for investigating.

adamecr commented 4 years ago

@fgollas : finally I decided to move forward with IXmlSerializable and proxy classes where needed. You can have a look in tst-pullreq1 branch - the first commit in branch (I have also added the net framework test project in the second commit and updated packages in the third one)

I'll do some more testing and publish the patch (merge to master + package) later on and then close this pull request.

adamecr commented 4 years ago

@fgollas - v0.1.1 released. The issues has been fixed using different approach that in this PR (proxy classes instead of "unhiding" the getters), so closing this PR

fgollas commented 4 years ago

thanks alot! Works now for me. Reintegrated your version 0.1.1