Merge pull request #27 from microsoft/po/alignWithSpec

Add validation for `apiDeploymentBaseUrl`
microsoft · Sep 28, 2023 · 2c5a119 · 2c5a119
2 parents fbe3de0 + 25bee65
commit 2c5a119
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 26 deletions.
diff --git a/.github/workflows/sonarcloud.yml b/.github/workflows/sonarcloud.yml
@@ -71,7 +71,7 @@ jobs:
           CoverletOutputFormat: "opencover" # https://github.com/microsoft/vstest/issues/4014#issuecomment-1307913682
         shell: pwsh
         run: |
-          ./.sonar/scanner/dotnet-sonarscanner begin /k:"microsoft_OpenApi.ApiManifest" /o:"microsoft" /d:sonar.login="${{ secrets.SONAR_TOKEN }}" /d:sonar.host.url="https://sonarcloud.io" /d:sonar.cs.opencover.reportsPaths="tests/**/coverage.opencover.xml"
+          ./.sonar/scanner/dotnet-sonarscanner begin /k:"microsoft_OpenApi.ApiManifest" /o:"microsoft" /d:sonar.login="${{ secrets.SONAR_TOKEN }}" /d:sonar.host.url="https://sonarcloud.io" /d:sonar.cs.opencover.reportsPaths="src/tests/**/coverage.opencover.xml"
           dotnet build
           dotnet test apimanifest.sln --no-build --verbosity normal /p:CollectCoverage=true /p:CoverletOutputFormat=opencover
           ./.sonar/scanner/dotnet-sonarscanner end /d:sonar.login="${{ secrets.SONAR_TOKEN }}"
diff --git a/src/lib/ApiDependency.cs b/src/lib/ApiDependency.cs
@@ -5,10 +5,17 @@ public class ApiDependency
 {
     public string? ApiDescriptionUrl { get; set; }
     public string? ApiDescriptionVersion { get; set; }
-    public string? ApiDeploymentBaseUrl { get; set; }
+    private string? _apiDeploymentBaseUrl;
+    public string? ApiDeploymentBaseUrl {
+        get { return _apiDeploymentBaseUrl; }
+        set
+        {
+            ValidateApiDeploymentBaseUrl(value);
+            _apiDeploymentBaseUrl = value;
+        }
+    }
     public AuthorizationRequirements? AuthorizationRequirements { get; set; }
-    public List<Request> Requests { get; set; } = new List<Request>();
-    // TODO: Settle on a name. Extensions vs extensibility as per https://www.ietf.org/archive/id/draft-miller-api-manifest-01.html.
+    public List<RequestInfo> Requests { get; set; } = new List<RequestInfo>();
     public Extensions? Extensions { get; set; }
 
     private const string ApiDescriptionUrlProperty = "apiDescriptionUrl";
@@ -53,22 +60,32 @@ public void Write(Utf8JsonWriter writer)
         writer.WriteEndObject();
     }
 
+
+    // Load Method
+    internal static ApiDependency Load(JsonElement value)
+    {
+        var apiDependency = new ApiDependency();
+        ParsingHelpers.ParseMap(value, apiDependency, handlers);
+        return apiDependency;
+    }
+
+    private static void ValidateApiDeploymentBaseUrl(string? apiDeploymentBaseUrl)
+    {
+        // Check if the apiDeploymentBaseUrl is a valid URL and ends in a slash.
+        if (apiDeploymentBaseUrl == null || !apiDeploymentBaseUrl.EndsWith("/", StringComparison.Ordinal) || !Uri.TryCreate(apiDeploymentBaseUrl, UriKind.Absolute, out _))
+        {
+            throw new ArgumentException($"The {nameof(apiDeploymentBaseUrl)} must be a valid URL and end in a slash.");
+        }
+    }
+
     // Fixed fieldmap for ApiDependency
     private static readonly FixedFieldMap<ApiDependency> handlers = new()
     {
         { ApiDescriptionUrlProperty, (o,v) => {o.ApiDescriptionUrl = v.GetString();  } },
         { ApiDescriptionVersionProperty, (o,v) => {o.ApiDescriptionVersion = v.GetString();  } },
         { ApiDeploymentBaseUrlProperty, (o,v) => {o.ApiDeploymentBaseUrl = v.GetString();  } },
         { AuthorizationRequirementsProperty, (o,v) => {o.AuthorizationRequirements = AuthorizationRequirements.Load(v);  } },
-        { RequestsProperty, (o,v) => {o.Requests = ParsingHelpers.GetList(v, Request.Load);  } },
+        { RequestsProperty, (o,v) => {o.Requests = ParsingHelpers.GetList(v, RequestInfo.Load);  } },
         { ExtensionsProperty, (o,v) => {o.Extensions = Extensions.Load(v);  } }
     };
