ignore additionalProperties when are declared as function

[antmendoza]

Many thanks for submitting your Pull Request ❤️!

What this PR does / why we need it:

When I was testing the sdk into other project I realice that the behaviour of the binary (./dist/umd/index.umd.js) was different than the actual source code. The validate function was throwing an error related to have found the "normalize" method as additional property

  {
    instancePath: '',
    schemaPath: '#/additionalProperties',
    keyword: 'additionalProperties',
    params: { additionalProperty: 'normalize' },
    message: 'must NOT have additional properties'
  }

I have changed the way we are deffinig "normalize" method to be able to reproduce the error in test.

Special notes for reviewers:
if you want to reproduce the error I've faced:

• in the sdk project (main branch): install the sdk in your npm: npm run build && cd dist && npm link
• in the project from where you want to test the sdk: install the sdk as a dependency npm link @severlessworkflow/sdk-typescript and create a workflow using builders,

import {
	actionBuilder,
	databasedswitchBuilder,
	defaultdefBuilder,
	functionBuilder,
	functionrefBuilder,
	operationstateBuilder,
	Specification,
	subflowstateBuilder,
	transitiondataconditionBuilder,
	workflowBuilder,
} from '@severlessworkflow/sdk-typescript';


const workflow = workflowBuilder()
	.id('applicantrequest')
	.version('1.0')
	.name('Applicant Request Decision Workflow')
	.description('Determine if applicant request is valid')
	.start('CheckApplication')
	.functions([
		functionBuilder()
			.name('sendRejectionEmailFunction')
			.operation('http://myapis.org/applicationapi.json#emailRejection')
			.build(),
	])
	.states([
		databasedswitchBuilder()
			.name('CheckApplication')
			.dataConditions([
				transitiondataconditionBuilder()
					.condition('${ .applicants | .age >= 18 }')
					.transition('StartApplication')
					.build(),
				transitiondataconditionBuilder()
					.condition('${ .applicants | .age < 18 }')
					.transition('RejectApplication')
					.build(),
			])
			.default(defaultdefBuilder().transition('RejectApplication').build())
			.build(),
		subflowstateBuilder().name('StartApplication').workflowId('startApplicationWorkflowId').build(),
		operationstateBuilder()
			.name('RejectApplication')
			.actionMode('sequential')
			.actions([
				actionBuilder()
					.functionRef(
						functionrefBuilder()
							.refName('sendRejectionEmailFunction')
							.arguments({applicant: '${ .applicant }'})
							.build(),
					)
					.build(),
			])
			.build(),
	])
	.build();

console.log(Specification.Workflow.toYaml(workflow));

you will see an error like the following:

  {
    instancePath: '/dataConditions/0',
    schemaPath: '#/additionalProperties',
    keyword: 'additionalProperties',
    params: { additionalProperty: 'normalize' },
    message: 'must NOT have additional properties'
  },

........                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                    

Error: Databasedswitch is invalid: /dataConditions/0 | #/additionalProperties | must NOT have additional properties
      data: {
    "type": "switch",
    "usedForCompensation": false,
    "name": "CheckApplication",
    "dataConditions": [
        {
            "condition": "${ .applicants | .age >= 18 }",
            "transition": "StartApplication"
        },
        {
            "condition": "${ .applicants | .age < 18 }",
            "transition": "RejectApplication"
        }
    ],
    "default": {
        "transition": "RejectApplication"
    }
}

Additional information (if needed):

[antmendoza]

Hi @JBBianchi , did you have the chance to look into this?

I have been reading through rollup documentation and plugins and I didn't find any option to generate the methods as plain functions instead of functions expressions

[JBBianchi]

@antmendoza I had a look yesterday but couldn't investigate that much. I must admit I don't really see where the difference lies. Maybe in one case the function/method is in the object prototype (no problem) and in another case the function/method is in the object directly (error). One dirty workaround might be to delete normalize() from the clone:

  normalize?(): Databasedswitch {
    const clone = new Databasedswitch(this);
    normalizeUsedForCompensationProperty(clone);
    normalizeOnErrorsProperty(clone);
    normalizeDataConditionsProperty(clone);
    delete clone.normalize;
    return clone;
  }

It also means normalize() will need to be checked before being called as it becomes nullable.

It's just an idea, it's probably not the solution.

[antmendoza]

thank you @JBBianchi ,

I have tested your solution and works fine, just it involves more changes:

• invoke "normalize" in constructors for each property we are "hydrating"
• delete normalize property in the validate method as well.

if you think that this is a better solution I can change the code, no problem

[tsurdilo]

@antmendoza lgtm :)

[JBBianchi]

thank you @JBBianchi ,

I have tested your solution and works fine, just it involves more changes:

* invoke "normalize" in constructors for each property we are "hydrating"

* delete normalize property in the validate method as well.

if you think that this is a better solution I can change the code, no problem

Does the PR actually fixes the problem? I read I have changed the way we are deffinig "normalize" method to be able to reproduce the error in test. and understood it was "only" making the code consistent between dev & published versions.
If the PR fixes the issue, then just ignore what I wrote and merge :)

[antmendoza]

understood it was "only" making the code consistent between dev & published versions

yes, those changes are not really needed. it can be reverted.

Does the PR actually fixes the problem?

Yes as far as I now.

I am using the sdk with this changes into other project and I am not facing the mentioned issue.

Also I have run the index.html example (I have to create a test here) that was having the same problem, since is using the same bundle, and it is working ok now.

[JBBianchi]

understood it was "only" making the code consistent between dev & published versions

yes, those changes are not really needed. it can be reverted.

Does the PR actually fixes the problem?

Yes as far as I now.

I am using the sdk with this changes into other project and I am not facing the mentioned issue.

Also I have run the index.html example (I have to create a test here) that was having the same problem, since is using the same bundle, and it is working ok now.

Ok, sorry I misunderstood. Then all green for me :)
Thanks a lot 👍