Merge pull request #174 from microsoft/feature/error-naming

errors naming conventions improvements
microsoft · Feb 9, 2022 · c9f167f · c9f167f
2 parents 820c68e + 1c4e85c
commit c9f167f
Show file tree

Hide file tree

Showing 19 changed files with 166 additions and 151 deletions.
diff --git a/src/Microsoft.OpenApi.OData.Reader/Generator/OpenApiErrorSchemaGenerator.cs b/src/Microsoft.OpenApi.OData.Reader/Generator/OpenApiErrorSchemaGenerator.cs
@@ -17,6 +17,11 @@ namespace Microsoft.OpenApi.OData.Generator
     /// </summary>
     internal static class OpenApiErrorSchemaGenerator
     {
+        internal const string ODataErrorClassName = "ODataError";
+        internal const string MainErrorClassName = "MainError";
+        internal const string ErrorDetailsClassName = "ErrorDetails";
+        internal const string InnerErrorClassName = "InnerError";
+
         /// <summary>
         /// Create the dictionary of <see cref="OpenApiSchema"/> object.
         /// The name of each pair is the namespace-qualified name of the type. It uses the namespace instead of the alias.
@@ -27,28 +32,36 @@ internal static class OpenApiErrorSchemaGenerator
         public static IDictionary<string, OpenApiSchema> CreateODataErrorSchemas(this ODataContext context)
         {
             Utils.CheckArgumentNull(context, nameof(context));
+            var rootNamespaceName = context.GetErrorNamespaceName();
 
             return new Dictionary<string, OpenApiSchema>()
             {
-                // odata.error
-                { "odata.error", CreateErrorSchema() },
-
-                // odata.error.main
-                { "odata.error.main", CreateErrorMainSchema() },
-
-                // odata.error.detail
-                { "odata.error.detail", CreateErrorDetailSchema() },
-
-                // odata.error.innererror
-                { "odata.error.innererror", CreateInnerErrorSchema(context) }
+                { $"{rootNamespaceName}{ODataErrorClassName}", CreateErrorSchema(rootNamespaceName) },
+                { $"{rootNamespaceName}{MainErrorClassName}", CreateErrorMainSchema(rootNamespaceName) },
+                { $"{rootNamespaceName}{ErrorDetailsClassName}", CreateErrorDetailSchema() },
+                { $"{rootNamespaceName}{InnerErrorClassName}", CreateInnerErrorSchema(context) }
             };
         }
 
         /// <summary>
-        /// Create <see cref="OpenApiSchema"/> for "odata.error".
+        /// Gets the error namespace name based on the root namespace of the model.
+        /// </summary>
+        /// <param name="context">The OData to Open API context.</param>
+        /// <returns>The error namespace name.</returns>
+        public static string GetErrorNamespaceName(this ODataContext context) {
+            Utils.CheckArgumentNull(context, nameof(context));
+            var rootNamespaceName = context.Model.DeclaredNamespaces.OrderBy(ns => ns.Count(y => y == '.')).FirstOrDefault();
+            rootNamespaceName += (string.IsNullOrEmpty(rootNamespaceName) ? string.Empty : ".") +
+                                "ODataErrors.";
+            return rootNamespaceName;
+        }
+
+        /// <summary>
+        /// Create <see cref="OpenApiSchema"/> for the error.
         /// </summary>
         /// <returns>The created <see cref="OpenApiSchema"/>.</returns>
-        public static OpenApiSchema CreateErrorSchema()
+        /// <param name="rootNamespaceName">The root namespace name. With a trailing dot.</param>
+        public static OpenApiSchema CreateErrorSchema(string rootNamespaceName)
         {
             return new OpenApiSchema
             {
@@ -66,7 +79,7 @@ public static OpenApiSchema CreateErrorSchema()
                             Reference = new OpenApiReference
                             {
                                 Type = ReferenceType.Schema,
-                                Id = "odata.error.main"
+                                Id = $"{rootNamespaceName}{MainErrorClassName}"
                             }
                         }
                     }
@@ -100,10 +113,11 @@ public static OpenApiSchema CreateInnerErrorSchema(ODataContext context)
         }
 
         /// <summary>
-        /// Create <see cref="OpenApiSchema"/> for "odata.error.main".
+        /// Create <see cref="OpenApiSchema"/> for main property of the error.
         /// </summary>
+        /// <param name="rootNamespaceName">The root namespace name. With a trailing dot.</param>
         /// <returns>The created <see cref="OpenApiSchema"/>.</returns>
-        public static OpenApiSchema CreateErrorMainSchema()
+        public static OpenApiSchema CreateErrorMainSchema(string rootNamespaceName)
         {
             return new OpenApiSchema
             {
@@ -133,7 +147,7 @@ public static OpenApiSchema CreateErrorMainSchema()
                                 Reference = new OpenApiReference
                                 {
                                     Type = ReferenceType.Schema,
-                                    Id = "odata.error.detail"
+                                    Id = $"{rootNamespaceName}{ErrorDetailsClassName}"
                                 }
                             }
                         }
