WIP Communicate between framework mscorlib and core framework with Unit Tests #116

Blind-Striker · 2019-05-28T12:11:42Z

In addition to the work done at #112, and according to the suggestion in #112 (comment), I have done followings:

I created a class called CrossFrameworkClass that contains almost all fundamental .NET types (int, long, enum, struct, DateTime, Exception, etc.). Then I initialized this class with dummy values and serialize and save it as files for all frameworks (net461, netcoreapp2.0, netcoreapp2.1, netcoreapp2.2). I included those files to Hyperion.Tests project.
In order to re-generate files in case we need again, I've added a CLI command to Hyperion.Tests project.
I created a test that read test files, deserializes it and then compares it to the class that initialized with dummy values.

Right now tests are working. In order to be sure, I reverted all of @humhei's changes and ran the tests again and observed that tests are not passed.

There is only one case where the tests are failing. When you serialize and deserialize exceptions that have extra property such as ArgumentNullException, these properties are being ignored. So instead of full compare, I just compared exception messages. I think this problem related to ExceptionSerializerFactory which a subject of another issue.

Except that I'm pretty sure everything is working.

…#112 (comment)

Blind-Striker · 2019-09-30T08:27:55Z

@Aaronontheweb, @Horusiath We really need these changes, right now we are using our own package that forked from this project. We need not only for Akka projects but also for other projects where we use Hyperion as a serializer.

See:
https://github.com/armutcom/CacheManager.Serialization.Hyperion

If there are things you want me to change at this Pull Request, or if you think there are things that don't meet the standards of the project, I can make these changes quickly.

@akselarzuman the person who opened the PR #119 is my collage. We love Hyperion as a company 😄 and we use it extensively, and we want to make contributions whenever it's possible.

Horusiath · 2019-10-01T22:54:55Z

src/Hyperion/Extensions/TypeEx.cs