-
-    // Load Method
-    internal static ApiDependency Load(JsonElement value)
-    {
-        var apiDependency = new ApiDependency();
-        ParsingHelpers.ParseMap(value, apiDependency, handlers);
-        return apiDependency;
-    }
 }
diff --git a/src/lib/Request.cs → src/lib/RequestInfo.cs b/src/lib/Request.cs → src/lib/RequestInfo.cs
@@ -1,7 +1,7 @@
 using System.Text.Json;
 
 namespace Microsoft.OpenApi.ApiManifest;
-public class Request
+public class RequestInfo
 {
     public string? Method { get; set; }
     public string? UriTemplate { get; set; }
@@ -26,7 +26,7 @@ public void Write(Utf8JsonWriter writer)
         writer.WriteEndObject();
     }
     // Fixed fieldmap for Request
-    private static readonly FixedFieldMap<Request> handlers = new()
+    private static readonly FixedFieldMap<RequestInfo> handlers = new()
     {
         { MethodProperty, (o,v) => {o.Method = v.GetString();  } },
         { UriTemplateProperty, (o,v) => {o.UriTemplate = v.GetString();  } },
@@ -35,9 +35,9 @@ public void Write(Utf8JsonWriter writer)
     };
 
     // Load Method
-    internal static Request Load(JsonElement value)
+    internal static RequestInfo Load(JsonElement value)
     {
-        var request = new Request();
+        var request = new RequestInfo();
         ParsingHelpers.ParseMap(value, request, handlers);
         return request;
     }

diff --git a/src/tests/BasicTests.cs b/src/tests/BasicTests.cs
@@ -34,6 +34,8 @@ public void SerializeDocument()
         Debug.WriteLine(json);
         var doc = JsonDocument.Parse(json);
         Assert.NotNull(doc);
+        Assert.Equal("application-name", doc.RootElement.GetProperty("applicationName").GetString());
+        Assert.Equal("Microsoft", doc.RootElement.GetProperty("publisher").GetProperty("name").GetString());
     }
 
     // Deserialize the ApiManifestDocument from a string
@@ -53,10 +55,11 @@ public void DeserializeDocument()
         Assert.Equivalent(exampleApiManifest.Publisher, apiManifest.Publisher);
         Assert.Equivalent(exampleApiManifest.ApiDependencies["example"].Requests, apiManifest.ApiDependencies["example"].Requests);
         Assert.Equivalent(exampleApiManifest.ApiDependencies["example"].ApiDescriptionUrl, apiManifest.ApiDependencies["example"].ApiDescriptionUrl);
+        Assert.Equivalent(exampleApiManifest.ApiDependencies["example"].ApiDeploymentBaseUrl, apiManifest.ApiDependencies["example"].ApiDeploymentBaseUrl);
         var expectedAuth = exampleApiManifest.ApiDependencies["example"].AuthorizationRequirements;
         var actualAuth = apiManifest.ApiDependencies["example"].AuthorizationRequirements;
         Assert.Equivalent(expectedAuth?.ClientIdentifier, actualAuth?.ClientIdentifier);
-        Assert.Equivalent(expectedAuth?.Access[0].Content.ToJsonString(), actualAuth.Access[0].Content.ToJsonString());
+        Assert.Equivalent(expectedAuth?.Access?[0]?.Content?.ToJsonString(), actualAuth?.Access?[0]?.Content?.ToJsonString());
     }
 
 
