Add upgrader 3.1 and support for upgrader chains

[adecler]

NOTE: Depends on

BHoM/BHoM_Engine#1477

Issues addressed by this PR

Closes #20

Test files

• for objects:
  https://burohappold.sharepoint.com/:u:/r/sites/BHoM/02_Current/12_Scripts/01_Test%20Scripts/Versioning_Toolkit/Issue21_BackwardCompatibilityTest.gh?csf=1&e=FuoX56
• for methods:
  https://burohappold.sharepoint.com/:u:/r/sites/BHoM/02_Current/12_Scripts/01_Test%20Scripts/Versioning_Toolkit/Issue7_MethodUpgradeTest_ForAdapters.gh?csf=1&e=mMnHaH

Same file as before and should give the same result. The difference is that the upgrades done for this files are now one level down the upgrader chain.

What you can do as a test is use the PR from the Engine (that correctly gets the assembly file version) and see that nothing gets upgraded properly. Then switch to this branch onV_TK and see that it works again.

The other test will obviously be when @IsakNaslundBh and @kThorsager start adding they versioning for this quarter.

Here's the result that you should get:

[kThorsager]

Tested this and will begin by saying that the provided file does not look like picture above.
This it the file I got (the red Push/Pull works for non-socket adapters):

So for a master, master configuration it worked for all the adapter methods in Rhino 6 except execute, but it is not among the methods in the dictionary so expected?
It did however not work for Rhino 5, which might be problematic since we are currently having some conversion problems in rhino 6

For PRbranch, master it did not work, which was expected.

But for PRbranch PRBranch it did not work either...
Will try to figure out whats going on there.

[adecler]

Sorry @kThorsager , I have updated the link to the test file

[kThorsager]

No worries
In that file the objects are converted nicely to their new type, but unexpectedly well. It works even when I'm only on the BHoM_Engine branch.
And it does not contain any tests for method upgrades

[adecler]

@kThorsager , let me start over. I rushed the PR just before a meeting yesterday and should have taken a bit more time to test and communicate.

I have now provided links to two test files in the top comment. One for objects, one for methods. Both should still open correctly. With a few caveats:

• The file testing the objects, should open as in the picture on the top comment. The two orange components and the red one are expected behaviour and testing that versioning fails properly when upgrades are not provided.
• The file testing the adapter methods should upgrade fine except for the Execute methods that doesn't look like it was provided an upgrade for. @alelom, can you check on that ? I have added you as a reviewer as I should have from the start.

However, I always recommend you run your own tests based on your own usage of what is provided in a PR. In this case, I added you because you are planning on using the versioning toolkit yourself so I wanted to make sure this PR was providing what you need (i.e. the upgrader 3.1) without breaking anything else. You are obviously welcome to test from any angle you see fit.

Now, for the two things you have highlighted already:

• You are saying that everything is upgraded nicely even when you are using only the Engine PR ? This means that the 3.0 upgrader is still used somehow since the upgrades necessary are only available there. So either you were still somehow on the new version of the versioning toolkit (or didn't recompiled BHoM_UI) when you tested or your current version of the BHoM is still 3.0 (but I would find that unlikely unless you haven't updated the UI repos for a while). Can you try again with that in mind ? (see useful tip below for a better way to check what is going on)
• The first test that you did on the adapter methods was failing massively and was doing the same for me. I have found the source of the problem and added a commit here that should fix this. So if you try again, everything should work except the Execute component.

Useful tip: if you comment those two lines in the GetPipe() method of the BHoM_Engine\Versioning_Engine\Convert\ToNewVersion.cs, you will have a command window poping up for each upgrader with their name in the window title. You can then also attach a debugger to them to see better what is going on.

[kThorsager]

Great, I got it to work properly now,
My issue was that once BHoM_Upgrader31 has been build it stays in Versioning_Toolkit\Build\bin\ as nothing overwrites it in the old branch. So despite clearing the appdata folder, BHoM_UI put it back when built. (and I was a bit unsure of where exactly it went in there so I did not find it yesterday)

