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
Expand Up @@ -89,10 +89,11 @@ public async Task<ResolutionDetails<bool>> ResolveBooleanValueAsync(string flagK

return new ResolutionDetails<bool>(
flagKey: flagKey,
value: (bool)resolveBooleanResponse.Value,
value: resolveBooleanResponse.Value,
reason: resolveBooleanResponse.Reason,
variant: resolveBooleanResponse.Variant
);
variant: resolveBooleanResponse.Variant,
flagMetadata: BuildFlagMetadata(resolveBooleanResponse.Metadata)
);
}, context).ConfigureAwait(false);
}

Expand All @@ -110,8 +111,9 @@ public async Task<ResolutionDetails<string>> ResolveStringValueAsync(string flag
flagKey: flagKey,
value: resolveStringResponse.Value,
reason: resolveStringResponse.Reason,
variant: resolveStringResponse.Variant
);
variant: resolveStringResponse.Variant,
flagMetadata: BuildFlagMetadata(resolveStringResponse.Metadata)
);
}, context).ConfigureAwait(false);
}

Expand All @@ -129,8 +131,9 @@ public async Task<ResolutionDetails<int>> ResolveIntegerValueAsync(string flagKe
flagKey: flagKey,
value: (int)resolveIntResponse.Value,
reason: resolveIntResponse.Reason,
variant: resolveIntResponse.Variant
);
variant: resolveIntResponse.Variant,
flagMetadata: BuildFlagMetadata(resolveIntResponse.Metadata)
);
}, context).ConfigureAwait(false);
}

Expand All @@ -148,8 +151,9 @@ public async Task<ResolutionDetails<double>> ResolveDoubleValueAsync(string flag
flagKey: flagKey,
value: resolveDoubleResponse.Value,
reason: resolveDoubleResponse.Reason,
variant: resolveDoubleResponse.Variant
);
variant: resolveDoubleResponse.Variant,
flagMetadata: BuildFlagMetadata(resolveDoubleResponse.Metadata)
);
}, context).ConfigureAwait(false);
}

Expand All @@ -167,8 +171,9 @@ public async Task<ResolutionDetails<Value>> ResolveStructureValueAsync(string fl
flagKey: flagKey,
value: ConvertObjectToValue(resolveObjectResponse.Value),
reason: resolveObjectResponse.Reason,
variant: resolveObjectResponse.Variant
);
variant: resolveObjectResponse.Variant,
flagMetadata: BuildFlagMetadata(resolveObjectResponse.Metadata)
);
}, context).ConfigureAwait(false);
}

Expand Down Expand Up @@ -451,6 +456,39 @@ private FeatureProviderException GetOFException(RpcException e)
}
}

#nullable enable
private static ImmutableMetadata? BuildFlagMetadata(Struct? metadata)
{
var items = new Dictionary<string, object>();

foreach (var entry in metadata?.Fields ?? [])
{
switch (entry.Value.KindCase)
{
case ProtoValue.KindOneofCase.NumberValue:
items.Add(entry.Key, entry.Value.NumberValue);
break;
case ProtoValue.KindOneofCase.StringValue:
items.Add(entry.Key, entry.Value.StringValue);
break;
case ProtoValue.KindOneofCase.BoolValue:
items.Add(entry.Key, entry.Value.BoolValue);
break;

// Unsupported types for metadata
case ProtoValue.KindOneofCase.None:
case ProtoValue.KindOneofCase.NullValue:
case ProtoValue.KindOneofCase.StructValue:
case ProtoValue.KindOneofCase.ListValue:
default:
break;
}
}

return items.Count > 0 ? new ImmutableMetadata(items) : null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to return null, or an empty Metadata object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were returning null before so I wanted to keep it consistent. I'm not sure what the spec says, should we always provide a metadata array/collection even if it contains no items?

}
#nullable restore

private Service.ServiceClient BuildClientForPlatform(FlagdConfig config)
{
var useUnixSocket = config.GetUri().ToString().StartsWith("unix://");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Google.Protobuf;
using Grpc.Core;
using NSubstitute;
using NSubstitute.ExceptionExtensions;
Expand Down Expand Up @@ -301,6 +302,107 @@ public static IEnumerable<object[]> ResolveValueDataLossData()
};
}

