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.

Sign up for GitHub

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

Add public ctor for SubscriptionValidationResponse #21975

Merged
merged 2 commits into from
Jun 18, 2021
Merged

Add public ctor for SubscriptionValidationResponse #21975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e2202f0
Add public ctor for SubscriptionValidationResponse
JoshLove-msft Jun 18, 2021
9b5862d
Comment out package reference.
JoshLove-msft Jun 18, 2021
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
5 changes: 3 additions & 2 deletions sdk/eventgrid/Azure.Messaging.EventGrid/api/Azure.Messaging.EventGrid.netstandard2.0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ public static partial class EventGridModelFactory
public static Azure.Messaging.EventGrid.SystemEvents.StorageLifecyclePolicyCompletedEventData StorageLifecyclePolicyCompletedEventData(string scheduleTime = null, Azure.Messaging.EventGrid.SystemEvents.StorageLifecyclePolicyActionSummaryDetail deleteSummary = null, Azure.Messaging.EventGrid.SystemEvents.StorageLifecyclePolicyActionSummaryDetail tierToCoolSummary = null, Azure.Messaging.EventGrid.SystemEvents.StorageLifecyclePolicyActionSummaryDetail tierToArchiveSummary = null) { throw null; }
public static Azure.Messaging.EventGrid.SystemEvents.SubscriptionDeletedEventData SubscriptionDeletedEventData(string eventSubscriptionId = null) { throw null; }
public static Azure.Messaging.EventGrid.SystemEvents.SubscriptionValidationEventData SubscriptionValidationEventData(string validationCode = null, string validationUrl = null) { throw null; }
[System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)]
public static Azure.Messaging.EventGrid.SystemEvents.SubscriptionValidationResponse SubscriptionValidationResponse(string validationResponse = null) { throw null; }
public static Azure.Messaging.EventGrid.SystemEvents.WebAppServicePlanUpdatedEventData WebAppServicePlanUpdatedEventData(Azure.Messaging.EventGrid.SystemEvents.AppServicePlanEventTypeDetail appServicePlanEventTypeDetail = null, Azure.Messaging.EventGrid.SystemEvents.WebAppServicePlanUpdatedEventDataSku sku = null, string name = null, string clientRequestId = null, string correlationRequestId = null, string requestId = null, string address = null, string verb = null) { throw null; }
public static Azure.Messaging.EventGrid.SystemEvents.WebAppServicePlanUpdatedEventDataSku WebAppServicePlanUpdatedEventDataSku(string name = null, string tier = null, string size = null, string family = null, string capacity = null) { throw null; }
Expand Down Expand Up @@ -1673,8 +1674,8 @@ internal SubscriptionValidationEventData() { }
}
public partial class SubscriptionValidationResponse
{
internal SubscriptionValidationResponse() { }
public string ValidationResponse { get { throw null; } }
public SubscriptionValidationResponse() { }
public string ValidationResponse { get { throw null; } set { } }
}
public partial class WebAppServicePlanUpdatedEventData
{
Expand Down
10 changes: 10 additions & 0 deletions sdk/eventgrid/Azure.Messaging.EventGrid/src/Customization/EventGridModelFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System.ComponentModel;
using System.Text.Json;
using Azure.Core;
using Azure.Messaging.EventGrid.SystemEvents;
Expand Down Expand Up @@ -180,6 +181,15 @@ public static ResourceActionCancelEventData ResourceActionCancelEventData(string
{
return new(tenantId, subscriptionId, resourceGroup, resourceProvider, resourceUri, operationName, status, JsonDocument.Parse(authorization).RootElement, JsonDocument.Parse(claims).RootElement, correlationId, JsonDocument.Parse(httpRequest).RootElement);
}

/// <summary> Initializes new instance of SubscriptionValidationResponse class. </summary>
/// <param name="validationResponse"> The validation response sent by the subscriber to Azure Event Grid to complete the validation of an event subscription. </param>
/// <returns> A new <see cref="SystemEvents.SubscriptionValidationResponse"/> instance for mocking. </returns>
[EditorBrowsable(EditorBrowsableState.Never)]
public static SubscriptionValidationResponse SubscriptionValidationResponse(string validationResponse = default)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to hide the factory? I get that you can use the constructor directly but it feels nice to be able to create any model from the model factory.

Copy link
Member Author

@JoshLove-msft JoshLove-msft Jun 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

We don't need to, but it would go against our pattern of only including factories for output models. Codegen no longer generates the model factory for this type, so I had to add it back manually.
/cc @tg-msft @KrzysztofCwalina

Copy link
Member

Choose a reason for hiding this comment

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

Yes - the model factory only exists for constructing the otherwise unconstructible per the guidelines today: https://azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-mocking-factory-builder

It would take nontrivial effort to design and implement this across all of Track 2 when there's already a way of accomplishing it now, so it's not a high priority to me.

{
return new(validationResponse);
}
}
#pragma warning restore CA1054 // URI-like parameters should not be strings
}
8 changes: 0 additions & 8 deletions sdk/eventgrid/Azure.Messaging.EventGrid/src/Generated/EventGridModelFactory.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 13 additions & 2 deletions ....Messaging.EventGrid/src/Generated/Models/SubscriptionValidationResponse.Serialization.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions ...ventgrid/Azure.Messaging.EventGrid/src/Generated/Models/SubscriptionValidationResponse.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion sdk/eventgrid/Azure.Messaging.EventGrid/src/autorest.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,15 @@ directive:
$[path]["x-namespace"] = namespace;
}
if (path.endsWith("EventData") ||
path.endsWith("SubscriptionValidationResponse") ||
path.includes("EventGridEvent") ||
($[path]["x-ms-client-name"] && $[path]["x-ms-client-name"].endsWith("EventData")))
{
$[path]["x-csharp-usage"] = "model,output,converter";
}
if (path.endsWith("SubscriptionValidationResponse"))
{
$[path]["x-csharp-usage"] = "model,input,output,converter";
}
$[path]["x-csharp-formats"] = "json";
if (path.includes("WebAppServicePlanUpdatedEventData"))
{
Expand Down
6 changes: 6 additions & 0 deletions ...osoft.Azure.WebJobs.Extensions.EventGrid/Microsoft.Azure.WebJobs.Extensions.EventGrid.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Azure.WebJobs.Ext
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Azure.WebJobs.Extensions.EventGrid", "src\Microsoft.Azure.WebJobs.Extensions.EventGrid.csproj", "{9322A9CD-ADC3-4BF3-B3AA-063A66585113}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Azure.Messaging.EventGrid", "..\Azure.Messaging.EventGrid\src\Azure.Messaging.EventGrid.csproj", "{3E40026E-2DE9-4FE0-8E25-73F7B35F5D24}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand All @@ -21,6 +23,10 @@ Global
{9322A9CD-ADC3-4BF3-B3AA-063A66585113}.Debug|Any CPU.Build.0 = Debug|Any CPU
{9322A9CD-ADC3-4BF3-B3AA-063A66585113}.Release|Any CPU.ActiveCfg = Release|Any CPU
{9322A9CD-ADC3-4BF3-B3AA-063A66585113}.Release|Any CPU.Build.0 = Release|Any CPU
{3E40026E-2DE9-4FE0-8E25-73F7B35F5D24}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{3E40026E-2DE9-4FE0-8E25-73F7B35F5D24}.Debug|Any CPU.Build.0 = Debug|Any CPU
{3E40026E-2DE9-4FE0-8E25-73F7B35F5D24}.Release|Any CPU.ActiveCfg = Release|Any CPU
{3E40026E-2DE9-4FE0-8E25-73F7B35F5D24}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down
8 changes: 7 additions & 1 deletion ...zure.WebJobs.Extensions.EventGrid/src/Microsoft.Azure.WebJobs.Extensions.EventGrid.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@

<ItemGroup>
<PackageReference Include="Microsoft.Azure.WebJobs" />
<PackageReference Include="Azure.Messaging.EventGrid" />
<!-- Add back after next Event Grid GA -->
<!-- <PackageReference Include="Azure.Messaging.EventGrid" /> -->
</ItemGroup>

<ItemGroup>
<!-- Remove after next Event Grid GA -->
<ProjectReference Include="..\..\Azure.Messaging.EventGrid\src\Azure.Messaging.EventGrid.csproj" />
JoshLove-msft marked this conversation as resolved.
Show resolved Hide resolved
</ItemGroup>

</Project>
6 changes: 4 additions & 2 deletions ...d/Microsoft.Azure.WebJobs.Extensions.EventGrid/src/TriggerBinding/HttpRequestProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
using Azure.Messaging.EventGrid.SystemEvents;
using Microsoft.Extensions.Logging;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using STJ = System.Text.Json;

namespace Microsoft.Azure.WebJobs.Extensions.EventGrid
{
Expand Down Expand Up @@ -69,7 +70,8 @@ internal async Task<HttpResponseMessage> ProcessAsync(
SubscriptionValidationResponse validationResponse = new(){ ValidationResponse = validationCode };
var returnMessage = new HttpResponseMessage(HttpStatusCode.OK)
{
Content = new StringContent(JsonConvert.SerializeObject(validationResponse))
// use System.Text.Json to leverage the custom converter so that the casing is correct.
Content = new StringContent(STJ.JsonSerializer.Serialize(validationResponse))
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean the behavior of JsonConvert.SerializeObject(validationResponse) differs before and after this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, previously we were using a ValidationResponse model defined in the extension library which had a JsonProperty attribute to get camel casing.

Now, we are using STJ converter which uses camel casing. I couldn't continue to use NewtonSoft and use the Event Grid SDK model. It seemed worth using the SDK model here to validate that it works.

Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW I tested it out and both Pascal and camelCase work fine when adding a subscription, but in order to maintain the same behavior as before (and to make the tests pass) it seems better to use camel case.

};
_logger.LogInformation($"perform handshake with eventGrid for function: {functionName}");
return returnMessage;
Expand Down
13 changes: 0 additions & 13 deletions ...t.Azure.WebJobs.Extensions.EventGrid/src/TriggerBinding/SubscriptionValidationResponse.cs
View file
Open in desktop

This file was deleted.