DataInputSchema can only be a string and not an object #196

spolti · 2023-11-29T18:28:30Z

fixes #194

Many thanks for submitting your Pull Request ❤️!

What this PR does / why we need it:

Special notes for reviewers:

Additional information (if needed):

spolti · 2023-11-29T18:28:59Z

@ricardozanini PTAL.

there is one test specific that I left a comment, we would need to revisit that later.

model/workflow.go

model/workflow_validator_test.go

ricardozanini · 2023-12-04T14:42:23Z

@ribeiromiranda wanna take a look?

model/workflow.go

parser/parser_test.go

model/workflow_test.go

ricardozanini · 2023-12-18T15:17:45Z

@spolti can you rebase and review @ribeiromiranda's comment?

github-actions · 2024-02-02T00:04:19Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

ricardozanini · 2024-02-02T12:25:59Z

@spolti are you still on this? :D

spolti · 2024-02-08T12:31:20Z

Yes, sry, will try to revisit it this or next week.

github-actions · 2024-03-25T00:04:49Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

spolti · 2024-04-03T14:26:09Z

@ribeiromiranda hi,
Just got it using your tool, the version in the go.mod does not looks correct.
https://github.com/galgotech/builder-gen/blob/main/go.mod#L3C1-L3C11?

go: github.com/galgotech/builder-gen@latest (in github.com/galgotech/builder-gen@v0.0.0-20231121195112-e44977c5b75f): go.mod:3: invalid go version '1.20.11': must match format 1.23

Makefile

spolti · 2024-04-03T19:17:55Z

There is a small issue on a third party lib, please take a look in the comments.

ricardozanini

@ribeiromiranda can you downgrade your library's Go version?

ribeiromiranda · 2024-04-05T01:34:29Z

There is a small issue on a third party lib, please take a look in the comments.

Changed version to 1.19

ricardozanini · 2024-04-05T12:42:45Z

@spolti ^

spolti · 2024-04-05T13:58:32Z

updated. Thanks @ribeiromiranda

ricardozanini · 2024-04-05T15:05:46Z

@ribeiromiranda can you please review?

ribeiromiranda · 2024-04-06T03:22:33Z

@ricardozanini @spolti Are all inputs valid?

id: Valid DataInputSchema
version: '1.0'
specVersion: '0.8'
start: Start
dataInputSchema: "file://testdata/datainputschema.json"
...

id: Valid DataInputSchema
version: '1.0'
specVersion: '0.8'
start: Start
dataInputSchema: "{}"
...

id: Valid DataInputSchema
version: '1.0'
specVersion: '0.8'
start: Start
dataInputSchema:
  schema: "file://testdata/datainputschema.json"
...

id: Valid DataInputSchema
version: '1.0'
specVersion: '0.8'
start: Start
dataInputSchema:
  schema: "{}"
...

ricardozanini · 2024-04-08T18:11:16Z

@ribeiromiranda yes, it should.

spolti · 2024-04-25T18:43:08Z

Option 2 should be invalid, since we check the schema field