@@ -125,6 +125,14 @@ private static Type GetTypeFromManifestName(Stream stream, DeserializerSession s
            return TypeNameLookup.GetOrAdd(byteArr, b =>
            {
                var shortName = StringEx.FromUtf8Bytes(b.Bytes, 0, b.Bytes.Length);
+                if (shortName.Contains("System.Private.CoreLib,%core%") && CoreAssemblyQualifiedName.Contains("mscorlib, Version="))


Isn't this if/else statement something that could be replaced by compilation flags (iike #if NETCORE)? I'm not sure if this check is something that we need to call at runtime for potentially every single type when we know that CoreAssemblyQualifiedName will never change in the application lifetime.

It isn't my commit but I'm not sure if we don't need runtime checks. Because the whole purpose of this PR is to provide interoperability between the .NET Framework and .NET Core, thus we might need runtime checks in case of serializing/deserializing between those two frameworks. As far as I understand Hyperion embeds the namespace into the serialized payload for types.

I'll check it and update the code if it's necessary. Thanks for pointing out.

@humhei fyi

if (shortName.Contains("System.Private.CoreLib,%core%") && CoreAssemblyQualifiedName.Contains("mscorlib, Version="))

The second predication can be replaced by etc #if NET461
But i more like define a global variable
private static readonly bool IsFullNetFramework = CoreAssemblyQualifiedName.Contains("mscorlib, Version="))
Then you can check it like
if (shortName.Contains("System.Private.CoreLib,%core%") && IsFullNetFramework

Blind-Striker#1

The reason here is that we want to avoid unnecessary branches and operations. While it's quite possible that .NET Core 3.0 knows how to optimize static readonly checks, it's wasteful to defer checks, which result is known at compile time, back to the runtime for every type call. It's only a microoptimization, but it's also a sane practice to do.

A middle ground here is to make IsFullNetFramework a const defined in #if/else pragmas and put it as a first segment of && expression - since consts cannot be changed by reflection (unlike readonly values), they are always optimized on every .NET platform.

Thanks for your explanation
Fixed In Blind-Striker#1
#if NETSTANDARD
#if NET45

Ok, everything seems to be in order. I merged @humhei 's PR and pull the latest CI improvements from the dev branch (@Danthar fyi). All tests are passed.

After @akselarzuman's PR #119 merged I need to update test files (mentioned in #116 (comment)) for .NET Core 3.0 and run the tests again.

FullFramework or CoreNetFramework runtime check

# Conflicts: # src/Hyperion/Hyperion.csproj

…with netstandard1.6

# Conflicts: # src/Hyperion.Tests/Hyperion.Tests.csproj

Blind-Striker · 2019-10-04T16:52:48Z

@Danthar conflict resolved

Blind-Striker · 2019-10-04T17:28:35Z

@Danthar I don't know why it failed, I have run build.ps1 on my local for all targets. The error message didn't specify anything :(

Danthar · 2019-10-04T20:08:12Z

Because of this:

Blind-Striker · 2019-10-04T21:09:06Z

@Danthar Would it be a problem if I change vmImage: 'vs2017-win2016' to vmImage: windows-2019. 2.2.7 is the latest version .NET Core 2.2. Either I have to explicitly set projects .NET Core SDK version to the previous version (which is not compatible with VS 2019) or I have to change Windows image.

# Conflicts: # src/Hyperion.Tests/Hyperion.Tests.csproj

Aaronontheweb · 2019-10-05T14:50:41Z

vmImage: windows-2019

@Danthar we should change this in our templates across all projects.

Update Windows vm image to windows-2019

Aaronontheweb

LGTM, looking at test output next to make sure what I've reviewed matches what is being tested...

Aaronontheweb · 2019-10-15T17:26:29Z

build-system/nightly-builds.yaml

@@ -2,7 +2,7 @@
 # See https://docs.microsoft.com/en-us/azure/devops/pipelines/yaml-schema for reference

 pool:
-  vmImage: vs2017-win2016
+  vmImage: windows-2019


Aaronontheweb · 2019-10-15T17:27:38Z

src/Hyperion.Tests/CrossFrameworkSerializationTests.cs

+        }
+
+        [Fact]
+        public void CanSerializeCrossFramework()


Nice, this the only way to do it - have to actually load the sucker and work the payloads on both ends.

Aaronontheweb · 2019-10-15T17:28:25Z

src/Hyperion.Tests/Generator/CrossFrameworkStruct.cs

+            }
+        }
+
+        private bool Equals(CrossFrameworkStruct other)


LGTM - validates that all of the primitives are covered.

Aaronontheweb · 2019-10-15T17:29:10Z

src/Hyperion.Tests/Hyperion.Tests.csproj

    <TargetFrameworks>net461;netcoreapp2.1;netcoreapp3.0</TargetFrameworks>
+    <TargetLatestRuntimePatch>true</TargetLatestRuntimePatch>
+    <LangVersion>latest</LangVersion>
+    <StartupObject>Hyperion.Tests.Generator.Program</StartupObject>


Fine with this - but I'll need to check the test output real fast to make sure everything is still there (pretty sure this PR would have exploded if it wasn't.)

Aaronontheweb · 2019-10-15T17:29:48Z

src/Hyperion/Extensions/TypeEx.cs