@@ -145,7 +159,7 @@ public static OpenApiSchema CreateErrorMainSchema()
                             Reference = new OpenApiReference
                             {
                                 Type = ReferenceType.Schema,
-                                Id = "odata.error.innererror"
+                                Id = $"{rootNamespaceName}{InnerErrorClassName}"
                             }
                         }
                     }
@@ -154,7 +168,7 @@ public static OpenApiSchema CreateErrorMainSchema()
         }
 
         /// <summary>
-        /// Create <see cref="OpenApiSchema"/> for "odata.error.detail".
+        /// Create <see cref="OpenApiSchema"/> for detail property of the error.
         /// </summary>
         /// <returns>The created <see cref="OpenApiSchema"/>.</returns>
         public static OpenApiSchema CreateErrorDetailSchema()

diff --git a/src/Microsoft.OpenApi.OData.Reader/Generator/OpenApiResponseGenerator.cs b/src/Microsoft.OpenApi.OData.Reader/Generator/OpenApiResponseGenerator.cs
@@ -81,7 +81,7 @@ public static IDictionary<string, OpenApiResponse> CreateResponses(this ODataCon
 
             var responses =  new Dictionary<string, OpenApiResponse>
             {
-                { "error", CreateErrorResponse() }
+                { "error", context.CreateErrorResponse() }
             };
 
             if(context.Settings.EnableDollarCountPath)
@@ -259,8 +259,9 @@ private static OpenApiResponse CreateCountResponse()
             };
         }
 
-        private static OpenApiResponse CreateErrorResponse()
+        private static OpenApiResponse CreateErrorResponse(this ODataContext context)
         {
+            var errorNamespaceName = context.GetErrorNamespaceName();
             return new OpenApiResponse
             {
                 Description = "error",
@@ -275,7 +276,7 @@ private static OpenApiResponse CreateErrorResponse()
                                 Reference = new OpenApiReference
                                 {
                                     Type = ReferenceType.Schema,
-                                    Id = "odata.error"
+                                    Id = $"{errorNamespaceName}{OpenApiErrorSchemaGenerator.ODataErrorClassName}"
                                 }
                             }
                         }

diff --git a/test/Microsoft.OpenAPI.OData.Reader.Tests/Generator/OpenApiResponseGeneratorTests.cs b/test/Microsoft.OpenAPI.OData.Reader.Tests/Generator/OpenApiResponseGeneratorTests.cs
@@ -113,7 +113,7 @@ public void CanSerializeAsJsonFromTheCreatedResponses()
   ""content"": {
     ""application/json"": {
       ""schema"": {
-        ""$ref"": ""#/components/schemas/odata.error""
+        ""$ref"": ""#/components/schemas/ODataErrors.ODataError""
       }
     }
   }

diff --git a/test/Microsoft.OpenAPI.OData.Reader.Tests/Resources/Basic.OpenApi.V2.json b/test/Microsoft.OpenAPI.OData.Reader.Tests/Resources/Basic.OpenApi.V2.json
@@ -1872,18 +1872,18 @@
         }
       }
     },
