feat: allow mandatory server variables (#341)

- synthesis missing server placeholder variables as strings with no
default values
- treat server placeholder variables with `""` or no default as
mandatory
- refactor to put placeholder extraction into one place

Author: mnahkies

integration-tests/typescript-angular/src/generated/azure-core-data-plane-service.tsp/client.service.ts

@@ -10,7 +10,7 @@ import {Callout} from "nextra/components"
 OpenAPI 3 has a `servers` property that can be used to define the base url for the whole document, or
 specific operations. This guide aims to explain how this is processed.
 
-You can find the specifications definition of the servers object [here](https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.1.md#server-object)
+You can find the specification's definition of the servers object [here](https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.1.md#server-object)
 
 This is fully supported, including placeholder/variable substitution, and overriding.
 
@@ -110,6 +110,20 @@ export class ApiClient {
 As you can see the overrides for each operation are exposed as `ApiClientServers.operations.<operationId>()` following
 the same pattern as the root servers.
 
+## Mandatory Variables
+Sometimes there are variables in urls that you can't provide a reasonable default for. A prime example would be
+an organization slug, eg: `https://api.myOrg.someSaas.com`
+
+Whilst the specification doesn't specifically discuss a concept of mandatory variables, we allow these using one
+of two methods:
+- Providing a default of `""` (as `default` is a required property)
+- Omitting the variable entirely from the `variables` map
+
+A warning will be emitted if you omit the variable entirely, so its suggested you use a default of `""` for
+this scenario.
+
+Regardless, any unconsumed/unused variables will trigger an error with the intention of picking up on typos.
+
 ## Configuration
 This behavior is optional, and be turned off by passing:
 ```

integration-tests/typescript-axios/src/generated/azure-core-data-plane-service.tsp/client.ts

@@ -29,11 +29,12 @@ import type {
   IRServerVariable,
   MaybeIRModel,
 } from "./openapi-types-normalized"
-import {isRef} from "./openapi-utils"
+import {extractPlaceholders, isRef} from "./openapi-utils"
 import {
   camelCase,
   coalesce,
   deepEqual,
+  isDefined,
   isHttpMethod,
   mediaTypeToIdentifier,
 } from "./utils"
@@ -233,20 +234,42 @@ export class Input {
   private normalizeServers(servers: Server[]): IRServer[] {
     return servers
       .filter((it) => it.url)
-      .map((it) => ({
-        url: it.url,
-        description: it.description,
-        variables: Object.fromEntries(
-          Object.entries(it.variables ?? {}).map(([key, value]) => [
-            key,
-            {
-              enum: value.enum ?? [],
-              default: value.default,
-              description: value.description,
-            } satisfies IRServerVariable,
-          ]),
-        ),
-      }))
+      .map((it) => {
+        const variables = Object.entries(it.variables ?? {}).map(
+          ([key, value]): IRServerVariable => ({
+            name: key,
+            enum: value.enum ?? [],
+            default: value.default,
+            description: value.description,
+          }),
+        )
+
+        const placeholders = extractPlaceholders(it.url)
+          .map((it) => it.placeholder)
+          .filter(isDefined)
+
+        for (const placeholder of placeholders) {
+          const variable = variables.find((it) => it.name === placeholder)
+
+          if (!variable) {
+            logger.warn(
+              `missing placeholder variable for server url '${it.url}' - inserting variable of type string with no default`,
+            )
+            variables.push({
+              name: placeholder,
+              default: undefined,
+              enum: [],
+              description: undefined,
+            })
+          }
+        }
+
+        return {
+          url: it.url,
+          description: it.description,
+          variables,
+        }
+      })
   }
 
   private normalizeRequestBodyObject(

integration-tests/typescript-fetch/src/generated/azure-core-data-plane-service.tsp/client.ts

@@ -106,14 +106,13 @@ export type MaybeIRModel = IRModel | IRRef
 export interface IRServer {
   url: string
   description: string | undefined
-  variables: {
-    [k: string]: IRServerVariable
-  }
+  variables: IRServerVariable[]
 }
 
 export interface IRServerVariable {
+  name: string
   enum: string[]
-  default: string
+  default: string | undefined
   description: string | undefined
 }
 

packages/documentation/src/app/guides/concepts/servers-object-handling/page.mdx

@@ -19,6 +19,19 @@ export function getNameFromRef({$ref}: Reference, prefix: string): string {
   return prefix + name.replace(/[-.]+/g, "_")
 }
 
+export function extractPlaceholders(
+  it: string,
+): {wholeString: string; placeholder: string | undefined}[] {
+  const regex = /{([^{}]+)}/g
+
+  return Array.from(it.matchAll(regex)).map((match) => ({
+    // includes the surrounding {}
+    wholeString: match[0],
+    // just the placeholder name
+    placeholder: match[1],
+  }))
+}
+
 export function getTypeNameFromRef(reference: Reference) {
   return getNameFromRef(reference, "t_")
 }

packages/openapi-code-generator/src/core/input.ts

@@ -4,6 +4,7 @@ import type {
   IRParameter,
   MaybeIRModel,
 } from "../../core/openapi-types-normalized"
+import {extractPlaceholders} from "../../core/openapi-utils"
 import {camelCase, isDefined} from "../../core/utils"
 import type {SchemaBuilder} from "../common/schema-builders/schema-builder"
 import type {TypeBuilder} from "../common/type-builder"
@@ -54,32 +55,29 @@ export class ClientOperationBuilder {
 
   routeToTemplateString(paramName = "p"): string {
     const {route, parameters} = this.operation
-    const placeholder = /{([^{}]+)}/g
+    const placeholders = extractPlaceholders(route)
 
-    return Array.from(route.matchAll(placeholder)).reduce((result, match) => {
-      const wholeString = match[0]
-      const placeholderName = match[1]
-
-      if (!placeholderName) {
+    return placeholders.reduce((result, {placeholder, wholeString}) => {
+      if (!placeholder) {
         throw new Error(
-          `invalid route parameter placeholder in route '${placeholder}'`,
+          `invalid route parameter placeholder in route '${route}'`,
         )
       }
 
       const parameter = parameters.find(
-        (it) => it.name === placeholderName && it.in === "path",
+        (it) => it.name === placeholder && it.in === "path",
       )
 
       if (!parameter) {
         throw new Error(
-          `invalid route ${route}. missing path parameter for '${placeholderName}'`,
+          `invalid route ${route}. missing path parameter for '${placeholder}'`,
         )
       }
 
       return result.replace(
         wholeString,
         // TODO: why do we camelCase here? Feels presumptuous
-        `\${${paramName}["${camelCase(placeholderName)}"]}`,
+        `\${${paramName}["${camelCase(placeholder)}"]}`,
       )
     }, route)
   }

packages/openapi-code-generator/src/core/openapi-types-normalized.ts

@@ -64,23 +64,25 @@ describe("typescript/common/client-servers-builder", () => {
       result = await runTest([
         {
           url: "{schema}://unit-tests-default.{tenant}.example.com",
-          variables: {
-            schema: {
+          variables: [
+            {
+              name: "schema",
               default: "https",
               enum: ["https", "http"],
               description: "The protocol to use",
             },
-            tenant: {
+            {
+              name: "tenant",
               default: "example",
               enum: [],
               description: "Your tenant slug",
             },
-          },
+          ],
           description: "Default Unit test server",
         },
         {
           url: "https://unit-tests-other.example.com",
-          variables: {},
+          variables: [],
           description: "Secondary Unit test server",
         },
       ])
@@ -148,12 +150,12 @@ describe("typescript/common/client-servers-builder", () => {
         [
           {
             url: "https://unit-tests-default.example.com",
-            variables: {},
+            variables: [],
             description: "Default Unit test server",
           },
           {
             url: "https://unit-tests-other.example.com",
-            variables: {},
+            variables: [],
             description: "Secondary Unit test server",
           },
         ],
@@ -163,18 +165,20 @@ describe("typescript/common/client-servers-builder", () => {
             servers: [
               {
                 url: "{schema}}://test-operation.{tenant}.example.com",
-                variables: {
-                  schema: {
+                variables: [
+                  {
+                    name: "schema",
                     default: "https",
                     enum: ["https", "http"],
                     description: "The protocol to use",
                   },
-                  tenant: {
+                  {
+                    name: "tenant",
                     default: "example",
                     enum: [],
                     description: "Your tenant slug",
                   },
-                },
+                ],
                 description: "Test operation server",
               },
             ],
@@ -184,7 +188,7 @@ describe("typescript/common/client-servers-builder", () => {
             servers: [
               {
                 url: "https://another-test-operation.example.com",
-                variables: {},
+                variables: [],
                 description: "Another test operation server",
               },
             ],