if d.Schema != nil && d.Schema.Type == String {

spolti · 2024-04-25T18:43:14Z

are we clear to merge this one?

ribeiromiranda · 2024-04-30T02:02:10Z

@ricardozanini @spolti
Another question, it wasn't clear to me in the specification.

Which is right, JSON with or without quotes?

1.
```yaml
id: Valid DataInputSchema
version: '1.0'
specVersion: '0.8'
start: Start
dataInputSchema: "{ \"test\": \"value\" }"
...

2.
```yaml
id: Valid DataInputSchema
version: '1.0'
specVersion: '0.8'
start: Start
dataInputSchema: {
  "test": "value"
}
...

ribeiromiranda · 2024-05-04T17:05:38Z

@ricardozanini @spolti With the patch all cases below will work.

dataInputSchema: "{\"key\": \"value\"}"
dataInputSchema: {"key": "value"}
dataInputSchema: file://...
dataInputSchema:
  schema: "{\"key\": \"value\"}"
  failOnValidationErrors: true
dataInputSchema:
  schema: {"key": "value"}
  failOnValidationErrors: true
dataInputSchema:
  schema: file://...
  failOnValidationErrors: true

diff --git a/model/workflow.go b/model/workflow.go
index d0ac4c7..aa72d1f 100644
--- a/model/workflow.go
+++ b/model/workflow.go
@@ -15,8 +15,9 @@
 package model
 
 import (
+	"bytes"
 	"encoding/json"
-	"regexp"
+	"errors"
 
 	"github.com/serverlessworkflow/sdk-go/v2/util"
 )
@@ -522,33 +523,41 @@ type dataInputSchemaUnmarshal DataInputSchema
 // UnmarshalJSON implements json.Unmarshaler
 func (d *DataInputSchema) UnmarshalJSON(data []byte) error {
 	d.ApplyDefault()
-	err := util.UnmarshalObject("dataInputSchema", data, (*dataInputSchemaUnmarshal)(d))
+
+	// expected: data = "{\"key\": \"value\"}"
+	//           data = {"key": "value"}
+	//	         data = "file://..."
+	//           data = { "schema": "{\"key\": \"value\"}", "failOnValidationErrors": true }
+	//           data = { "schema": {"key": "value"}, "failOnValidationErrors": true }
+	//           data = { "schema": "file://...", "failOnValidationErrors": true }
+
+	schemaString := ""
+	err := util.UnmarshalPrimitiveOrObject("dataInputSchema", data, &schemaString, (*dataInputSchemaUnmarshal)(d))
 	if err != nil {
 		return err
 	}
 
-	if d.Schema != nil && d.Schema.Type == String {
-		// Define the regex pattern to match the prefixes
-		pattern := `^(http|https|file)`
-		regex := regexp.MustCompile(pattern)
-		// if it is not external, treat as JSON object
-		if !regex.MatchString(d.Schema.StringValue) {
-			point := FromString(d.Schema.StringValue)
-			d.Schema = &point
+	if d.Schema != nil {
+		if d.Schema.Type == Map {
 			return nil
-		}
 
-		data, err := util.LoadExternalResource(d.Schema.StringValue)
-		if err != nil {
-			return err
+		} else if d.Schema.Type == String {
+			schemaString = d.Schema.StringValue
+
+		} else {
+			return errors.New("invalid dataInputSchema must be a string or object")
 		}
+	}
 
-		er := util.UnmarshalObject("schema", data, &d.Schema)
-		// clean the string value to avoid the json URI being appended
-		d.Schema.StringValue = ""
-		return er
+	if schemaString != "" {
+		data = []byte(schemaString)
+		if bytes.TrimSpace(data)[0] != '{' {
+			data = []byte("\"" + schemaString + "\"")
+		}
 	}
-	return nil
+
+	d.Schema = new(Object)
+	return util.UnmarshalObjectOrFile("schema", data, &d.Schema)
 }
 
 // ApplyDefault set the default values for Data Input Schema
diff --git a/model/workflow_test.go b/model/workflow_test.go
index 70aa2d8..a5aa42a 100644
--- a/model/workflow_test.go
+++ b/model/workflow_test.go
@@ -499,7 +499,11 @@ func TestTransitionUnmarshalJSON(t *testing.T) {
 
 func TestDataInputSchemaUnmarshalJSON(t *testing.T) {
 
-	schemaName := FromString("schema name")
+	var schemaName Object
+	err := json.Unmarshal([]byte("{\"key\": \"value\"}"), &schemaName)
+	if !assert.NoError(t, err) {
+		return
+	}
 
 	type testCase struct {
 		desp   string
@@ -511,16 +515,25 @@ func TestDataInputSchemaUnmarshalJSON(t *testing.T) {
 	testCases := []testCase{
 		{
 			desp: "string success",
-			data: `"schema name"`,
+			data: "{\"key\": \"value\"}",
 			expect: DataInputSchema{
 				Schema:                 &schemaName,
 				FailOnValidationErrors: true,
 			},
-			err: `dataInputSchema must be string or object`,
+			err: ``,
 		},
 		{
-			desp: `object success`,
-			data: `{"schema": "schema name"}`,
+			desp: "string fail",
+			data: "{\"key\": }",
+			expect: DataInputSchema{
+				Schema:                 &schemaName,
+				FailOnValidationErrors: true,
+			},
+			err: `invalid character '}' looking for beginning of value`,
+		},
+		{
+			desp: `object success (without quotes)`,
+			data: `{"key": "value"}`,
 			expect: DataInputSchema{
 				Schema:                 &schemaName,
 				FailOnValidationErrors: true,
@@ -528,22 +541,32 @@ func TestDataInputSchemaUnmarshalJSON(t *testing.T) {
 			err: ``,
 		},
 		{
-			desp: `object fail`,
-			data: `{"schema": "schema name}`,
+			desp: `schema object success`,
+			data: `{"schema": "{\"key\": \"value\"}"}`,
 			expect: DataInputSchema{
 				Schema:                 &schemaName,
 				FailOnValidationErrors: true,
 			},
-			err: `unexpected end of JSON input`,
+			err: ``,
 		},
 		{
-			desp: `object key invalid`,
-			data: `{"schema_invalid": "schema name"}`,
+			desp: `schema object success (without quotes)`,
+			data: `{"schema": {"key": "value"}}`,
 			expect: DataInputSchema{
+				Schema:                 &schemaName,
 				FailOnValidationErrors: true,
 			},
 			err: ``,
 		},
+		{
+			desp: `schema object fail`,
+			data: `{"schema": "schema name}`,
+			expect: DataInputSchema{
+				Schema:                 &schemaName,
+				FailOnValidationErrors: true,
+			},
+			err: `unexpected end of JSON input`,
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.desp, func(t *testing.T) {
@@ -551,13 +574,14 @@ func TestDataInputSchemaUnmarshalJSON(t *testing.T) {
 			err := json.Unmarshal([]byte(tc.data), &v)
 
 			if tc.err != "" {
-				assert.Error(t, err)
-				assert.Regexp(t, tc.err, err)
+				assert.Error(t, err, tc.desp)
+				assert.Regexp(t, tc.err, err, tc.desp)
 				return
 			}
 
-			assert.NoError(t, err)
-			assert.Equal(t, tc.expect, v)
+			assert.NoError(t, err, tc.desp)
+			assert.Equal(t, tc.expect.Schema, v.Schema, tc.desp)
+			assert.Equal(t, tc.expect.FailOnValidationErrors, v.FailOnValidationErrors, tc.desp)
 		})
 	}
 }
diff --git a/util/unmarshal.go b/util/unmarshal.go
index 9d5b1c2..d00e9d2 100644
--- a/util/unmarshal.go
+++ b/util/unmarshal.go
@@ -221,7 +221,7 @@ func UnmarshalObjectOrFile[U any](parameterName string, data []byte, valObject *
 	}
 
 	data = bytes.TrimSpace(data)
-	if data[0] == '{' && parameterName != "constants" && parameterName != "timeouts" {
+	if data[0] == '{' && parameterName != "constants" && parameterName != "timeouts" && parameterName != "schema" {
 		extractData := map[string]json.RawMessage{}
 		err = json.Unmarshal(data, &extractData)
 		if err != nil {

ricardozanini · 2024-05-08T13:13:24Z

@ricardozanini @spolti Another question, it wasn't clear to me in the specification.

Which is right, JSON with or without quotes?
1.
```yaml
id: Valid DataInputSchema
version: '1.0'
specVersion: '0.8'
start: Start
dataInputSchema: "{ \"test\": \"value\" }"
...
2.
```yaml
id: Valid DataInputSchema
version: '1.0'
specVersion: '0.8'
start: Start
dataInputSchema: {
  "test": "value"
}
...

If we can convert the { "test": "value" } into a valid JSON Schema, both are.

ricardozanini · 2024-05-08T13:13:52Z

@spolti can you apply @ribeiromiranda's patch?

fixes serverlessworkflow#194 Signed-off-by: Spolti <filippespolti@gmail.com>

Signed-off-by: Spolti <filippespolti@gmail.com>

spolti · 2024-05-08T14:33:19Z

Done.

Thanks @ribeiromiranda

spolti · 2024-05-08T14:36:00Z

Codecov is complaining about rate limit:
Rate limit reached. Please upload with the Codecov repository upload token to resolve issue.

ricardozanini · 2024-05-08T16:43:14Z

@spolti I'll merge and solve this issue later.

spolti requested a review from ricardozanini as a code owner November 29, 2023 18:28

github-actions bot added the source label Nov 29, 2023

spolti force-pushed the issue194 branch from 190bf31 to 0ba22a1 Compare November 29, 2023 18:31

ricardozanini reviewed Nov 30, 2023

View reviewed changes

model/workflow.go Outdated Show resolved Hide resolved

ricardozanini approved these changes Dec 1, 2023

View reviewed changes

model/workflow_validator_test.go Outdated Show resolved Hide resolved

ricardozanini requested a review from ribeiromiranda December 4, 2023 14:42

ribeiromiranda suggested changes Dec 6, 2023

View reviewed changes

model/workflow.go Outdated Show resolved Hide resolved

parser/parser_test.go Outdated Show resolved Hide resolved

model/workflow_test.go Outdated Show resolved Hide resolved

github-actions bot added the Stale PR label Feb 2, 2024

ricardozanini removed the Stale PR label Feb 8, 2024

github-actions bot added the Stale PR label Mar 25, 2024

ricardozanini removed the Stale PR label Mar 25, 2024

spolti force-pushed the issue194 branch from 8f72198 to 54ba2c4 Compare April 3, 2024 14:17

spolti commented Apr 3, 2024

View reviewed changes

Makefile Outdated Show resolved Hide resolved

spolti requested review from ricardozanini and ribeiromiranda April 3, 2024 19:17

spolti force-pushed the issue194 branch from 5870cde to 5e8f031 Compare April 3, 2024 19:18

ricardozanini reviewed Apr 4, 2024

View reviewed changes

spolti force-pushed the issue194 branch from 5e8f031 to 3c3f359 Compare April 5, 2024 13:58

spolti force-pushed the issue194 branch from 3c3f359 to 55817c0 Compare April 5, 2024 17:35

spolti added 5 commits May 8, 2024 11:28

DataInputSchema can only be a string and not an object

37e7c15

fixes serverlessworkflow#194 Signed-off-by: Spolti <filippespolti@gmail.com>

use dataInputSchema.schema as pointer

6c6dca2

Signed-off-by: Spolti <filippespolti@gmail.com>

remove comment

20441b6

Signed-off-by: Spolti <filippespolti@gmail.com>

review additions

68dccba

Signed-off-by: Spolti <filippespolti@gmail.com>

aaplying @ribeiromiranda's patch

554918b

Signed-off-by: Spolti <filippespolti@gmail.com>

spolti force-pushed the issue194 branch from 55817c0 to 554918b Compare May 8, 2024 14:32

ricardozanini merged commit bb89ed0 into serverlessworkflow:main May 8, 2024
5 of 6 checks passed

spolti deleted the issue194 branch May 9, 2024 14:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DataInputSchema can only be a string and not an object #196

DataInputSchema can only be a string and not an object #196

spolti commented Nov 29, 2023

spolti commented Nov 29, 2023 •

edited

Loading

ricardozanini commented Dec 4, 2023

ricardozanini commented Dec 18, 2023

github-actions bot commented Feb 2, 2024

ricardozanini commented Feb 2, 2024

spolti commented Feb 8, 2024

github-actions bot commented Mar 25, 2024

spolti commented Apr 3, 2024

spolti commented Apr 3, 2024

ricardozanini left a comment

ribeiromiranda commented Apr 5, 2024

ricardozanini commented Apr 5, 2024

spolti commented Apr 5, 2024

ricardozanini commented Apr 5, 2024

ribeiromiranda commented Apr 6, 2024 •

edited

Loading

ricardozanini commented Apr 8, 2024

spolti commented Apr 25, 2024

spolti commented Apr 25, 2024

ribeiromiranda commented Apr 30, 2024

ribeiromiranda commented May 4, 2024 •

edited

Loading

ricardozanini commented May 8, 2024

ricardozanini commented May 8, 2024

spolti commented May 8, 2024

spolti commented May 8, 2024

ricardozanini commented May 8, 2024

DataInputSchema can only be a string and not an object #196

DataInputSchema can only be a string and not an object #196

Conversation

spolti commented Nov 29, 2023

spolti commented Nov 29, 2023 • edited Loading

ricardozanini commented Dec 4, 2023

ricardozanini commented Dec 18, 2023

github-actions bot commented Feb 2, 2024

ricardozanini commented Feb 2, 2024

spolti commented Feb 8, 2024

github-actions bot commented Mar 25, 2024

spolti commented Apr 3, 2024

spolti commented Apr 3, 2024

ricardozanini left a comment

Choose a reason for hiding this comment

ribeiromiranda commented Apr 5, 2024

ricardozanini commented Apr 5, 2024

spolti commented Apr 5, 2024

ricardozanini commented Apr 5, 2024

ribeiromiranda commented Apr 6, 2024 • edited Loading

ricardozanini commented Apr 8, 2024

spolti commented Apr 25, 2024

spolti commented Apr 25, 2024

ribeiromiranda commented Apr 30, 2024

ribeiromiranda commented May 4, 2024 • edited Loading

ricardozanini commented May 8, 2024

ricardozanini commented May 8, 2024

spolti commented May 8, 2024

spolti commented May 8, 2024

ricardozanini commented May 8, 2024

spolti commented Nov 29, 2023 •

edited

Loading

ribeiromiranda commented Apr 6, 2024 •

edited

Loading

ribeiromiranda commented May 4, 2024 •

edited

Loading