chore: reintroduce nonempty object validation (instill-ai#104)

Because - We want to avoid empty objects in our definitions if possible. - New **Connection** section is empty for some connectors. This commit - **chore: reintroduce nonempty object validation** - **fix: remove connection section if empty** --------- Co-authored-by: Chang, Hui-Tang <[email protected]>
namwoam · Apr 25, 2024 · c848e47 · c848e47
1 parent b7d7cd5
commit c848e47
Show file tree

Hide file tree

Showing 10 changed files with 44 additions and 45 deletions.
diff --git a/pkg/base/connector.go b/pkg/base/connector.go
@@ -90,7 +90,7 @@ func (c *BaseConnector) LoadConnectorDefinition(definitionJSONBytes []byte, task
 	if err != nil {
 		return err
 	}
-	renderedTasksJSON, nil := RenderJSON(tasksJSONBytes, additionalJSONBytes)
+	renderedTasksJSON, err := RenderJSON(tasksJSONBytes, additionalJSONBytes)
 	if err != nil {
 		return nil
 	}
@@ -130,6 +130,9 @@ func (c *BaseConnector) LoadConnectorDefinition(definitionJSONBytes []byte, task
 
 	c.definition.Name = fmt.Sprintf("connector-definitions/%s", c.definition.Id)
 	c.definition.Tasks = tasks
+	if c.definition.Spec == nil {
+		c.definition.Spec = &pipelinePB.ConnectorSpec{}
+	}
 	c.definition.Spec.ComponentSpecification, err = generateComponentSpec(c.definition.Title, tasks, taskStructs)
 	if err != nil {
 		return err
@@ -140,15 +143,19 @@ func (c *BaseConnector) LoadConnectorDefinition(definitionJSONBytes []byte, task
 	if err != nil {
 		return err
 	}
-	connection, err := c.refineResourceSpec(raw.Fields["spec"].GetStructValue().Fields["connection_specification"].GetStructValue())
-	if err != nil {
-		return err
+	// TODO: Avoid using structpb traversal here.
+	if _, ok := raw.Fields["spec"]; ok {
+		if v, ok := raw.Fields["spec"].GetStructValue().Fields["connection_specification"]; ok {
+			connection, err := c.refineResourceSpec(v.GetStructValue())
+			if err != nil {
+				return err
+			}
+			connectionPropStruct := &structpb.Struct{Fields: map[string]*structpb.Value{}}
+			connectionPropStruct.Fields["connection"] = structpb.NewStructValue(connection)
+			c.definition.Spec.ComponentSpecification.Fields["properties"] = structpb.NewStructValue(connectionPropStruct)
+		}
 	}
 
-	connectionPropStruct := &structpb.Struct{Fields: map[string]*structpb.Value{}}
-	connectionPropStruct.Fields["connection"] = structpb.NewStructValue(connection)
-	c.definition.Spec.ComponentSpecification.Fields["properties"] = structpb.NewStructValue(connectionPropStruct)
-
 	c.definition.Spec.DataSpecifications, err = generateDataSpecs(taskStructs)
 	if err != nil {
 		return err

diff --git a/pkg/base/operator.go b/pkg/base/operator.go
@@ -76,7 +76,7 @@ func (o *BaseOperator) LoadOperatorDefinition(definitionJSONBytes []byte, tasksJ
 	if err != nil {
 		return err
 	}
-	renderedTasksJSON, nil := RenderJSON(tasksJSONBytes, additionalJSONBytes)
+	renderedTasksJSON, err := RenderJSON(tasksJSONBytes, additionalJSONBytes)
 	if err != nil {
 		return nil
 	}

diff --git a/pkg/connector/instill/v0/README.mdx b/pkg/connector/instill/v0/README.mdx
@@ -28,8 +28,6 @@ It can carry out the following tasks:
 
 The component configuration is defined and maintained [here](https://github.com/instill-ai/component/blob/main/pkg/connector/instill/v0/config/definition.json).
 
-## Connection
-
 ## Supported Tasks
 
 ### Classification

diff --git a/pkg/connector/instill/v0/config/definition.json b/pkg/connector/instill/v0/config/definition.json
@@ -18,15 +18,6 @@
   "icon_url": "",
   "id": "instill-model",
   "public": true,
-  "spec": {
-    "connection_specification": {
-      "$schema": "http://json-schema.org/draft-07/schema#",
-      "additionalProperties": true,
-      "properties": {},
-      "title": "Instill Model Connector",
-      "type": "object"
-    }
-  },
   "title": "Instill Model",
   "description": "Connect the AI models served on the Instill Model Platform",
   "tombstone": false,

diff --git a/pkg/connector/website/v0/README.mdx b/pkg/connector/website/v0/README.mdx
@@ -18,8 +18,6 @@ It can carry out the following tasks:
 
 The component configuration is defined and maintained [here](https://github.com/instill-ai/component/blob/main/pkg/connector/website/v0/config/definition.json).
 
-## Connection
-
 ## Supported Tasks
 
 ### Scrape Website

diff --git a/pkg/connector/website/v0/config/definition.json b/pkg/connector/website/v0/config/definition.json
@@ -8,16 +8,6 @@
   "icon_url": "",
   "id": "website",
   "public": true,
-  "spec": {
-    "connection_specification": {
-      "$schema": "http://json-schema.org/draft-07/schema#",
-      "additionalProperties": true,
-      "properties": {},
-      "required": [],
-      "title": "Website Connector Resource",
-      "type": "object"
-    }
-  },
   "title": "Website",
   "description": "Scrape websites",
   "type": "CONNECTOR_TYPE_DATA",

diff --git a/tools/compogen/pkg/gen/definition_test.go b/tools/compogen/pkg/gen/definition_test.go
@@ -78,6 +78,18 @@ func TestDefinition_Validate(t *testing.T) {
 			},
 			wantErr: "SourceURL field must be a valid URL",
 		},
+		{
+			// This validates the omitnil tag and the nested validation.
+			// Resource specification validation details are covered in a
+			// separate test.
+			name: "nok - resource specification must be valid if present",
+			modifier: func(d *definition) {
+				d.Spec = spec{
+					ConnectionSpecification: &objectSchema{},
+				}
+			},
+			wantErr: "Properties field doesn't reach the minimum value / number of elements",
+		},
 		{
 			name: "nok - multiple errors",
 			modifier: func(d *definition) {

diff --git a/tools/compogen/pkg/gen/resources/templates/readme.mdx.tmpl b/tools/compogen/pkg/gen/resources/templates/readme.mdx.tmpl
@@ -17,20 +17,20 @@ It can carry out the following tasks:
 ## Configuration
 
 The component configuration is defined and maintained [here]({{ .SourceURL }}/config/definition.json).
-{{ if .ComponentType.HasConnectionConfig}}
+{{ if and .ComponentType.HasConnectionConfig ( or .ConnectionConfig.Prerequisites .ConnectionConfig.Properties ) }}
 ## Connection
-{{ if .ConnectionConfig.Prerequisites}}
-<InfoBlock type="info" title="Prerequisites">{{ .ConnectionConfig.Prerequisites }}</InfoBlock>
-{{ end }}{{ if .ConnectionConfig.Properties }}
+{{ with .ConnectionConfig.Prerequisites}}
+<InfoBlock type="info" title="Prerequisites">{{ . }}</InfoBlock>
+{{ end }}{{ with .ConnectionConfig.Properties }}
 | Field | Field ID | Type | Note |
-| :--- | :--- | :--- | :--- |{{ range .ConnectionConfig.Properties }}
+| :--- | :--- | :--- | :--- |{{ range . }}
 | {{ .Title }}{{ if .Required }} (required){{ end }} | `{{ .ID }}` | {{ .Type }} | {{ .Description }} |{{ end }}
 {{ end }}{{ end }}
 ## Supported Tasks{{ range $i, $task := .Tasks}}
 
 ### {{ $task.Title }}
-{{ if $task.Description }}
-{{ $task.Description }}
+{{ with $task.Description }}
+{{ . }}
 {{ end }}{{ if $task.Input }}
 | Input | ID | Type | Description |
 | :--- | :--- | :--- | :--- |

diff --git a/tools/compogen/pkg/gen/schema.go b/tools/compogen/pkg/gen/schema.go
@@ -14,10 +14,6 @@ type property struct {
 }
 
 type objectSchema struct {
-	Required []string `json:"required"`
-
-	// TODO we could validate gt=0 here to avoid empty objects. At this moment
-	// there's a connector (Instill Model) that requires this, but if we
-	// overcome that limitation nonempty objects should be enforced.
-	Properties map[string]property `json:"properties" validate:"dive"`
+	Properties map[string]property `json:"properties" validate:"gt=0,dive"`
+	Required   []string            `json:"required"`
 }
diff --git a/tools/compogen/pkg/gen/schema_test.go b/tools/compogen/pkg/gen/schema_test.go
@@ -44,6 +44,13 @@ func TestObjectSchema_Validate(t *testing.T) {
 		modifier func(*objectSchema)
 		wantErr  string
 	}{
+		{
+			name: "nok - no properties",
+			modifier: func(rs *objectSchema) {
+				rs.Properties = map[string]property{}
+			},
+			wantErr: "Properties field doesn't reach the minimum value / number of elements",
+		},
 		{
 			name: "nok - no title",
 			modifier: func(rs *objectSchema) {