Skip to content

Commit

Permalink
[c# grpc] Allow [Bond.Attribute] on methods
Browse files Browse the repository at this point in the history
* This fixes broken C# codegen when a service method has attributes.
* Add both C++ and C# unit test to make sure that service and method
  attributes are properly generated and accessible.

Fixes #617
  • Loading branch information
chwarr authored Dec 8, 2017
1 parent 7b6dced commit 5249212
Show file tree
Hide file tree
Showing 13 changed files with 266 additions and 11 deletions.
11 changes: 10 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ different versioning scheme, following the Haskell community's
* IDL core version: TBD
* IDL comm version: TBD
* C++ version: (major bump needed)
* C# NuGet version: TBD
* C# NuGet version: (minor bump needed)
* C# Comm NuGet version: TBD

### `gbc` and Bond compiler library ###
Expand Down Expand Up @@ -49,6 +49,15 @@ different versioning scheme, following the Haskell community's
* Fixed includes for gRPC services with events or parameterless methods.
[Issue #735](https://github.com/Microsoft/bond/issues/735)

### C# ###

* The C# attribute `Bond.Attribute` can now be applied to methods. This
fixes broken codegen when attributes are used on service methods.
[Issue #617](https://github.com/Microsoft/bond/issues/617)
* Bond Attributes on service methods are now present on all the client
overloads for the methods. Previously, just the "friendly" method had the
attributes.

## 7.0.2: 2017-10-30 ##
* `gbc` & compiler library: 0.10.1.0
* IDL core version: 2.0
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/Language/Bond/Codegen/Cs/Grpc_cs.hs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ namespace #{csNamespace}
return #{methodName}Async(message, new global::Grpc.Core.CallOptions(headers, deadline, cancellationToken));
}

public virtual global::Grpc.Core.AsyncUnaryCall<global::Bond.Grpc.IMessage<#{getMessageTypeName methodResult}>> #{methodName}Async(global::Bond.Grpc.IMessage<#{getMessageTypeName methodInput}> request, global::Grpc.Core.CallOptions options)
#{CS.schemaAttributes 3 methodAttributes}public virtual global::Grpc.Core.AsyncUnaryCall<global::Bond.Grpc.IMessage<#{getMessageTypeName methodResult}>> #{methodName}Async(global::Bond.Grpc.IMessage<#{getMessageTypeName methodInput}> request, global::Grpc.Core.CallOptions options)
{
return CallInvoker.AsyncUnaryCall(Method_#{methodName}, null, options, request);
}|]
Expand All @@ -136,7 +136,7 @@ namespace #{csNamespace}
#{methodName}Async(message, new global::Grpc.Core.CallOptions(headers, deadline, cancellationToken));
}

public virtual void #{methodName}Async(global::Bond.Grpc.IMessage<#{getMessageTypeName methodInput}> request, global::Grpc.Core.CallOptions options)
#{CS.schemaAttributes 3 methodAttributes}public virtual void #{methodName}Async(global::Bond.Grpc.IMessage<#{getMessageTypeName methodInput}> request, global::Grpc.Core.CallOptions options)
{
global::Bond.Grpc.Internal.NothingCallInvoker.NothingCall(CallInvoker, Method_#{methodName}, null, options, request);
}|]
Expand Down
2 changes: 2 additions & 0 deletions compiler/tests/generated/service_attributes_grpc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ protected FooClient(global::Grpc.Core.ClientBase.ClientBaseConfiguration configu
return fooAsync(message, new global::Grpc.Core.CallOptions(headers, deadline, cancellationToken));
}

[global::Bond.Attribute("foo", "method")]
[global::Bond.Attribute("method", "")]
public virtual global::Grpc.Core.AsyncUnaryCall<global::Bond.Grpc.IMessage<Result>> fooAsync(global::Bond.Grpc.IMessage<Param> request, global::Grpc.Core.CallOptions options)
{
return CallInvoker.AsyncUnaryCall(Method_foo, null, options, request);
Expand Down
14 changes: 14 additions & 0 deletions cpp/test/grpc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,22 @@ target_link_libraries (grpc_test_common PUBLIC
${Boost_UNIT_TEST_FRAMEWORK_LIBRARY}
grpc++)

add_bond_codegen(TARGET grpc_test_services_codegen
services.bond
GRPC)

add_unit_test (io_manager.cpp)

add_unit_test (service_attributes.cpp
"${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/services_types.cpp"
"${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/services_grpc.cpp")
target_include_directories(service_attributes
PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}")
add_dependencies(service_attributes grpc_test_services_codegen)

add_unit_test (thread_pool.cpp)

add_unit_test (unary_call.cpp)

add_unit_test (wait_callback.cpp)
target_link_libraries(wait_callback PRIVATE bond_apply)
72 changes: 72 additions & 0 deletions cpp/test/grpc/service_attributes.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

#include "services_grpc.h"

#include <boost/test/unit_test.hpp>

#include <algorithm>
#include <iterator>
#include <map>
#include <string>

using unit_test::SimpleService;

static bool AttributeMapsEqual(
const std::map<std::string, std::string>& lhs,
const std::map<std::string, std::string>& rhs)
{
return lhs.size() == rhs.size()
&& std::equal(std::begin(lhs), std::end(lhs), std::begin(rhs));
}

BOOST_AUTO_TEST_SUITE(ServiceAttributesTests)

BOOST_AUTO_TEST_CASE(AttributesOnService_CanBeFound)
{
static const std::map<std::string, std::string> expected
{
{ "SomeAttribute", "service value" }
};

BOOST_CHECK(
AttributeMapsEqual(
expected,
SimpleService::Schema::metadata.attributes));
}

BOOST_AUTO_TEST_CASE_TEMPLATE(
AttributesOnMethods_CanBeFound,
Method,
SimpleService::Schema::methods)
{
static const std::map<std::string, std::map<std::string, std::string>> expectedPerMethod
{
{
"IntToInt",
{
{ "SomeAttribute", "method value" },
{ "DifferentAttribute", "" }
}
},
{
"NothingToInt",
{
{"NothingToInt", "no clash"}
}
},
{ "IntToNothing", { } },
{"NothingToNothing", { } }
};

const auto expected = expectedPerMethod.find(Method::metadata.name);
BOOST_CHECK(expected != std::end(expectedPerMethod));
BOOST_CHECK(AttributeMapsEqual(expected->second, Method::metadata.attributes));
}

BOOST_AUTO_TEST_SUITE_END()

bool init_unit_test()
{
return true;
}
18 changes: 18 additions & 0 deletions cpp/test/grpc/services.bond
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import "bond/core/bond.bond"

namespace unit_test;

[SomeAttribute("service value")]
service SimpleService
{
[SomeAttribute("method value")]
[DifferentAttribute("")] // empty string, but present
bond.Box<int32> IntToInt(bond.Box<int32>);

[NothingToInt("no clash")]
bond.Box<int32> NothingToInt();

nothing IntToNothing(bond.Box<int32>);

nothing NothingToNothing();
}
1 change: 1 addition & 0 deletions cs/dnc/test/grpc.tests/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/gen/
14 changes: 14 additions & 0 deletions cs/dnc/test/grpc.tests/grpc.tests.codegen.proj
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="BondCodegenCs" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$(MSBuildExtensionsPath32)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath32)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
<Import Project="$(MSBuildThisFileDirectory)\..\..\..\build\nuget\Common.props" />
<PropertyGroup>
<BOND_INCLUDE_PATH>..\..\..\..\idl</BOND_INCLUDE_PATH>
<BondOutputDirectory>gen\</BondOutputDirectory>
<BondOptions>--grpc</BondOptions>
</PropertyGroup>
<ItemGroup>
<BondCodegen Include="..\..\..\test\grpc\services.bond" />
</ItemGroup>
<Import Project="$(MSBuildThisFileDirectory)\..\..\..\build\nuget\Common.targets" />
</Project>
3 changes: 2 additions & 1 deletion cs/dnc/test/grpc.tests/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"buildOptions": {
"compile": {
"include": [
"../../../test/grpc/*.cs"
"../../../test/grpc/*.cs",
"gen/*.cs"
]
},
"debugType": "portable",
Expand Down
16 changes: 11 additions & 5 deletions cs/src/attributes/Attributes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Bond
// ReSharper disable InconsistentNaming
// ReSharper disable UnusedTypeParameter
namespace Tag
{
{
/// <summary>
/// Represents wstring Bond schema type
/// </summary>
Expand Down Expand Up @@ -46,7 +46,7 @@ public abstract class classT { }
/// <summary>
/// Specifies that a type represents Bond schema
/// </summary>
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Interface,
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Interface,
Inherited = false)]
public sealed class SchemaAttribute : Attribute
{
Expand Down Expand Up @@ -82,7 +82,7 @@ public IdAttribute(ushort id)
}

/// <summary>
/// Specifies type of a field in Bond schema
/// Specifies type of a field in Bond schema
/// </summary>
/// <remarks>
/// If absent the type is inferred from C# type using the following rules:
Expand Down Expand Up @@ -142,8 +142,14 @@ public sealed class RequiredOptionalAttribute : Attribute
/// <summary>
/// Specifies user defined schema attribute
/// </summary>
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Enum |
AttributeTargets.Interface | AttributeTargets.Field | AttributeTargets.Property,
[AttributeUsage(
AttributeTargets.Class |
AttributeTargets.Enum |
AttributeTargets.Field |
AttributeTargets.Interface |
AttributeTargets.Method |
AttributeTargets.Property |
AttributeTargets.Struct ,
AllowMultiple = true, Inherited = false)]
public sealed class AttributeAttribute : Attribute
{
Expand Down
94 changes: 94 additions & 0 deletions cs/test/grpc/AttributesTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace UnitTest.Grpc
{
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using NUnit.Framework;
using NUnit.Framework.Constraints;

[TestFixture]
class AttributesTests
{
private static Dictionary<string, Dictionary<string, string>> ExpectedMethodAttributes = new Dictionary
<string, Dictionary<string, string>>
{
{
"IntToInt",
new Dictionary<string, string>
{
{"SomeAttribute", "method value"},
{
"DifferentAttribute", string.Empty
}
}
},
{
"NothingToInt",
new Dictionary<string, string>
{
{"NothingToInt", "no clash"}
}
},
{"IntToNothing", new Dictionary<string, string>()},
{"NothingToNothing", new Dictionary<string, string>()}
};

[Test]
public void ServiceAttributes_CanBeFound()
{
var serviceTypeInfo = typeof(SimpleService).GetTypeInfo();
var bondAttributes =
serviceTypeInfo.GetCustomAttributes<Bond.AttributeAttribute>()
.ToDictionary(ba => ba.Name, ba => ba.Value);

CollectionAssert.AreEquivalent(
new Dictionary<string, string> {{"SomeAttribute", "service value"}},
bondAttributes);
}

[Test]
public void MethodAttributes_OnServiceBase_CanBeFound()
{
var serviceBaseTypeInfo = typeof(SimpleService.SimpleServiceBase).GetTypeInfo();
var methodAttributes = serviceBaseTypeInfo.DeclaredMethods.Where(mi => mi.IsPublic && mi.IsAbstract)
.ToDictionary(mi => mi.Name, CollectMethodAttributes);

CollectionAssert.AreEquivalent(ExpectedMethodAttributes, methodAttributes);
}

[Test]
public void MethodAttributes_OnClient_CanBeFound()
{
var clientTypeInfo = typeof(SimpleService.SimpleServiceClient).GetTypeInfo();
var clientMethods = clientTypeInfo.DeclaredMethods.Where(mi => mi.IsPublic && mi.IsVirtual);

foreach (MethodInfo mi in clientMethods)
{
string grpcMethodName = ClientNameToGrpcMethodName(mi.Name);

Dictionary<string, string> expected;
bool found = ExpectedMethodAttributes.TryGetValue(grpcMethodName, out expected);
Assert.IsTrue(found, "No expected attributes found for '{0}'", grpcMethodName);

Dictionary<string, string> actual = CollectMethodAttributes(mi);

CollectionAssert.AreEquivalent(expected, actual, "Attribute mismatch for '{0}'", grpcMethodName);
}
}

static Dictionary<string, string> CollectMethodAttributes(MethodInfo mi)
{
return mi.GetCustomAttributes<Bond.AttributeAttribute>().ToDictionary(ba => ba.Name, ba => ba.Value);
}

static string ClientNameToGrpcMethodName(string clientName)
{
const string clientNameSuffix = "Async";
Assert.That(clientName, new EndsWithConstraint(clientNameSuffix));
return clientName.Substring(0, clientName.Length - clientNameSuffix.Length);
}
}
}
10 changes: 8 additions & 2 deletions cs/test/grpc/grpc.csproj
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$(MSBuildExtensionsPath32)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath32)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
<Import Project="$(MSBuildThisFileDirectory)\..\..\build\internal\Common.Internal.props" />
Expand All @@ -12,6 +12,7 @@
<IsCodedUITest>False</IsCodedUITest>
<TestProjectType>UnitTest</TestProjectType>
<DependentOutputPath>$(OutputPath)</DependentOutputPath>
<BondOptions>--grpc</BondOptions>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' == 'Debug' ">
<IntermediateOutputPath>$(IntermediateOutputPath)\Properties\</IntermediateOutputPath>
Expand All @@ -20,14 +21,19 @@
<PropertyGroup Condition=" '$(Configuration)' == 'Fields' ">
<IntermediateOutputPath>$(IntermediateOutputPath)\Fields\</IntermediateOutputPath>
<OutputPath>$(OutputPath)\Fields\</OutputPath>
<BondOptions>--fields --collection-interfaces</BondOptions>
<BondOptions>--collection-interfaces --fields --grpc</BondOptions>
</PropertyGroup>
<ItemGroup>
<Compile Include="AttributesTests.cs" />
<Compile Include="MarshallerTests.cs" />
<Compile Include="MessageTests.cs" />
<Compile Include="NothingCallsTests.cs" />
<Compile Include="Schemas.cs" />
</ItemGroup>
<ItemGroup>
<BondCodegen Include="services.bond" />
<Compile Include="$(IntermediateOutputPath)services_grpc.cs" Condition="false" />
</ItemGroup>
<ItemGroup>
<Reference Include="Grpc.Core">
<HintPath>..\..\packages\Grpc.Core.1.3.0\lib\net45\Grpc.Core.dll</HintPath>
Expand Down
18 changes: 18 additions & 0 deletions cs/test/grpc/services.bond
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import "bond/core/bond.bond"

namespace UnitTest;

[SomeAttribute("service value")]
service SimpleService
{
[SomeAttribute("method value")]
[DifferentAttribute("")] // empty string, but present
bond.Box<int32> IntToInt(bond.Box<int32>);

[NothingToInt("no clash")]
bond.Box<int32> NothingToInt();

nothing IntToNothing(bond.Box<int32>);

nothing NothingToNothing();
}

0 comments on commit 5249212

Please sign in to comment.