Repository: lukeautry/tsoa (ts)

Issue:
validateUnion overwriting params with undefined
## Sorting

* **I'm submitting a ...**
  - [x] bug report
  - [ ] feature request
  - [ ] support request

* I confirm that I
  - [x] used the [search](https://github.com/lukeautry/tsoa/search?type=Issues) to make sure that a similar issue hasn't already been submit

## Expected Behavior
Trying to validate an array of a union of discriminated types results in only the discriminator of the last subschema to be generated to be correctly passed.

```ts
interface PetStore {
   animals: Array<Cat | Dog>;
}
interface Cat {
   type: 'cat';
   height: number;
}
interface Dog {
   type: 'dog';
   weight: number;
}
```

The following JSON object should be correctly passed to the controller method:

```json
{
   "animals": [
      { "type": "cat", "height": 100 },
      { "type": "dog", "weight": 20 }
   ]
}
```

## Current Behavior
What currently happens is the `type` property is overwritten by `undefined` unless its value is that of the last subschema passed of the union. The actual object passed to the controller method:

```ts
{
   "animals": [
      { "type": undefined, "height": 100 },
      { "type": "dog", "weight": 20 }
   ]
}
```

## Possible Solution
I've already written a PR ready to fix this that seems to pass all the tests except one that I've adapted since it makes sense either way.

The changes are here https://github.com/Passiverecords/tsoa/commit/d03e401066a16eb8e52ea9b36c8f216667785a87

## Context (Environment)
Version of the library: 2.5.7
Version of NodeJS: 12.7.0
* Confirm you were using yarn not npm: [x]

## Breaking change?
I am not 100% what implications the changes reflected by the test have... should values that do not exist be explicitly passed as undefined? As far as JS goes, I assume it shouldn't impact anyone.

Pull request:
Fix validateUnion overwriting params with undefined
### All Submissions:

* [x] Have you followed the guidelines in our [Contributing](https://github.com/lukeautry/tsoa/tree/master/docs/CONTRIBUTING.md) document?
* [x] Have you checked to ensure there aren't other open [Pull Requests](https://github.com/lukeautry/tsoa/pulls) for the same update/change?
* [ ] Have you written unit tests?
* [ ] Have you written unit tests that cover the negative cases (i.e.: if bad data is submitted, does the library respond properly)?
* [x] This PR is associated with an existing issue?

**Closing issues**

Close #520 

### If this is a new feature submission:

* [x] Has the issue had a maintainer respond to the issue and clarify that the feature is something that aligns with the [goals](https://github.com/lukeautry/tsoa#goal) and [philosophy](https://github.com/lukeautry/tsoa#philosophy) of the project?

**Potential Problems With The Approach**
Am not sure how the if statement impacts existing implementations. I've tested it on my own and it seems to work

**Test plan**
I would like to note that I've tried implementing a test for the problem described in the issue that seemed to pass, however my implementation didn't so I might have called the wrong method writing the test. Please feel free to discuss/provide more info on testing since I am really unfamiliar with the project's code

Patch:
diff --git a/src/routeGeneration/templateHelpers.ts b/src/routeGeneration/templateHelpers.ts
index 371d47e9..8712c778 100644
--- a/src/routeGeneration/templateHelpers.ts
+++ b/src/routeGeneration/templateHelpers.ts
@@ -468,10 +468,10 @@ export class ValidationService {
       const subFieldError: FieldErrors = {};
       const cleanValue = this.ValidateParam(subSchema, JSON.parse(JSON.stringify(value)), name, subFieldError, parent, swaggerConfig);
       subFieldErrors.push(subFieldError);
-      cleanValues = {
-        ...cleanValues,
-        ...cleanValue,
-      };
+
+      if (Object.keys(subFieldError).length === 0) {
+        cleanValues = { ...cleanValues, ...cleanValue };
+      }
     });
 
     if (subFieldErrors.length > 0 && !subFieldErrors.some(subFieldError => Object.keys(subFieldError).length === 0)) {


Test patch:
diff --git a/tests/unit/swagger/templateHelpers.spec.ts b/tests/unit/swagger/templateHelpers.spec.ts
index 001fb523..91fa4c79 100644
--- a/tests/unit/swagger/templateHelpers.spec.ts
+++ b/tests/unit/swagger/templateHelpers.spec.ts
@@ -8,7 +8,7 @@ describe('ValidationService', () => {
     it('should validate a model with declared properties', () => {
       const v = new ValidationService({
         ExampleModel: {
-          dataType: "refObject",
+          dataType: 'refObject',
           properties: {
             a: { dataType: 'string', required: true },
           },
@@ -34,7 +34,7 @@ describe('ValidationService', () => {
       const refName = 'ExampleModel';
       const v = new ValidationService({
         [refName]: {
-          dataType: "refObject",
+          dataType: 'refObject',
           additionalProperties: false,
           properties: {
             a: { dataType: 'string', required: true },
@@ -77,7 +77,7 @@ describe('ValidationService', () => {
       const refName = 'ExampleModel';
       const v = new ValidationService({
         [refName]: {
-          dataType: "refObject",
+          dataType: 'refObject',
           additionalProperties: false,
           properties: {
             a: { dataType: 'string', required: true },
@@ -117,7 +117,7 @@ describe('ValidationService', () => {
       const refName = 'ExampleModel';
       const v = new ValidationService({
         [refName]: {
-          dataType: "refObject",
+          dataType: 'refObject',
           additionalProperties: false,
           properties: {
             a: { dataType: 'string', required: true },
@@ -160,7 +160,7 @@ describe('ValidationService', () => {
       const refName = 'ExampleModel';
       const v = new ValidationService({
         [refName]: {
-          dataType: "refObject",
+          dataType: 'refObject',
           additionalProperties: false,
           properties: {
             a: { dataType: 'string', required: true },
@@ -195,7 +195,7 @@ describe('ValidationService', () => {
     it('should not require optional properties', () => {
       const v = new ValidationService({
         ExampleModel: {
-          dataType: "refObject",
+          dataType: 'refObject',
           properties: {
             a: { dataType: 'string' },
           },
@@ -213,7 +213,7 @@ describe('ValidationService', () => {
     it('should validate a model with additional properties', () => {
       const v = new ValidationService({
         ExampleModel: {
-          dataType: "refObject",
+          dataType: 'refObject',
           properties: {},
           additionalProperties: { dataType: 'any' },
         },
@@ -231,7 +231,7 @@ describe('ValidationService', () => {
     it('should validate a model with optional and additional properties', () => {
       const v = new ValidationService({
         ExampleModel: {
-          dataType: "refObject",
+          dataType: 'refObject',
           additionalProperties: { dataType: 'any' },
           properties: {
             a: { dataType: 'string' },
@@ -252,7 +252,7 @@ describe('ValidationService', () => {
     it('should validate additional properties only against non-explicitly stated properties', () => {
       const v = new ValidationService({
         ExampleModel: {
-          dataType: "refObject",
+          dataType: 'refObject',
           additionalProperties: {
             dataType: 'integer',
             validators: { minimum: { value: 10 } },
@@ -604,4 +604,35 @@ describe('ValidationService', () => {
       expect(error[name].message).to.equal(`required unique array`);
     });
   });
+
+  describe('Union validate', () => {
+    it('should validate discriminated union with silently-remove-extras on', () => {
+      const v = new ValidationService({
+        TypeA: {
+          dataType: 'refObject',
+          properties: {
+            type: { dataType: 'enum', enums: ['A'], required: true },
+            a: { dataType: 'double', required: true },
+          },
+        },
+        TypeB: {
+          dataType: 'refObject',
+          properties: {
+            type: { dataType: 'enum', enums: ['B'], required: true },
+            b: { dataType: 'double', required: true },
+          },
+        },
+      });
+      const name = 'name';
+      const error = {};
+      const minimalSwaggerConfig: SwaggerConfigRelatedToRoutes = {
+        noImplicitAdditionalProperties: 'silently-remove-extras',
+      };
+      const subSchemas = [{ ref: 'TypeA' }, { ref: 'TypeB' }];
+      const resultA = v.validateUnion(name, { type: 'A', a: 100 }, error, minimalSwaggerConfig, subSchemas);
+      const resultB = v.validateUnion(name, { type: 'B', b: 20 }, error, minimalSwaggerConfig, subSchemas);
+      expect(resultA).to.deep.equal({ type: 'A', a: 100 });
+      expect(resultB).to.deep.equal({ type: 'B', b: 20 });
+    });
+  });
 });
diff --git a/tests/unit/templating/templateHelpers.spec.ts b/tests/unit/templating/templateHelpers.spec.ts
index c4f8e01e..dec89321 100644
--- a/tests/unit/templating/templateHelpers.spec.ts
+++ b/tests/unit/templating/templateHelpers.spec.ts
@@ -51,10 +51,7 @@ it('should allow additionalProperties (on a union) if noImplicitAdditionalProper
 
   // Assert
   expect(errorDictionary).to.deep.eq({});
-  expect(result).to.eql({
-    value1: 'this is value 1',
-    value2: undefined,
-  });
+  expect(result).to.eql({ value1: 'this is value 1' });
   if (result[nameOfAdditionalProperty]) {
     throw new Error(`dataToValidate.${nameOfAdditionalProperty} should have been removed because "silently-remove-extras" requires that excess properties be stripped.`);
   }