mirrorsharp icon indicating copy to clipboard operation
mirrorsharp copied to clipboard

Allow developer to set a custom web socket url

Open FortuneN opened this issue 5 years ago • 2 comments

  • Allow developer to set a custom web socket url
  • Default behavior does not break current usages
  • Default behavior allows more usages whose URL ends with "/mirrorsharp"
  • When a WebSocketUrl is set, it is used instead of the default behaviour
  • We are lenient with the case of the URL
  • We are lenient with leading or following spaces

[ISSUE] https://github.com/ashmind/mirrorsharp/issues/110

FortuneN avatar Jan 15 '20 01:01 FortuneN

Thanks for this! I think the goal definitely makes sense. I would be keen to investigate a bit further on the new endpoint routing -- seems like something we could use here? Not sure yet, haven't tried it myself.

ashmind avatar Jan 18 '20 21:01 ashmind

I have investigated the endpoint routing a bit, and I think we don't have to handle the URL manually.

What we could do instead is:

  1. If endpoint middleware is not used, specific URL can be mapped as
app.Map("/mirrorsharp", app => app.UseMirrorSharp());

Obviously we want people to avoid mistakes, so I would actually obsolete UseMirrorSharp and expose MapMirrorSharp instead, but put URL as a separate parameter instead of options to match standard Map.

  1. If endpoint middleware is used, specific URL can be mapped as
    app.UseEndpoints(endpoints =>
    {
        endpoints.Map("/mirrorsharp", endpoints.CreateApplicationBuilder()
            .UseMirrorSharp()
            .Build());
    });

This is obviously too verbose, so providing another MapMirrorSharp method on endpoints would make sense (with required URL I think, for clarity).

So, in summary, I suggest the following:

  1. New MapMirrorSharp extension method on IApplicationBuilder, taking an URL (should it be a required and a first parameter, for clarity?)
  2. New MapMirrorSharp extension method on IEndpointRouteBuilder, taking a URL as a required first parameter for clarity
  3. Obsolete current UseMirrorSharp usage.

ashmind avatar Jan 22 '20 08:01 ashmind