[Theory]
[MemberData(nameof(ResolveValueFlagdMetadata))]
internal async Task ResolveValueAsync_AddsFlagMetadata<T>(Func<RpcResolver, Task<ResolutionDetails<T>>> act,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

thought: I wonder if the tests would be cleaner and more understandable if I separated them out into ResolveStringValueAsync and ResolveBooleanValueAsync which all check the flag metadata is added?

I think the test theory is nice but probably makes it hard to follow. Thoughts?

Action<Service.ServiceClient, Google.Protobuf.WellKnownTypes.Struct> setup)
{
// Arrange
var mockGrpcClient = Substitute.For<Service.ServiceClient>();

var setupMetadata = new Google.Protobuf.WellKnownTypes.Struct()
{
Fields =
{
{ "key1", Google.Protobuf.WellKnownTypes.Value.ForString("value1") },
{ "key2", Google.Protobuf.WellKnownTypes.Value.ForString(string.Empty) },
{ "key3", Google.Protobuf.WellKnownTypes.Value.ForBool(true) },
{ "key4", Google.Protobuf.WellKnownTypes.Value.ForBool(false) },
{ "key5", Google.Protobuf.WellKnownTypes.Value.ForNumber(1) },
{ "key6", Google.Protobuf.WellKnownTypes.Value.ForNumber(3.14) },
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also add a test case for a really large number

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added test case for int.MaxValue in eef8af6

{ "key7", Google.Protobuf.WellKnownTypes.Value.ForNumber(-0.531921) },
{ "key8", Google.Protobuf.WellKnownTypes.Value.ForList(Google.Protobuf.WellKnownTypes.Value.ForString("1"), Google.Protobuf.WellKnownTypes.Value.ForString("2")) },
{ "key9", Google.Protobuf.WellKnownTypes.Value.ForNull() },
{ "key10", Google.Protobuf.WellKnownTypes.Value.ForStruct(new Google.Protobuf.WellKnownTypes.Struct()
{
Fields = { { "innerkey", Google.Protobuf.WellKnownTypes.Value.ForBool(true) } }
}) },
{ "key11", Google.Protobuf.WellKnownTypes.Value.ForNumber(int.MaxValue) }
}
};

setup(mockGrpcClient, setupMetadata);

var config = new FlagdConfig();
var resolver = new RpcResolver(mockGrpcClient, config, null);

// Act
var value = await act(resolver);

// Assert
var metadata = value.FlagMetadata;
Assert.NotNull(metadata);
Assert.Equal("value1", metadata.GetString("key1"));
Assert.Equal(string.Empty, metadata.GetString("key2"));
Assert.True(metadata.GetBool("key3"));
Assert.False(metadata.GetBool("key4"));
Assert.Equal(1, metadata.GetInt("key5"));
Assert.Equal(3.14, metadata.GetDouble("key6"));
Assert.Equal(-0.531921, metadata.GetDouble("key7"));
Assert.Null(metadata.GetString("key8"));
Assert.Null(metadata.GetString("key9"));
Assert.Null(metadata.GetString("key10"));
Assert.Equal(int.MaxValue, metadata.GetInt("key11"));
}

public static IEnumerable<object[]> ResolveValueFlagdMetadata()
{
const string flagKey = "test-key";

yield return new object[]
{
new Func<RpcResolver, Task<ResolutionDetails<bool>>>(r => r.ResolveBooleanValueAsync(flagKey, false)),
new Action<Service.ServiceClient, Google.Protobuf.WellKnownTypes.Struct>((client, metadata) => client.ResolveBooleanAsync(Arg.Any<ResolveBooleanRequest>())
.Returns(CreateRpcResponse(new ResolveBooleanResponse() { Value = true, Variant = "true", Reason = "TARGETING_MATCH", Metadata = metadata })))
};
yield return new object[]
{
new Func<RpcResolver, Task<ResolutionDetails<string>>>(r => r.ResolveStringValueAsync(flagKey, "def")),
new Action<Service.ServiceClient, Google.Protobuf.WellKnownTypes.Struct>((client, metadata) => client.ResolveStringAsync(Arg.Any<ResolveStringRequest>())
.Returns(CreateRpcResponse(new ResolveStringResponse() { Value = "one", Variant = "default", Reason = "TARGETING_MATCH", Metadata = metadata })))
};
yield return new object[]
{
new Func<RpcResolver, Task<ResolutionDetails<int>>>(r => r.ResolveIntegerValueAsync(flagKey, 3)),
new Action<Service.ServiceClient, Google.Protobuf.WellKnownTypes.Struct>((client, metadata) => client.ResolveIntAsync(Arg.Any<ResolveIntRequest>())
.Returns(CreateRpcResponse(new ResolveIntResponse() { Value = 1, Variant = "one", Reason = "TARGETING_MATCH", Metadata = metadata })))
};
yield return new object[]
{
new Func<RpcResolver, Task<ResolutionDetails<double>>>(r => r.ResolveDoubleValueAsync(flagKey, 3.5)),
new Action<Service.ServiceClient, Google.Protobuf.WellKnownTypes.Struct>((client, metadata) => client.ResolveFloatAsync(Arg.Any<ResolveFloatRequest>())
.Returns(CreateRpcResponse(new ResolveFloatResponse() { Value = 1.61, Variant = "one", Reason = "TARGETING_MATCH", Metadata = metadata })))
};
yield return new object[]
{
new Func<RpcResolver, Task<ResolutionDetails<Value>>>(r => r.ResolveStructureValueAsync(flagKey, new Value(Structure.Builder().Set("value1", true).Build()))),
new Action<Service.ServiceClient, Google.Protobuf.WellKnownTypes.Struct>((client, metadata) => client.ResolveObjectAsync(Arg.Any<ResolveObjectRequest>())
.Returns(CreateRpcResponse(new ResolveObjectResponse()
{
Value = new Google.Protobuf.WellKnownTypes.Struct(),
Variant = "one",
Reason = "TARGETING_MATCH",
Metadata = metadata
})))
};
}

private static AsyncUnaryCall<T> CreateRpcResponse<T>(T resp)
where T : IMessage<T>, IBufferMessage
{
return new AsyncUnaryCall<T>(Task.FromResult(resp), Task.FromResult(Grpc.Core.Metadata.Empty), () => Status.DefaultSuccess, () => Grpc.Core.Metadata.Empty, () => { });
}

private static Service.ServiceClient SetupGrpcStream(List<EventStreamResponse> responses)
{
var mockGrpcClient = Substitute.For<Service.ServiceClient>();
Expand Down