guess this will only be a problem for people going backwards, but slightly annoying while testing.

But I will continue doing tests for my specific cases

[kThorsager]

So what I'm trying to do is changing Geometry.IElement to Dimensional.IElement (and the other similar interfaces)

The does't seem to be any framework in Versioning_toolkit for updating a interface.
Now specifically updating a type component to the new type component is not strictly necessary (but would be nice).
The biggest issue is that every method with one of these interfaces as input will no longer be found.

So while trying to fix that I notice that when loading a document with components not linked to methods, this part:

has _t = DBNull and no System.Reflection.MethodBase are pulled trough. Unsure if the methods are sent in as DBNull or just not sent in. (or maybe I have misunderstood this)
EDIT: I have now after a unclear amount of switching back and forth between different BHoM versions and adding different versions of the same component, make it pick up on the components added in the newest version which otherwise would not be linked to a method, it is however not picking up a lot of other components which are not linked to methods, and I have as yet been unable to make it convert in the right direction. But I will see if I can formalise what the issue is tomorrow.

A bug:
if someone specifies a method here:

without the parameter input for a overloaded method, then grasshopper will buffer forever(? a long time at least).
Also potentially problematic as this will lead to keeping old BHoM types somewhere to be able to be referenced here (i.e. when we want to upgrade into something deprecated in the chain after the type has been removed it won't work, but perhaps it won't matter by then?)

I'm currently writing a component which create all the lines of code for the dictionary(to be copied over) but would like some confirmation on that this is how it should be formatted?
typeof(static class).GetMethod(methodName,arrayOfInputTypes)
Seems to work now but the input types will break if we continue updating (and it makes Versioning_Toolkit dependent on a lot of things)

[adecler]

@kThorsager ,

The does't seem to be any framework in Versioning_toolkit for updating a interface.

You do it the same was as you do any class, you use Converter.ToNewType.

The biggest issue is that every method with one of these interfaces as input will no longer be found.

Correct, this is currently only supported for objects but not methods. So the only way right now is to provide an update description for each method. This is way beyond the scope of this PR though so I would raise a separate issue for that.

when we want to upgrade into something deprecated in the chain after the type has been removed it won't work

Very true. I was planning to set up the necessary NuGet packages for this next week so that each upgrader depends on its own version of the BHoM instead of the current one. Given how little time I have at the moment, I am starting to wonder if it wouldn't be better to simply store the dlls locally even it it means pushing dlls to GitHub. @al-fisher, what do you think ?

and it makes Versioning_Toolkit dependent on a lot of things

That is the point but keep in mind we only need the dlls that correspond to upgraded code during that quarter.

[al-fisher]

I am starting to wonder if it wouldn't be better to simply store the dlls locally even it it means pushing dlls to GitHub. @al-fisher, what do you think ?

Yes. Think this is fine for now.
With plan to migrate to NuGets to scale in medium long term

[kThorsager]

@adecler

The does't seem to be any framework in Versioning_toolkit for updating a interface.

You do it the same was as you do any class, you use Converter.ToNewType.

Sorry, I should have been more clear:

This is what I meant. And it does work for every other part of it.

A small thing, but still

[kThorsager]

I have tested through this properly now and most of my concerns have been show to not necessarily be about this PR and I have raised separate issues for them. (Except for comment above about converting Type components.)

So LGTM

[adecler]

Thanks @kThorsager .
I have raised a few more issues to better highlight the aspects of the versioning toolkit that still need immediate work. This toolkit is still very much in its infancy with features pretty much created on a need-to-have basis. I however plan to make sure you have everything you need by the end of next week at the latest. So don't hesitate to raise further issues if you see missing features that are preventing you from doing your job on the Geometry to Dimensional refactoring.