pm7y / AzureEventGridSimulator

A simulator that provides endpoints to mimic the functionality of Azure Event Grid topics and subscribers and is compatible with the Azure.Messaging.EventGrid client library.
MIT License
82 stars 40 forks source link

Test with Azure.Messaging.EventGrid #22

Closed jongio closed 3 years ago

jongio commented 3 years ago

We are shipping a new Event Grid lib. https://www.nuget.org/packages/Azure.Messaging.EventGrid/4.0.0-beta.5

Do you have a test harness that I could plug this into to ensure your simulator works as expected?

Thanks, Jon

azsdke2e azsdke2e2

simbaja commented 3 years ago

@jongio I just tried the new library against this emulator and ran into an issue. The new library is only using the host name portion of the uri internally, which means that it ignores the port. I ended up working around the issue by creating a custom UriParser that puts the port into the hostname to make it work. Unless you're running the emulator on port 443, it doesn't seem like it would work with the newest library. Hope that this helps.

pm7y commented 3 years ago

Hey @jongio, thanks for the question and thanks to @simbaja for taking the time to reply. Do you think you are likely to change the new Event Grid lib to respect the port (as previous versions have) in light of this @jongio or nah?

simbaja commented 3 years ago

In case anyone else needs a quick and dirty way of handling this, the UriParser is below. You'd need to register it by calling UriParser.Register(), but that should get the port into the host name, which will allow this emulator to work. Perhaps instead of Host property, the EventGridPublisherClient should be leveraging the Authority property?

public class EventGridUriParser : GenericUriParser
    {
        public EventGridUriParser() : base(GenericUriParserOptions.Default)
        {

        }

        protected override void InitializeAndValidate(Uri uri, out UriFormatException parsingError)
        {
            parsingError = null;
        }

        protected override bool IsWellFormedOriginalString(Uri uri)
        {
            return true;
        }

        protected override string GetComponents(Uri uri, UriComponents components, UriFormat format)
        {
            var httpUri = new Uri(uri.OriginalString.Replace("eg://", "https://"));
            return components == UriComponents.Host ?
                String.Format("{0}:{1}", httpUri.Host, httpUri.Port) :
                httpUri.GetComponents(components, format);

        }
    }
jongio commented 3 years ago

@pmcilreavy - Could you please file an issue in the Azure SDK .NET repo? https://aka.ms/netsdk? I think it is best coming from you b/c you have all the context and working code above. Feel free to ref this issue.

We are discussing internally and would like to flesh this out.

Thanks! Jon

simbaja commented 3 years ago

@pmcilreavy @jongio I think that there's only one line that needs to change to make it work: https://github.com/Azure/azure-sdk-for-net/blob/5d5a994afa64c190772ee645a0a40bf289b72b26/sdk/eventgrid/Azure.Messaging.EventGrid/src/Customization/EventGridPublisherClient.cs#L24

If this line is changed to be private string _hostName => _endpoint.Authority; the emulator should work as expected (feel free to change the variable name throughout as well). That's essentially what my hack is doing - it's forcing the Uri to interpret Host as Authority so it connects correctly.

@pmcilreavy I'd probably agree with Jon that it's probably better coming from you than some random person, but let me know if I can help further here.

jongio commented 3 years ago

@pmcilreavy or @simbaja - Could you please retest? PR for fix has been merged https://github.com/Azure/azure-sdk-for-net/pull/19192

You can either wait until the next release which should be next week or try the from the Dev Feed: https://github.com/Azure/azure-sdk-for-net/blob/master/CONTRIBUTING.md#nuget-package-dev-feed

Thank you

pm7y commented 3 years ago

@jongio I tested with 4.0.0-beta.4 and it did not work (as expected) and then I tested with 4.0.0-beta.5 and it worked 🎉

simbaja commented 3 years ago

@jongio I tested with 4.0.0-beta.4 and it did not work (as expected) and then I tested with 4.0.0-beta.5 and it worked 🎉

@pmcilreavy , did you perhaps test the dev feed? I don't think beta.5 has the fix to honor the port (says it was published 2/9)?

I'll have to wait until the next release on NuGet, but I'll test at that time and let you know what I see. The fix looks like it should work though. Thanks again for resolving.

pm7y commented 3 years ago

@jongio @simbaja Sorry my bad. You're right. I initially tried with 4.0.0-alpha.20210304.1 which worked (event received by the simulator below). I then changed to 4.0.0-beta.5 and it seemed like that also worked. I just re-tested and yes, beta.5 does not work. I guess I didn't clean or restore nuget properly perhaps. Not sure what happen. Anyway, apologies for the confusion.

[
  {
    "id": "eb32aeec-32ed-42ee-8956-d2505e6af2a2",
    "subject": "/the/subject/4.0.0-alpha.20210304.1+e55b6a1fadeff56090f0eaaeeaae2b495eee0b57",
    "data": {
      "Id": 1
    },
    "eventType": "The.Event.Type",
    "eventTime": "2021-03-05T22:28:49.4033463Z",
    "dataVersion": "v1",
    "metadataVersion": "1",
    "topic": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/eventGridSimulator/providers/Microsoft.EventGrid/topics/AzureMessagingEventGridClientTest"
  }
]

Test harness attached GridTest.zip

jongio commented 3 years ago

@pmcilreavy - I see that you closed this. Can you confirm it works with the latest? Can you also commit your tests so I can try it here?

pm7y commented 3 years ago

@jongio Looks ok from my point of view based on my non-exhaustive testing with 4.1.0.

I've included those tests now in the src solution here. So you can now pull the latest and run the tests in AzureMessagingEventGridTest.cs which will exercise the simulator via Azure.Messaging.EventGrid (v4.1.0). Not many tests at the moment (it's a work in progress), but hopefully covers the main question (i.e. can you still send an event to the sim). Let me know if you see any glaring omissions though.