Skip to content
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.

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

Only raise events from RPC targets once #520

Merged
1 commit merged into from
Aug 12, 2020
Merged
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
25 changes: 24 additions & 1 deletion src/StreamJsonRpc.Tests/TargetObjectEventsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,14 @@ public void GenericServerEventSubscriptionLifetime()
Assert.Null(this.server.ServerEventWithCustomArgsAccessor);
}

/// <summary>Ensures that JsonRpc only adds one event handler to target objects where events are declared multiple times in an type hierarchy.</summary>
/// <remarks>This is a regression test for <see href="https://github.com/microsoft/vs-streamjsonrpc/issues/481">this bug</see>.</remarks>
[Fact]
public void EventOverridesStillGetJustOneHandler()
{
Assert.Equal(1, this.server.HandlersAttachedToAbstractBaseEvent);
}

[Fact]
public void IncompatibleEventHandlerType()
{
Expand Down Expand Up @@ -298,10 +306,17 @@ private class Client
public void ServerEventWithCustomGenericDelegateAndArgs(MessageEventArgs<string> args) => this.ServerEventWithCustomGenericDelegateAndArgsRaised?.Invoke(args);
}

private class Server : IServer
private abstract class ServerBase
{
public abstract event EventHandler? AbstractBaseEvent;
}

private class Server : ServerBase, IServer
{
private EventHandler? explicitInterfaceImplementationEvent;

private EventHandler? abstractBaseEvent;

public delegate void MessageReceivedEventHandler<T>(object sender, MessageEventArgs<T> args)
where T : class;

Expand All @@ -315,6 +330,12 @@ public delegate void MessageReceivedEventHandler<T>(object sender, MessageEventA

public event EventHandler? InterfaceEvent;

public override event EventHandler? AbstractBaseEvent
{
add => this.abstractBaseEvent += value;
remove => this.abstractBaseEvent -= value;
}

private static event EventHandler? PrivateStaticServerEvent;

event EventHandler IServer.ExplicitInterfaceImplementation_Event
Expand All @@ -329,6 +350,8 @@ event EventHandler IServer.ExplicitInterfaceImplementation_Event

internal EventHandler<CustomEventArgs>? ServerEventWithCustomArgsAccessor => this.ServerEventWithCustomArgs;

internal int HandlersAttachedToAbstractBaseEvent => this.abstractBaseEvent?.GetInvocationList().Length ?? 0;

public static void TriggerPublicStaticServerEvent(EventArgs args) => PublicStaticServerEvent?.Invoke(null, args);

public void TriggerEvent(EventArgs args)
Expand Down
12 changes: 12 additions & 0 deletions src/StreamJsonRpc/JsonRpc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,7 @@ public void AddLocalRpcTarget(Type exposingMembersOn, object target, JsonRpcTarg

if (options.NotifyClientOfEvents)
{
HashSet<string>? eventsDiscovered = null;
for (TypeInfo? t = exposingMembersOn.GetTypeInfo(); t != null && t != typeof(object).GetTypeInfo(); t = t.BaseType?.GetTypeInfo())
{
foreach (EventInfo evt in t.DeclaredEvents)
Expand All @@ -764,6 +765,17 @@ public void AddLocalRpcTarget(Type exposingMembersOn, object target, JsonRpcTarg
this.eventReceivers = new List<EventReceiver>();
}

if (eventsDiscovered is null)
{
eventsDiscovered = new HashSet<string>(StringComparer.Ordinal);
}

if (!eventsDiscovered.Add(evt.Name))
{
// Do not add the same event again. It can appear multiple times in a type hierarchy.
continue;
}

if (this.TraceSource.Switch.ShouldTrace(TraceEventType.Information))
{
this.TraceSource.TraceEvent(TraceEventType.Information, (int)TraceEvents.LocalEventListenerAdded, "Listening for events from {0}.{1} to raise notification.", target.GetType().FullName, evt.Name);
Expand Down