Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// Copyright 2004-2021 Castle Project - http://www.castleproject.org/
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

namespace Castle.DynamicProxy.Tests
{
using System;
using System.Reflection;

using NUnit.Framework;

using INestedSharedNameFromA = Interfaces.OuterWrapper.InnerWrapperA.ISharedName;
using INestedSharedNameFromB = Interfaces.OuterWrapper.InnerWrapperB.ISharedName;
using INestedSharedNameFromC = Interfaces.OuterWrapper.InnerWrapperC.ISharedName;

[TestFixture]
public class ExplicitlyImplementedNestedMethodNamesTestCase
{
[Test]
public void DynamicProxy_includes_namespace_and_declaring_type_and_type_name_in_names_of_explicitly_implemented_methods()
{
var a = typeof(INestedSharedNameFromA);
var b = typeof(INestedSharedNameFromB);
var c = typeof(INestedSharedNameFromC);

var proxy = new ProxyGenerator().CreateInterfaceProxyWithoutTarget(
interfaceToProxy: a,
additionalInterfacesToProxy: new[] { b, c },
interceptors: new StandardInterceptor());

var implementingType = proxy.GetType();

AssertNamingSchemeOfExplicitlyImplementedMethods(b, c, implementingType);
}

private void AssertNamingSchemeOfExplicitlyImplementedMethods(Type b, Type c, Type implementingType)
{
const BindingFlags bindingFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance;

// The assertions at the end of this method only make sense if certain preconditions
// are met. We verify those using NUnit assumptions:

// We require two interface types that have the same name and a method named `M` each:
Assume.That(b.IsInterface);
Assume.That(c.IsInterface);
Assume.That(b.Name == c.Name);
Assume.That(b.GetMethod("M") != null);
Assume.That(c.GetMethod("M") != null);

// We also need a type that implements the above interfaces:
Assume.That(b.IsAssignableFrom(implementingType));
Assume.That(c.IsAssignableFrom(implementingType));

// If all of the above conditions are met, we expect the methods from the interfaces
// to be implemented explicitly. For our purposes, this means that they follow the
// naming scheme `<namespace>.<parent types>.<type>.M`:
Assert.NotNull(implementingType.GetMethod($"{b.Namespace}.{b.DeclaringType.DeclaringType.Name}.{b.DeclaringType.Name}.{b.Name}.M", bindingFlags));
Assert.NotNull(implementingType.GetMethod($"{c.Namespace}.{b.DeclaringType.DeclaringType.Name}.{b.DeclaringType.Name}.{c.Name}.M", bindingFlags));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright 2004-2021 Castle Project - http://www.castleproject.org/
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

namespace Castle.DynamicProxy.Tests.Interfaces
{
public static class OuterWrapper
{
public static class InnerWrapperA
{
public interface ISharedName
{
void M();
}
}

public static class InnerWrapperB
{
public interface ISharedName
{
void M();
}
}

public static class InnerWrapperC
{
public interface ISharedName
{
void M();
}
}
}
}
40 changes: 36 additions & 4 deletions src/Castle.Core/DynamicProxy/Generators/MetaTypeElement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,23 +62,30 @@ protected void SwitchToExplicitImplementationName()
nameBuilder.Append(ns);
nameBuilder.Append('.');
}
AppendTypeName(nameBuilder, sourceType);
AppendTypeName(nameBuilder, sourceType, true);
Copy link
Member

Choose a reason for hiding this comment

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

Why not simply call AppendFullTypeName directly instead of going through AppendTypeName using that new parameter?

nameBuilder.Append('.');
nameBuilder.Append(name);
this.name = nameBuilder.ToString();
}
else if (ns != null)
{
this.name = string.Concat(ns, ".", sourceType.Name, ".", name);
this.name = string.Concat(ns, ".", GetFullTypeName(sourceType), ".", name);
Copy link
Member

Choose a reason for hiding this comment

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

If I remember correctly, the pre-existing code in SwitchToExplicitImplementationName was written the way it is based on the assumption that string.Concat would be faster than instantiating a StringBuilder. The first if clause is therefore the slow path, while the other two are supposedly fast paths.

Your code change introduces StringBuilders in the fast paths, too, so from a performance perspective, they're now all the same and it seems likely that the distinction between three cases no longer needs to be made.

So if we go with your change, that whole method could perhaps be simplified to a single case where a StringBuilder is always instantiated at the start and there are various ifs for the presence/absence of a namespace, for the presence/absence of generic type arguments, etc.

If we go with the existing code's structure, you'd have to find a way of not relying on StringBuilder on the fast paths.

I think I'd favor simplifying the code by rewriting the if conditions to something more obvious, even if it means we no longer have the string.Concat fast paths.

}
else
{
this.name = string.Concat(sourceType.Name, ".", name);
this.name = string.Concat(GetFullTypeName(sourceType), ".", name);
}

static void AppendTypeName(StringBuilder nameBuilder, Type type)
static void AppendTypeName(StringBuilder nameBuilder, Type type, bool fullName = false)
{
if (fullName)
{
AppendFullTypeName(nameBuilder, type);
return;
}

nameBuilder.Append(type.Name);

if (type.IsGenericType)
{
nameBuilder.Append('[');
Expand All @@ -94,6 +101,31 @@ static void AppendTypeName(StringBuilder nameBuilder, Type type)
nameBuilder.Append(']');
}
}

static string GetFullTypeName(Type type)
{
var nameBuilder = new StringBuilder();
AppendFullTypeName(nameBuilder, type);
return nameBuilder.ToString();
}

static void AppendFullTypeName(StringBuilder nameBuilder, Type type)
{
if (type.IsNested)
{
AppendFullTypeName(nameBuilder, type.DeclaringType);
nameBuilder.Append('.');
}

if (type.IsGenericType)
{
AppendTypeName(nameBuilder, type);
}
else
{
nameBuilder.Append(type.Name);
}
}
}
}
}