fix: map required create-path parameters to Required attributes

DESIGN.md

@@ -51,6 +51,8 @@ In these OAS operations, the generator will search the `create` and `read` for s
 4. `read` operation: [parameters](https://spec.openapis.org/oas/v3.1.0#parameterObject)
     - The generator will merge all `query` and `path` parameters to the root of the schema.
     - The generator will consider as parameters the ones in the [OAS Path Item](https://spec.openapis.org/oas/v3.1.0#path-item-object) and the ones in the [OAS Operation](https://spec.openapis.org/oas/v3.1.0#operation-object), merged based on the rules in the specification
+    - Two parameters that map to the same attribute name, like a `path` and a `query` parameter both named `zone`, or a parameter aliased onto another's name, cannot both be kept, and neither can win without silently discarding the other. The resource is skipped with a warning, and the specification or the aliases must be fixed.
+    - The `create` operation's `path` parameters are read to decide whether a `read` operation `path` parameter is a practitioner input, as described in [Resources - Required, Computed or Optional](#resources---required-computed-or-optional). They are not themselves mapped to attributes.
 
 All schemas found will be deep merged together, with the `requestBody` schema from the `create` operation being the **main schema** that the others will be merged on top. The deep merge has the following characteristics:
 
@@ -74,6 +76,7 @@ The generator uses the `read` operation to map to the provider code specificatio
 1. `read` operation: [parameters](https://spec.openapis.org/oas/v3.1.0#parameterObject)
     - The generator will merge all `query` and `path` parameters to the root of the schema.
     - The generator will consider as parameters the ones in the [Path Item Object](https://spec.openapis.org/oas/v3.1.0#path-item-object) and the ones in the [Operation Object](https://spec.openapis.org/oas/v3.1.0#operation-object), merged based on the rules in the specification
+    - Two parameters that map to the same attribute name, like a `path` and a `query` parameter both named `zone`, or a parameter aliased onto another's name, cannot both be kept, and neither can win without silently discarding the other. The data source is skipped with a warning, and the specification or the aliases must be fixed.
 2. `read` operation: response body in [responses](https://spec.openapis.org/oas/v3.1.0#responsesObject)
     - The response body is the only schema **required** for data sources. If not found, the generator will skip the data source without mapping.
     - Will attempt to use `200` or `201` response body. If not found, will grab the first available `2xx` response code with a schema (lexicographic order)
@@ -215,6 +218,12 @@ If not required, then the field will be mapped as `computed_optional`.
 
 If the field is only present in a schema other than the `create` operation `requestBody`, then the field will be mapped as `computed`.
 
+A `read` operation `path` parameter marked as `required` that also appears as a `path` parameter of the `create` operation will be mapped as `required`. Such a parameter is addressed by the URL the `create` operation is sent to, so the practitioner has to supply it. Any other `read` operation parameter will be mapped as `computed_optional`, including a `required` `path` parameter that only appears on the `read` path -- that one identifies something the API assigned.
+
+`required` on a parameter is mandatory for `path` parameters, so it cannot distinguish the two cases on its own. On `/parents/{parent_id}/children/{child_id}`, created with `POST /parents/{parent_id}/children`, `parent_id` is `required` and `child_id` is `computed_optional`.
+
+A `required` mapped from a parameter also takes precedence over any other classification of the same field name, even though the parameters are otherwise the lowest precedence schema. That covers a `computed` mapped from a response body as well as a `computed_optional` mapped from a create request body property that is not `required`: a value addressed by the create URL has to be supplied either way, so discarding the parameter's `required` would leave an attribute the API needs as an input mapped as something the practitioner may omit.
+
 #### Data Sources - Required, Computed or Optional
 For data sources, all fields in the `read` operation `parameters` OAS schema marked as [required](https://json-schema.org/understanding-json-schema/reference/object.html#required-properties) will be mapped as `required`.
 

internal/cmd/testdata/edgecase/generator_config.yml

@@ -33,6 +33,14 @@ resources:
       path: /composition_siblings
       method: GET
 
+  nested_path:
+    create:
+      path: /parents/{parent_id}/children
+      method: POST
+    read:
+      path: /parents/{parent_id}/children/{child_id}
+      method: GET
+
 data_sources:
   nested_collections:
     read:

internal/cmd/testdata/edgecase/openapi_spec.yml

@@ -199,8 +199,75 @@ paths:
                       deprecated: true
                   allOf:
                   - $ref: "#/components/schemas/composition_refinable_object"
+  /parents/{parent_id}/children:
+    # Declared on the path item rather than the operation, so that resolving the create
+    # operation's path parameters has to merge the path item in.
+    parameters:
+    - name: parent_id
+      in: path
+      description: The parent this child hangs off, supplied by the practitioner
+      required: true
+      schema:
+        type: string
+    post:
+      summary: Create a child under a parent addressed only by the URL
+      requestBody:
+        required: true
+        content:
+          application/json:
+            schema:
+              type: object
+              required:
+              - name
+              properties:
+                name:
+                  description: The child's name
+                  type: string
+      responses:
+        "201":
+          description: Created
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/child_object"
+  /parents/{parent_id}/children/{child_id}:
+    get:
+      summary: Both path parameters are required, but only parent_id is a practitioner input
+      parameters:
+      - name: parent_id
+        in: path
+        description: The parent this child hangs off, supplied by the practitioner
+        required: true
+        schema:
+          type: string
+      - name: child_id
+        in: path
+        description: The child's identifier, assigned by the API
+        required: true
+        schema:
+          type: string
+      responses:
+        "200":
+          description: OK
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/child_object"
 components:
   schemas:
+    child_object:
+      description: Echoes both path parameters back, so the response reaches the attributes first
+      type: object
+      properties:
+        parent_id:
+          description: The response is merged first, so this description outlives the parameter's
+          type: string
+        child_id:
+          description: The response is merged first, so this description outlives the parameter's
+          type: string
+        name:
+          description: The child's name
+          type: string
     composition_object:
       description: This description loses to the composing schema's
       type: object

internal/cmd/testdata/edgecase/provider_code_spec.json

@@ -532,6 +532,34 @@
 				]
 			}
 		},
+		{
+			"name": "nested_path",
+			"schema": {
+				"attributes": [
+					{
+						"name": "name",
+						"string": {
+							"computed_optional_required": "required",
+							"description": "The child's name"
+						}
+					},
+					{
+						"name": "child_id",
+						"string": {
+							"computed_optional_required": "computed",
+							"description": "The response is merged first, so this description outlives the parameter's"
+						}
+					},
+					{
+						"name": "parent_id",
+						"string": {
+							"computed_optional_required": "required",
+							"description": "The response is merged first, so this description outlives the parameter's"
+						}
+					}
+				]
+			}
+		},
 		{
 			"name": "set_test",
 			"schema": {

internal/cmd/testdata/kubernetes/provider_code_spec.json

@@ -9253,7 +9253,7 @@
 					{
 						"name": "namespace",
 						"string": {
-							"computed_optional_required": "computed_optional",
+							"computed_optional_required": "required",
 							"description": "object name and auth scope, such as for teams and projects"
 						}
 					},

internal/cmd/testdata/scaleway/provider_code_spec.json

@@ -161,7 +161,7 @@
 					{
 						"name": "zone",
 						"string": {
-							"computed_optional_required": "computed_optional",
+							"computed_optional_required": "required",
 							"description": "The zone you want to target",
 							"validators": [
 								{
@@ -302,7 +302,7 @@
 					{
 						"name": "zone",
 						"string": {
-							"computed_optional_required": "computed_optional",
+							"computed_optional_required": "required",
 							"description": "The zone you want to target",
 							"validators": [
 								{

internal/explorer/config_explorer.go

@@ -87,14 +87,21 @@ func (e configExplorer) FindResources() (map[string]Resource, error) {
 			continue
 		}
 
+		createCommonParameters, err := extractCommonParameters(e.spec.Paths, resourceConfig.Create.Path)
+		if err != nil {
+			errResult = errors.Join(errResult, fmt.Errorf("failed to extract '%s' create common parameters: %w", name, err))
+			continue
+		}
+
 		resources[name] = Resource{
-			CreateOp:         createOp,
-			ReadOp:           readOp,
-			UpdateOp:         updateOp,
-			DeleteOp:         deleteOp,
-			CommonParameters: commonParameters,
-			SchemaOptions:    extractSchemaOptions(resourceConfig.SchemaOptions),
-			Description:      getOperationTagDescription(e.spec, createOp),
+			CreateOp:               createOp,
+			ReadOp:                 readOp,
+			UpdateOp:               updateOp,
+			DeleteOp:               deleteOp,
+			CommonParameters:       commonParameters,
+			CreateCommonParameters: createCommonParameters,
+			SchemaOptions:          extractSchemaOptions(resourceConfig.SchemaOptions),
+			Description:            getOperationTagDescription(e.spec, createOp),
 		}
 	}
 

internal/explorer/config_explorer_test.go

@@ -174,6 +174,67 @@ func Test_ConfigExplorer_FindResources(t *testing.T) {
 				},
 			},
 		},
+		"path item parameters are extracted per operation path": {
+			config: config.Config{
+				Resources: map[string]config.Resource{
+					"test_resource": {
+						Create: &config.OpenApiSpecLocation{
+							Path:   "/parents/{parent_id}/resources",
+							Method: "POST",
+						},
+						Read: &config.OpenApiSpecLocation{
+							Path:   "/parents/{parent_id}/resources/{resource_id}",
+							Method: "GET",
+						},
+					},
+				},
+			},
+			pathItems: orderedmap.ToOrderedMap(map[string]*high.PathItem{
+				"/parents/{parent_id}/resources": {
+					Parameters: []*high.Parameter{
+						{Name: "parent_id", In: "path", Required: new(true)},
+					},
+					Post: &high.Operation{
+						Description: "create op here",
+						OperationId: "create_resource",
+					},
+				},
+				"/parents/{parent_id}/resources/{resource_id}": {
+					Parameters: []*high.Parameter{
+						{Name: "parent_id", In: "path", Required: new(true)},
+						{Name: "resource_id", In: "path", Required: new(true)},
+					},
+					Get: &high.Operation{
+						Description: "read op here",
+						OperationId: "read_resource",
+					},
+				},
+			}),
+			want: map[string]explorer.Resource{
+				"test_resource": {
+					CreateOp: &high.Operation{
+						Description: "create op here",
+						OperationId: "create_resource",
+					},
+					ReadOp: &high.Operation{
+						Description: "read op here",
+						OperationId: "read_resource",
+					},
+					CommonParameters: []*high.Parameter{
+						{Name: "parent_id", In: "path", Required: new(true)},
+						{Name: "resource_id", In: "path", Required: new(true)},
+					},
+					CreateCommonParameters: []*high.Parameter{
+						{Name: "parent_id", In: "path", Required: new(true)},
+					},
+					SchemaOptions: explorer.SchemaOptions{
+						AttributeOptions: explorer.AttributeOptions{
+							Overrides: map[string]explorer.Override{},
+						},
+					},
+				},
+			},
+		},
 		"non-existent create path throws error": {
 			config: config.Config{
 				Resources: map[string]config.Resource{
@@ -341,7 +402,7 @@ func Test_ConfigExplorer_FindResources(t *testing.T) {
 				return
 			}
 
-			if diff := cmp.Diff(got, testCase.want, cmpopts.IgnoreUnexported(high.Operation{})); testCase.expectedErr == nil && diff != "" {
+			if diff := cmp.Diff(got, testCase.want, cmpopts.IgnoreUnexported(high.Operation{}, high.Parameter{})); testCase.expectedErr == nil && diff != "" {
 				t.Errorf("unexpected difference: %s", diff)
 			}
 		})

internal/explorer/explorer.go

@@ -17,13 +17,16 @@ type Explorer interface {
 
 // Resource contains CRUD operations and schema options for configuration.
 type Resource struct {
-	CreateOp         *high.Operation
-	ReadOp           *high.Operation
-	UpdateOp         *high.Operation
-	DeleteOp         *high.Operation
+	CreateOp *high.Operation
+	ReadOp   *high.Operation
+	UpdateOp *high.Operation
+	DeleteOp *high.Operation
+	// CommonParameters are the path item parameters of the read path.
 	CommonParameters []*high.Parameter
-	SchemaOptions    SchemaOptions
-	Description      string // Resource-level description from OpenAPI tags
+	// CreateCommonParameters are the path item parameters of the create path.
+	CreateCommonParameters []*high.Parameter
+	SchemaOptions          SchemaOptions
+	Description            string // Resource-level description from OpenAPI tags
 }
 
 // DataSource contains a Read operation and schema options for configuration.

internal/explorer/explorer_utils.go

@@ -12,7 +12,11 @@ func mergeParameters(commonParameters []*high.Parameter, operation *high.Operati
 		for _, operationParameter := range operation.Parameters {
 			found := false
 			for i, mergedParameter := range mergedParameters {
-				if operationParameter.Name == mergedParameter.Name {
+				// A parameter is identified by name and location, so an operation parameter only
+				// overrides a path item parameter when both match. Matching on the name alone
+				// would let, say, an operation's `zone` query parameter displace the path item's
+				// `zone` path parameter, which the specification treats as a different parameter.
+				if operationParameter.Name == mergedParameter.Name && operationParameter.In == mergedParameter.In {
 					found = true
 					mergedParameters[i] = operationParameter
 					break
@@ -30,6 +34,10 @@ func (e *Resource) ReadOpParameters() []*high.Parameter {
 	return mergeParameters(e.CommonParameters, e.ReadOp)
 }
 
+func (e *Resource) CreateOpParameters() []*high.Parameter {
+	return mergeParameters(e.CreateCommonParameters, e.CreateOp)
+}
+
 func (e *DataSource) ReadOpParameters() []*high.Parameter {
 	return mergeParameters(e.CommonParameters, e.ReadOp)
 }