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

Serialiser_Engine: Auto-versioning of methods and types during serialisation #1762

Merged
merged 5 commits into from
May 21, 2020

Conversation

adecler
Copy link
Member

@adecler adecler commented May 13, 2020

Issues addressed by this PR

Closes #1738
Closes #1766

Methods and types are now upgraded directly during deserialisation, not need to call ToNewVersion separately.

Test files

Here's what I have tested on. But I recommend to use the usual serialisation test files on SharePoint:

  • Serialisation from Serialiser_Engine: Fix remaining problems with serialisation #1756 is still giving me a 100% success as before
  • I have tested various levels of inclusion of a type that needs upgrading and all seems fine.
    image
    Here's the json used in there (in order of complexity, raw type, Custom object containing the type as property, List of that type to cover generic types, method that needs that type to be updated for it to work ):
{ "_t" : "System.Type", "Name" : "BH.oM.Geometry.IElement2D" }
{"Type": { "_t" : "System.Type", "Name" : "BH.oM.Geometry.IElement2D" }, "A": 1}
{ "_t" : "System.Type", "Name" : "System.Collections.Generic.List`1, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089", "GenericArguments" : [{ "_t" : "System.Type", "Name" : "BH.oM.Geometry.IElement2D" }] }
{ "_t" : "System.Reflection.MethodBase", "TypeName" : "{ \"_t\" : \"System.Type\", \"Name\" : \"BH.Engine.ModelLaundry.Modify, ModelLaundry_Engine, Version=3.0.0.0, Culture=neutral, PublicKeyToken=null\" }", "MethodName" : "ExtendToEachOther", "Parameters" : ["{ \"_t\" : \"System.Type\", \"Name\" : \"System.Collections.Generic.List`1, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089\", \"GenericArguments\" : [{ \"_t\" : \"System.Type\", \"Name\" : \"BH.oM.Geometry.IElement2D\" }] }", "{ \"_t\" : \"System.Type\", \"Name\" : \"System.Double, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089\" }", "{ \"_t\" : \"System.Type\", \"Name\" : \"System.Double, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089\" }", "{ \"_t\" : \"System.Type\", \"Name\" : \"System.Double, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089\" }"] }
  • A method that needs to be upgraded through versioning. You probably want to do more testing on that one by using your own json or scripts that need upgrading:
{ "_t" : "System.Reflection.MethodBase", "TypeName" : "{ \"_t\" : \"System.Type\", \"Name\" : \"BH.Engine.Geometry.Compute, Geometry_Engine, Version=3.0.0.0, Culture=neutral, PublicKeyToken=null\" }", "MethodName" : "ClipPolylines", "Parameters" : ["{ \"_t\" : \"System.Type\", \"Name\" : \"BH.oM.Geometry.Polyline\" }", "{ \"_t\" : \"System.Type\", \"Name\" : \"BH.oM.Geometry.Polyline\" }"] }

I strongly recommend reviewing and merging this alongside BHoM/BHoM_UI#263 as that fixes the missing wires on upgrades.

@adecler adecler added the type:bug Error or unexpected behaviour label May 13, 2020
@adecler adecler added this to the BHoM 3.2 β MVP milestone May 13, 2020
@adecler adecler requested a review from FraserGreenroyd as a code owner May 13, 2020 09:38
@adecler adecler self-assigned this May 13, 2020
@adecler
Copy link
Member Author

adecler commented May 14, 2020

Now also takes care of issue #1766 .

@adecler adecler added type:feature New capability or enhancement and removed type:bug Error or unexpected behaviour labels May 14, 2020
@adecler adecler requested a review from alelom May 14, 2020 06:10
@adecler
Copy link
Member Author

adecler commented May 14, 2020

I strongly recommend reviewing and merging this alongside BHoM/BHoM_UI#263 as that fixes the missing wires on upgrades.

@adecler adecler force-pushed the Serialiser_Engine-#1737-SerialisationAutoVersioning branch from ba8ed19 to c7477f1 Compare May 20, 2020 08:26
Copy link
Contributor

@IsakNaslundBh IsakNaslundBh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have tested this together with BHoM/BHoM_UI#263 and it all seem to be running fine for me.

I have started to collect various version tests from different UIs and BHoM versions on sharepoint here: https://burohappold.sharepoint.com/:f:/s/BHoM/EopxQb5d9QdDqKoD--1178kBUpVHnKPZ1WWwyI2Eg1l18Q?e=hDsyDU

Good place to put more scripts for testing the versioning if/when it changes.

Tested through the GH scripts and most dynamo scripts, and all seem to be running with a few exceptions, that are somewhat expected.

Would be good though if one more person would be able to have a look at this before merging.

@peterjamesnugent can you test that your adapter versioning is working well for you.
@kThorsager I have probably tested through most of your stuff already, but always good to get a second pair of eyes.
@FraserGreenroyd if you have anything from BEnv that has been versioned, would you mind running those scripts as well.

Then all, if you have any script that has seen versioning, would you mind putting them in the linked sharepoint folder under the correct version, for ease of testing going forward?

Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran a couple of my scripts and all seemed to upgrade ok as I would have expected, though I don't have an abundance of versioning testing scripts.

@adecler
Copy link
Member Author

adecler commented May 21, 2020

/azp run BHoM_Engine.CheckInstaller

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@adecler adecler merged commit 3b6f6ba into master May 21, 2020
@adecler adecler deleted the Serialiser_Engine-#1737-SerialisationAutoVersioning branch May 21, 2020 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New capability or enhancement
Projects
None yet
3 participants