@@ -127,42 +130,42 @@ public void ParseApiDescriptionVersionField()
     public void ParsesApiDeploymentBaseUrl()
     {
         // Given
-        var serializedValue = "{\"applicationName\": \"application-name\", \"apiDependencies\": { \"graph\": {\"apiDeploymentBaseUrl\":\"https://example.org\"}}}";
+        var serializedValue = "{\"applicationName\": \"application-name\", \"apiDependencies\": { \"graph\": {\"apiDeploymentBaseUrl\":\"https://example.org/\"}}}";
         var doc = JsonDocument.Parse(serializedValue);
 
         // When
         var apiManifest = ApiManifestDocument.Load(doc.RootElement);
 
         // Then
-        Assert.Equal("https://example.org", apiManifest.ApiDependencies["graph"].ApiDeploymentBaseUrl);
+        Assert.Equal("https://example.org/", apiManifest.ApiDependencies["graph"].ApiDeploymentBaseUrl);
     }
 
     [Fact]
     public void ParsesApiDeploymentBaseUrlWithDifferentCasing()
     {
         // Given
-        var serializedValue = "{\"applicationName\": \"application-name\", \"apiDependencies\": { \"graph\": {\"APIDeploymentBaseUrl\":\"https://example.org\"}}}";
+        var serializedValue = "{\"applicationName\": \"application-name\", \"apiDependencies\": { \"graph\": {\"APIDeploymentBaseUrl\":\"https://example.org/\"}}}";
         var doc = JsonDocument.Parse(serializedValue);
 
         // When
         var apiManifest = ApiManifestDocument.Load(doc.RootElement);
 
         // Then
-        Assert.Equal("https://example.org", apiManifest.ApiDependencies["graph"].ApiDeploymentBaseUrl);
+        Assert.Equal("https://example.org/", apiManifest.ApiDependencies["graph"].ApiDeploymentBaseUrl);
     }
 
     [Fact]
     public void DoesNotFailOnExtraneousProperty()
     {
         // Given
-        var serializedValue = "{\"applicationName\": \"application-name\", \"apiDependencies\": { \"graph\": {\"APIDeploymentBaseUrl\":\"https://example.org\", \"APISensitivity\":\"low\"}}}";
+        var serializedValue = "{\"applicationName\": \"application-name\", \"apiDependencies\": { \"graph\": {\"APIDeploymentBaseUrl\":\"https://example.org/\", \"APISensitivity\":\"low\"}}}";
         var doc = JsonDocument.Parse(serializedValue);
 
         // When
         var apiManifest = ApiManifestDocument.Load(doc.RootElement);
 
         // Then
-        Assert.Equal("https://example.org", apiManifest.ApiDependencies["graph"].ApiDeploymentBaseUrl);
+        Assert.Equal("https://example.org/", apiManifest.ApiDependencies["graph"].ApiDeploymentBaseUrl);
     }
 
     private static ApiManifestDocument CreateDocument()
@@ -174,6 +177,7 @@ private static ApiManifestDocument CreateDocument()
                 { "example", new()
                     {
                         ApiDescriptionUrl = "https://example.org",
+                        ApiDeploymentBaseUrl = "https://example.org/v1.0/",
                         AuthorizationRequirements = new()
                         {
                             ClientIdentifier = "1234",
@@ -187,7 +191,7 @@ private static ApiManifestDocument CreateDocument()
                             }
                         },
                         Requests = new() {
-                            new() { Method = "GET", UriTemplate = "/api/v1/endpoint" },
+                            new () { Method = "GET", UriTemplate = "/api/v1/endpoint" },
                             new () { Method = "POST", UriTemplate = "/api/v1/endpoint"}
                         }
                     }

diff --git a/src/tests/CreateTests.cs b/src/tests/CreateTests.cs
@@ -41,6 +41,23 @@ public void CreatePublisherWithInvalidEmail(string contactEmail)
         );
     }
 
+    [Theory]
+    [InlineData("foo")]
+    [InlineData("https://foo.com")]
+    [InlineData("http://128.0.0.0")]
+    [InlineData("https://foo@@bar.com")]
+    [InlineData("https://foo bar.com/")]
+    [InlineData("https://graph.microsoft.com/v1.0")]
+    public void CreateApiDependencyWithInvalidApiDeploymentBaseUrl(string apiDeploymentBaseUrl)
+    {
+        _ = Assert.Throws<ArgumentException>(() =>
+        {
+            var apiDependency = new ApiDependency();
+            apiDependency.ApiDeploymentBaseUrl = apiDeploymentBaseUrl;
+        }
+        );
+    }
+
     // Create test to instantiate ApiManifest with auth
     [Fact]
     public void CreateApiManifestWithAuthorizationRequirements()
@@ -50,6 +67,7 @@ public void CreateApiManifestWithAuthorizationRequirements()
             Publisher = new(name: "Contoso", contactEmail: "[email protected]"),
             ApiDependencies = new() {
                 { "Contoso.Api", new() {
+                    ApiDeploymentBaseUrl = "https://api.contoso.com/",
                     AuthorizationRequirements = new() {
                         ClientIdentifier = "2143234-234324-234234234-234",
                         Access = new() {
@@ -65,8 +83,9 @@ public void CreateApiManifestWithAuthorizationRequirements()
             }
         };
         Assert.NotNull(apiManifest.ApiDependencies["Contoso.Api"].AuthorizationRequirements);
+        Assert.Equal("https://api.contoso.com/", apiManifest.ApiDependencies["Contoso.Api"].ApiDeploymentBaseUrl);
         Assert.Equal("2143234-234324-234234234-234", apiManifest?.ApiDependencies["Contoso.Api"]?.AuthorizationRequirements?.ClientIdentifier);
-        Assert.Equal("oauth2", apiManifest?.ApiDependencies["Contoso.Api"]?.AuthorizationRequirements?.Access[0].Type);
+        Assert.Equal("oauth2", apiManifest?.ApiDependencies["Contoso.Api"]?.AuthorizationRequirements?.Access?[0].Type);
     }
 
 }