-    "odata.error": {
+    "DefaultNs.ODataErrors.ODataError": {
       "required": [
         "error"
       ],
       "type": "object",
       "properties": {
         "error": {
-          "$ref": "#/definitions/odata.error.main"
+          "$ref": "#/definitions/DefaultNs.ODataErrors.MainError"
         }
       }
     },
-    "odata.error.main": {
+    "DefaultNs.ODataErrors.MainError": {
       "required": [
         "code",
         "message"
@@ -1902,15 +1902,15 @@
         "details": {
           "type": "array",
           "items": {
-            "$ref": "#/definitions/odata.error.detail"
+            "$ref": "#/definitions/DefaultNs.ODataErrors.ErrorDetails"
           }
         },
         "innererror": {
-          "$ref": "#/definitions/odata.error.innererror"
+          "$ref": "#/definitions/DefaultNs.ODataErrors.InnerError"
         }
       }
     },
-    "odata.error.detail": {
+    "DefaultNs.ODataErrors.ErrorDetails": {
       "required": [
         "code",
         "message"
@@ -1928,7 +1928,7 @@
         }
       }
     },
-    "odata.error.innererror": {
+    "DefaultNs.ODataErrors.InnerError": {
       "description": "The structure of this object is service-specific",
       "type": "object"
     },
@@ -2023,7 +2023,7 @@
     "error": {
       "description": "error",
       "schema": {
-        "$ref": "#/definitions/odata.error"
+        "$ref": "#/definitions/DefaultNs.ODataErrors.ODataError"
       }
     },
     "ODataCountResponse": {

diff --git a/test/Microsoft.OpenAPI.OData.Reader.Tests/Resources/Basic.OpenApi.V2.yaml b/test/Microsoft.OpenAPI.OData.Reader.Tests/Resources/Basic.OpenApi.V2.yaml
@@ -1237,14 +1237,14 @@ definitions:
         '@odata.type': DefaultNs.City
       CountryOrRegion:
         '@odata.type': DefaultNs.CountryOrRegion
-  odata.error:
+  DefaultNs.ODataErrors.ODataError:
     required:
       - error
     type: object
     properties:
       error:
-        $ref: '#/definitions/odata.error.main'
-  odata.error.main:
+        $ref: '#/definitions/DefaultNs.ODataErrors.MainError'
+  DefaultNs.ODataErrors.MainError:
     required:
       - code
       - message
@@ -1259,10 +1259,10 @@ definitions:
       details:
         type: array
         items:
-          $ref: '#/definitions/odata.error.detail'
+          $ref: '#/definitions/DefaultNs.ODataErrors.ErrorDetails'
       innererror:
-        $ref: '#/definitions/odata.error.innererror'
-  odata.error.detail:
+        $ref: '#/definitions/DefaultNs.ODataErrors.InnerError'
+  DefaultNs.ODataErrors.ErrorDetails:
     required:
       - code
       - message
@@ -1274,7 +1274,7 @@ definitions:
         type: string
       target:
         type: string
-  odata.error.innererror:
+  DefaultNs.ODataErrors.InnerError:
     description: The structure of this object is service-specific
     type: object
   ODataCountResponse:
@@ -1344,7 +1344,7 @@ responses:
   error:
     description: error
     schema:
-      $ref: '#/definitions/odata.error'
+      $ref: '#/definitions/DefaultNs.ODataErrors.ODataError'
   ODataCountResponse:
     description: The count of the resource
     schema:

diff --git a/test/Microsoft.OpenAPI.OData.Reader.Tests/Resources/Basic.OpenApi.json b/test/Microsoft.OpenAPI.OData.Reader.Tests/Resources/Basic.OpenApi.json
@@ -2103,18 +2103,18 @@
           }
         }
       },
-      "odata.error": {
+      "DefaultNs.ODataErrors.ODataError": {
         "required": [
           "error"
         ],
         "type": "object",
         "properties": {
           "error": {
-            "$ref": "#/components/schemas/odata.error.main"
+            "$ref": "#/components/schemas/DefaultNs.ODataErrors.MainError"
           }
         }
       },
-      "odata.error.main": {
+      "DefaultNs.ODataErrors.MainError": {
         "required": [
           "code",
           "message"
@@ -2134,15 +2134,15 @@
           "details": {
             "type": "array",
             "items": {
-              "$ref": "#/components/schemas/odata.error.detail"
+              "$ref": "#/components/schemas/DefaultNs.ODataErrors.ErrorDetails"
             }
           },
           "innererror": {
-            "$ref": "#/components/schemas/odata.error.innererror"
+            "$ref": "#/components/schemas/DefaultNs.ODataErrors.InnerError"
           }
         }
       },
-      "odata.error.detail": {
+      "DefaultNs.ODataErrors.ErrorDetails": {
         "required": [
           "code",
           "message"
@@ -2161,7 +2161,7 @@
           }
         }
       },
-      "odata.error.innererror": {
+      "DefaultNs.ODataErrors.InnerError": {
         "type": "object",
         "description": "The structure of this object is service-specific"
       },
@@ -2224,7 +2224,7 @@
         "content": {
           "application/json": {
             "schema": {
-              "$ref": "#/components/schemas/odata.error"
+              "$ref": "#/components/schemas/DefaultNs.ODataErrors.ODataError"
             }
           }
         }

diff --git a/test/Microsoft.OpenAPI.OData.Reader.Tests/Resources/Basic.OpenApi.yaml b/test/Microsoft.OpenAPI.OData.Reader.Tests/Resources/Basic.OpenApi.yaml
@@ -1395,14 +1395,14 @@ components:
           '@odata.type': DefaultNs.City
         CountryOrRegion:
           '@odata.type': DefaultNs.CountryOrRegion
-    odata.error:
+    DefaultNs.ODataErrors.ODataError:
       required:
         - error
       type: object
       properties:
         error:
-          $ref: '#/components/schemas/odata.error.main'
-    odata.error.main:
+          $ref: '#/components/schemas/DefaultNs.ODataErrors.MainError'
+    DefaultNs.ODataErrors.MainError:
       required:
         - code
         - message
@@ -1418,10 +1418,10 @@ components:
         details:
           type: array
           items:
-            $ref: '#/components/schemas/odata.error.detail'
+            $ref: '#/components/schemas/DefaultNs.ODataErrors.ErrorDetails'
         innererror:
-          $ref: '#/components/schemas/odata.error.innererror'
-    odata.error.detail:
+          $ref: '#/components/schemas/DefaultNs.ODataErrors.InnerError'
+    DefaultNs.ODataErrors.ErrorDetails:
       required:
         - code
         - message
@@ -1434,7 +1434,7 @@ components:
         target:
           type: string
           nullable: true
-    odata.error.innererror:
+    DefaultNs.ODataErrors.InnerError:
       type: object
       description: The structure of this object is service-specific
     ODataCountResponse:
@@ -1478,7 +1478,7 @@ components:
       content:
         application/json:
           schema:
-            $ref: '#/components/schemas/odata.error'
+            $ref: '#/components/schemas/DefaultNs.ODataErrors.ODataError'
     ODataCountResponse:
       description: The count of the resource
       content: