Skip to content

Fix cross framework incompatibility for System.Drawing.Color #208

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Apr 13, 2021
Merged

Fix cross framework incompatibility for System.Drawing.Color #208

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4f9a643
Fix cross framework problem with System.Drawing
Arkatufus Mar 25, 2021
228aea6
Merge PR #206 test case
Arkatufus Mar 25, 2021
92bbaa8
Generalize type package name transformation for cross framework compa…
Arkatufus Mar 26, 2021
821133e
Define compiler directives that are missing in CI/CD
Arkatufus Mar 26, 2021
91a4d8f
Change class based configuration to lambda function
Arkatufus Apr 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/Hyperion.Tests/Bugs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Collections;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Drawing;
using System.IO;
using System.Linq;
using System.Reflection;
Expand All @@ -24,7 +25,7 @@
namespace Hyperion.Tests
{

public class Bugs
public class Bugs : TestBase
{
private readonly ITestOutputHelper _output;

Expand Down Expand Up @@ -218,6 +219,19 @@ public void CanSerializeUri()
Assert.Equal(stream.Length, stream.Position);
}

#region Issue 117

[Fact]
public void CanSerializeColor()
{
var expected = Color.Aquamarine;
Serialize(expected);
Reset();
var actual = Deserialize<Color>();
Assert.Equal(expected, actual);
}

#endregion

public class SnapshotSelectionCriteria
{
Expand Down
18 changes: 16 additions & 2 deletions src/Hyperion.Tests/CrossFrameworkSerializationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using System.Collections;
using System;
using System.Collections;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using FluentAssertions;
using Hyperion.Tests.Generator;
using Xunit;
Expand All @@ -19,9 +21,21 @@ public class CrossFrameworkSerializationTests
public CrossFrameworkSerializationTests(ITestOutputHelper log)
{
_log = log;
_serializer = new Serializer();
_originalObject = CrossFrameworkInitializer.Init();
_originalMixedObject = CrossFrameworkInitializer.InitMixed();

// Demonstrating the use of custom dll package name override
// to convert netcore System.Drawing.Primitives to netfx
// System.Drawing package.
#if NETFX
_serializer = new Serializer(new SerializerOptions(
packageNameOverrides: new List<CrossPlatformPackageNameOverride>
{
new CrossPlatformPackageNameOverride("System.Drawing.Primitives", ".Primitives", "")
Copy link
Member

Choose a reason for hiding this comment

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

So how does this work exactly @Arkatufus ? What's the formula for substituting names in one package for another?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first string is the "fingerprint", the unique string that indicates the wrong package name. It is matched using string.Contain.
The second string is the string pattern that needs to be replaced.
The third is the string to replace it with.

In this example, "if the package name contains the string System.Drawing.Primitives, replace .Primitives with an empty string.

Copy link
Member

Choose a reason for hiding this comment

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

How could we make that more intuitive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know, any ideas?

Copy link
Member

Choose a reason for hiding this comment

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

Think passing in a Func<string,string> predicate would make more sense? Leave it up to the user how to transform the name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I'll make the changes

Copy link
Contributor Author

@Arkatufus Arkatufus Apr 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

oh... I just remembered... we can't do that, it has to be a set of strings, because this will have to be configurable from hocon

Copy link
Member

Choose a reason for hiding this comment

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

We can always pass in a Setup for this for Hyperion just like we did for Newtonsoft.Json .

Copy link
Member

Choose a reason for hiding this comment

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

And you can add a layer that translates HOCON into these delegates too but for this level of customization passing in an array of typed functions is probably better.

}));
#elif NETCOREAPP
_serializer = new Serializer();
#endif
}

public static IEnumerable<object[]> SerializationFiles()
Expand Down
11 changes: 11 additions & 0 deletions src/Hyperion.Tests/Generator/CrossFrameworkClass.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System;
using System.Collections.Generic;
using System.Drawing;

namespace Hyperion.Tests.Generator
{
Expand Down Expand Up @@ -144,6 +146,15 @@ public class CrossFrameworkMixedClass : CrossFrameworkBase
public Type FriendType { get; set; }
public CrossFrameworkClass Data { get; set; }

// Test case for (netcore) System.Drawing.Primitives to (net45) System.Drawing
public Color Color { get; set; }
public Point Point { get; set; }
public PointF PointF { get; set; }
public Rectangle Rectangle { get; set; }
public RectangleF RectangleF { get; set; }
public Size Size { get; set; }
public SizeF SizeF { get; set; }

public override bool Equals(object obj)
{
if (!(obj is CrossFrameworkMixedClass other))
Expand Down
9 changes: 9 additions & 0 deletions src/Hyperion.Tests/Generator/CrossFrameworkInitializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System;
using System.Collections.Generic;
using System.Drawing;

namespace Hyperion.Tests.Generator
{
Expand All @@ -13,6 +15,13 @@ public static CrossFrameworkMixedClass InitMixed()
Name = "Cookie",
Sound = "Bark",
FriendType = typeof(CrossFrameworkClass),
Color = Color.Blue,
Point = new Point(10, 10),
PointF = new PointF(10, 10),
Rectangle = new Rectangle(10, 10, 10, 10),
RectangleF = new RectangleF(10, 10, 10, 10),
Size = new Size(10, 10),
SizeF = new SizeF(10, 10),
Data = Init()
};
}
Expand Down
13 changes: 13 additions & 0 deletions src/Hyperion.Tests/Hyperion.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@
<StartupObject>Hyperion.Tests.Generator.Program</StartupObject>
</PropertyGroup>

<PropertyGroup Condition=" '$(TargetFramework)' == 'netcoreapp3.1' or '$(TargetFramework)' == 'net5.0' ">
<DefineConstants>$(DefineConstants);NETCOREAPP</DefineConstants>
</PropertyGroup>

<PropertyGroup Condition=" '$(TargetFramework)' == 'net461' ">
<DefineConstants>$(DefineConstants);NETFX</DefineConstants>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="FluentAssertions" Version="5.10.3" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(TestSdkVersion)" />
Expand All @@ -22,4 +30,9 @@
<ProjectReference Include="..\Hyperion\Hyperion.csproj" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'net461'">
<Reference Include="System.Drawing">
<Private>true</Private>
</Reference>
</ItemGroup>
</Project>
Binary file modified src/Hyperion.Tests/testfiles/mixed_test_file_.netcoreappversionv2.0.tf
View file
Open in desktop
Binary file not shown.
Binary file modified src/Hyperion.Tests/testfiles/mixed_test_file_.netcoreappversionv2.1.tf
View file
Open in desktop
Binary file not shown.
Binary file modified src/Hyperion.Tests/testfiles/mixed_test_file_.netcoreappversionv2.2.tf
View file
Open in desktop
Binary file not shown.
Binary file modified src/Hyperion.Tests/testfiles/mixed_test_file_.netcoreappversionv3.0.tf
View file
Open in desktop
Binary file not shown.
Binary file modified src/Hyperion.Tests/testfiles/mixed_test_file_.netcoreappversionv3.1.tf
View file
Open in desktop
Binary file not shown.
Binary file modified src/Hyperion.Tests/testfiles/mixed_test_file_.netcoreappversionv5.0.tf
View file
Open in desktop
Binary file not shown.
Binary file modified src/Hyperion.Tests/testfiles/mixed_test_file_.netframeworkversionv4.6.1.tf
View file
Open in desktop
Binary file not shown.
18 changes: 8 additions & 10 deletions src/Hyperion/Extensions/TypeEx.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,18 +118,16 @@ 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
if (shortName.Contains("System.Private.CoreLib,%core%"))
{
shortName = shortName.Replace("System.Private.CoreLib,%core%", "mscorlib,%core%");
}
#endif
#if NETSTANDARD
if (shortName.Contains("mscorlib,%core%"))
var overrides = session.Serializer.Options.CrossFrameworkPackageNameOverrides;

foreach (var value in overrides)
{
shortName = shortName.Replace("mscorlib,%core%", "System.Private.CoreLib,%core%");
if (shortName.Contains(value.Fingerprint))
{
shortName = shortName.Replace(value.RenameFrom, value.RenameTo);
}
}
#endif

return LoadTypeByName(shortName);
});
}
Expand Down
38 changes: 37 additions & 1 deletion src/Hyperion/SerializerOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,24 @@ namespace Hyperion
{
public class SerializerOptions
{
internal static List<CrossPlatformPackageNameOverride> DefaultPackageNameOverrides()
{
return new List<CrossPlatformPackageNameOverride>
{
#if NET45
new CrossPlatformPackageNameOverride(
fingerprint: "System.Private.CoreLib,%core%",
from: "System.Private.CoreLib,%core%",
to: "mscorlib,%core%")
#elif NETSTANDARD
new CrossPlatformPackageNameOverride(
fingerprint: "mscorlib,%core%",
from: "mscorlib,%core%",
to: "System.Private.CoreLib,%core%")
#endif
};
}

internal static readonly Surrogate[] EmptySurrogates = new Surrogate[0];


Expand Down Expand Up @@ -53,8 +71,10 @@ public class SerializerOptions
internal readonly bool VersionTolerance;
internal readonly Type[] KnownTypes;
internal readonly Dictionary<Type, ushort> KnownTypesDict = new Dictionary<Type, ushort>();
internal readonly List<CrossPlatformPackageNameOverride> CrossFrameworkPackageNameOverrides =
DefaultPackageNameOverrides();

public SerializerOptions(bool versionTolerance = false, bool preserveObjectReferences = false, IEnumerable<Surrogate> surrogates = null, IEnumerable<ValueSerializerFactory> serializerFactories = null, IEnumerable<Type> knownTypes = null, bool ignoreISerializable = false)
public SerializerOptions(bool versionTolerance = false, bool preserveObjectReferences = false, IEnumerable<Surrogate> surrogates = null, IEnumerable<ValueSerializerFactory> serializerFactories = null, IEnumerable<Type> knownTypes = null, bool ignoreISerializable = false, IEnumerable<CrossPlatformPackageNameOverride> packageNameOverrides = null)
{
VersionTolerance = versionTolerance;
Surrogates = surrogates?.ToArray() ?? EmptySurrogates;
Expand All @@ -72,6 +92,22 @@ public SerializerOptions(bool versionTolerance = false, bool preserveObjectRefer

PreserveObjectReferences = preserveObjectReferences;
IgnoreISerializable = ignoreISerializable;

if(packageNameOverrides != null)
CrossFrameworkPackageNameOverrides.AddRange(packageNameOverrides);
}
}
public class CrossPlatformPackageNameOverride
{
public CrossPlatformPackageNameOverride(string fingerprint, string @from, string to)
{
Fingerprint = fingerprint;
RenameFrom = @from;
RenameTo = to;
}

public string Fingerprint { get; }
public string RenameFrom { get; }
public string RenameTo { get; }
}
}
16 changes: 10 additions & 6 deletions src/common.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@
<PropertyGroup>
<Copyright>Copyright © 2016-2017 Akka.NET Team</Copyright>
<Authors>Akka.NET Team</Authors>
<VersionPrefix>0.9.16</VersionPrefix>
<PackageReleaseNotes>[Bump Microsoft.NET.Test.Sdk from 16.5.0 to 16.6.1](https://github.com/akkadotnet/Hyperion/pull/174)
[Add deserialization support for ReadOnlyDictionary](https://github.com/akkadotnet/Hyperion/pull/177)
[Bump FluentAssertions from 5.10.2 to 5.10.3](https://github.com/akkadotnet/Hyperion/pull/171)
[Bump System.Collections.Immutable from 1.7.0 to 1.7.1](https://github.com/akkadotnet/Hyperion/pull/175)
[Bump BenchmarkDotNet from 0.12.0 to 0.12.1](https://github.com/akkadotnet/Hyperion/pull/172)</PackageReleaseNotes>
<VersionPrefix>0.9.17</VersionPrefix>
<PackageReleaseNotes>[Bump Microsoft.NET.Test.Sdk from 16.6.1 to 16.7.1](https://github.com/akkadotnet/Hyperion/pull/182)
[Fix unit test problem](https://github.com/akkadotnet/Hyperion/pull/191)
[Bump FSharp.Core from 4.7.2 to 5.0.0](https://github.com/akkadotnet/Hyperion/pull/189)
[Fix issue #40 regarding partial streams](https://github.com/akkadotnet/Hyperion/pull/185)
[Fix Hyperion not using known serializers when defined](https://github.com/akkadotnet/Hyperion/pull/184)
[Bump Microsoft.NET.Test.Sdk from 16.7.1 to 16.8.3](https://github.com/akkadotnet/Hyperion/pull/196)
[Bump System.Collections.Immutable from 1.7.1 to 5.0.0](https://github.com/akkadotnet/Hyperion/pull/195)
[Bump FSharp.Core from 5.0.0 to 5.0.1](https://github.com/akkadotnet/Hyperion/pull/202)
[Update the cross framework spec to include complex POCO object, Type serialization, and support for netcoreapp3.1 and net5.0](https://github.com/akkadotnet/Hyperion/pull/204)</PackageReleaseNotes>
<PackageIconUrl>http://getakka.net/images/akkalogo.png</PackageIconUrl>
<PackageProjectUrl>https://github.com/akkadotnet/Hyperion</PackageProjectUrl>
<PackageLicenseUrl>https://github.com/akkadotnet/Hyperion/blob/master/LICENSE</PackageLicenseUrl>
Expand Down