@@ -118,11 +118,23 @@ private static Type GetTypeFromManifestName(Stream stream, DeserializerSession s
            return TypeNameLookup.GetOrAdd(byteArr, b =>
            {
                var shortName = StringEx.FromUtf8Bytes(b.Bytes, 0, b.Bytes.Length);
+#if NET45


And this is where all of the heavy-lifting for working between frameworks occurs I take it...

Aaronontheweb · 2019-10-15T17:38:07Z

Tests look good.

Blind-Striker · 2019-10-15T20:09:28Z

Thanks @Aaronontheweb

I couldn't make the test project work with .NET Core 2.2. I think it's about Azure VM. I have got the error below.

The specified framework 'Microsoft.NETCore.App', version '2.2.7' was not found.

We can add the task below to install the latest .NET Core 2.2. But I didn't want to add that task without getting your opinion, and it also didn't feel like it was the right way to do this. Better wait for Microsoft to update their VM images.

  - task: UseDotNet@2
    displayName: 'Install .net core 2.2'
    inputs:
      packageType: sdk
      version: '2.2.402'
      installationPath: $(Agent.ToolsDirectory)/dotnet

But I'm pretty sure it's working because I have run the tests on my local and custom docker container with .NET Core 2.2.

Aaronontheweb · 2019-10-15T20:55:46Z

@Blind-Striker go ahead and add it.... Can we do something similar for the Linux images as well so we could get the test suite to run on that too?

Blind-Striker · 2019-10-15T21:19:13Z

@Aaronontheweb We can actually. I already did something similar in my projects before. We can use a matrix strategy

strategy:
  matrix:
    linux:
      imageName: 'ubuntu-latest'
    mac:
      imageName: 'macos-10.13'
    windows:
      imageName: 'windows-2019'

The only thing we need to do is add a condition to the task that checks the OS and run build.ps1 or build.sh according to that. Something like below

  - task: PowerShell@2
    displayName: "Compile & Tests"
    inputs:
      targetType: filePath
      filePath: ./build.ps1
    condition: eq( variables['Agent.OS'], 'Windows_NT' )

  - task: Bash@3
    displayName: 'Compile & Tests'
    inputs:
      targetType: filePath
      filePath: ./build.sh
    condition: or(eq( variables['Agent.OS'], 'Darwin' ), eq( variables['Agent.OS'], 'Linux' ))

The only drawback (it's not a drawback actually) of matrix strategy is you can't get separate build badges for each OS. So I always prefer to create separate YAML files per OS. But it's an aesthetic issue, not a technical one :)

I'll do the necessary changes and open a new PR.

Aaronontheweb · 2019-10-16T19:07:13Z

@Blind-Striker thank you! We would really appreciate that.

humhei · 2019-11-28T08:27:07Z

src/Hyperion/Hyperion.csproj

@@ -20,11 +20,7 @@
    <Reference Include="Microsoft.CSharp" />
  </ItemGroup>



#145

$(DefineConstants);NETSTANDARD

Maybe it is because The NETSTANDARD constrant is deleted here

Cc @Blind-Striker

The tests should have failed on this case, I'll inform you after check it @DaniilSokolyuk @Aaronontheweb @humhei

Looks like the issue is specific to .NET Core 3+

Based on what I've found with #151 thus far

humhei and others added 5 commits April 20, 2019 17:26

Communicate between mscorlib and System.Private.CoreLib

c0c1ad1

Strict string match

217aa61

Add a binary file generator for all .NET frameworks

d82a50a

Add generated serialized files to project as suggestted in akkadotnet…

b688a73

…#112 (comment)

Add unit tests to test cross framework serialization and deserialization

cd68375

Horusiath reviewed Oct 1, 2019

View reviewed changes

humhei and others added 6 commits October 4, 2019 08:48

FullFramework or CoreNetFramework runtime check

98bc2b2

Using Constants instead

7653d1a

Merge pull request #1 from humhei/Communication

72a878a

FullFramework or CoreNetFramework runtime check

Merge branch 'dev' of github.com:akkadotnet/Hyperion into Communication

9616faf

# Conflicts: # src/Hyperion/Hyperion.csproj

Downgrade FSharp.Core 4.7.0 to 4.6.2 because 4.7.0 is not compatible …

bcff54b

…with netstandard1.6

Merge branch 'dev' of github.com:akkadotnet/Hyperion into Communication

7b2b127

# Conflicts: # src/Hyperion.Tests/Hyperion.Tests.csproj

Add generated serialized file for .NET Core 3.0 to project

e5cfd40

Blind-Striker force-pushed the Communication branch from 52772e8 to e5cfd40 Compare October 4, 2019 17:09

Blind-Striker added 2 commits October 5, 2019 09:54

Merge branch 'dev' of github.com:akkadotnet/Hyperion into Communication

5aa54b2

# Conflicts: # src/Hyperion.Tests/Hyperion.Tests.csproj

Merge branch 'dev' of github.com:akkadotnet/Hyperion into Communication

360b0a5

Blind-Striker added 2 commits October 6, 2019 16:13

Add netcoreapp2.2 support to Hyperion.Tests

f3c3c05

Update Windows vm image to windows-2019

Remove netcoreapp2.2 from Hyperion.Tests

305885c

Blind-Striker requested a review from Horusiath October 6, 2019 14:54

Aaronontheweb approved these changes Oct 15, 2019

View reviewed changes

Aaronontheweb merged commit 034ba5c into akkadotnet:dev Oct 15, 2019

This was referenced Oct 15, 2019

WIP Communicate between framework mscorlib and core framework System.Private.CoreLib #112

Closed

Cannot deserialize types when simplified qualified name is not resolvable by .NET framework #117

Closed

This was referenced Oct 15, 2019

Serialization between dotnet core clusternode and framework clusternode does not work properly #108

Closed

added v0.9.10 Release Notes #134

Merged

Blind-Striker deleted the Communication branch October 15, 2019 20:17

Aaronontheweb mentioned this pull request Oct 16, 2019

v0.9.10 Release #136

Merged

DaniilSokolyuk mentioned this pull request Nov 19, 2019

Communicate between framework mscorlib and core still not working correctly #145

Open

humhei reviewed Nov 28, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP Communicate between framework mscorlib and core framework with Unit Tests #116

WIP Communicate between framework mscorlib and core framework with Unit Tests #116

Blind-Striker commented May 28, 2019 •

edited

Loading

Blind-Striker commented Sep 30, 2019

Horusiath Oct 1, 2019

Blind-Striker Oct 3, 2019 •

edited

Loading

humhei Oct 4, 2019 •

edited

Loading

Horusiath Oct 4, 2019

humhei Oct 4, 2019 •

edited

Loading

Blind-Striker Oct 4, 2019 •

edited

Loading

Blind-Striker commented Oct 4, 2019

Blind-Striker commented Oct 4, 2019

Danthar commented Oct 4, 2019

Blind-Striker commented Oct 4, 2019 •

edited

Loading

Aaronontheweb commented Oct 5, 2019

Aaronontheweb left a comment

Aaronontheweb Oct 15, 2019

Aaronontheweb Oct 15, 2019

Aaronontheweb Oct 15, 2019

Aaronontheweb Oct 15, 2019

Aaronontheweb Oct 15, 2019

Aaronontheweb commented Oct 15, 2019

Blind-Striker commented Oct 15, 2019

Aaronontheweb commented Oct 15, 2019

Blind-Striker commented Oct 15, 2019

Aaronontheweb commented Oct 16, 2019

humhei Nov 28, 2019

Aaronontheweb Nov 28, 2019

Blind-Striker Dec 11, 2019

Aaronontheweb Jan 20, 2020

Aaronontheweb Jan 20, 2020

		@@ -20,11 +20,7 @@
		<Reference Include="Microsoft.CSharp" />
		</ItemGroup>

WIP Communicate between framework mscorlib and core framework with Unit Tests #116

WIP Communicate between framework mscorlib and core framework with Unit Tests #116

Conversation

Blind-Striker commented May 28, 2019 • edited Loading

Blind-Striker commented Sep 30, 2019

Choose a reason for hiding this comment

Blind-Striker Oct 3, 2019 • edited Loading

Choose a reason for hiding this comment

humhei Oct 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

humhei Oct 4, 2019 • edited Loading

Choose a reason for hiding this comment

Blind-Striker Oct 4, 2019 • edited Loading

Choose a reason for hiding this comment

Blind-Striker commented Oct 4, 2019

Blind-Striker commented Oct 4, 2019

Danthar commented Oct 4, 2019

Blind-Striker commented Oct 4, 2019 • edited Loading

Aaronontheweb commented Oct 5, 2019

Aaronontheweb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aaronontheweb commented Oct 15, 2019

Blind-Striker commented Oct 15, 2019

Aaronontheweb commented Oct 15, 2019

Blind-Striker commented Oct 15, 2019

Aaronontheweb commented Oct 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Blind-Striker commented May 28, 2019 •

edited

Loading

Blind-Striker Oct 3, 2019 •

edited

Loading

humhei Oct 4, 2019 •

edited

Loading

humhei Oct 4, 2019 •

edited

Loading

Blind-Striker Oct 4, 2019 •

edited

Loading

Blind-Striker commented Oct 4, 2019 •

edited

Loading