Skip to content
This repository has been archived by the owner on Apr 13, 2022. It is now read-only.

Issue with properties in complex objects and tests. #52

Open
friendly-tech opened this issue Feb 27, 2019 · 4 comments
Open

Issue with properties in complex objects and tests. #52

friendly-tech opened this issue Feb 27, 2019 · 4 comments
Labels
bug Something isn't working core Core packages

Comments

@friendly-tech
Copy link
Contributor

The problem

When passing a model to a controller in a test and that model has a property that is a simple class (not a convector model), any properties on that child class are not populated when hitting the controller method.

Details

I have a convector Model (lets call it Parent), this has a property called Child with a type of ChildClass (this is a simple class, not Convector model) - this child class has a property called Name (string).

I decorate all of my properties correctly and manually create the schema using yup for the plain old typescript object.

I have a simple controller with a method that just returns whats passed in (and calls .ToJSON()), i write a test that creates an instance of Parent, then creates and instance of Parent.Child and sets Name equal to "NAME".

i simply pass this instance of parent into my controller method - when i hit the controller method, my instance of Parent indeed has an instance of ChildClass in the .Child property but the Name property of this instance is not set.

When running this in the actual network, this does not happen - only in the test.

Current Behavior

Child properties of child classes are not populated on the way in to the controller.

Expected Behavior

All properties should be populated correctly.

Code To Reproduce Issue [ Good To Have ]

Models


import * as yup from 'yup';
import {
  ConvectorModel,
  Default,
  ReadOnly,
  Required,
  Validate,
  getPropertiesValidation
} from '@worldsibu/convector-core-model';

export class ChildClass {
    @Required()
    @Validate(yup.string())
    public childClassName: string;

    public static schema()  {
        var inst = new ChildClass();
        return yup.object().shape({
            ...getPropertiesValidation(inst)
        });
    }
}

export class ParentClass extends ConvectorModel<ParentClass> {
  @ReadOnly()
  @Required()
  public readonly type = 'io.worldsibu.ParentClass';

  @Required()
  @Validate(yup.string())
  public name: string;

  @Required()
  @Validate(ChildClass.schema())
  public Child: ChildClass;
  
  
}

Controller

import {
  Controller,
  ConvectorController,
  Invokable,
  Param
} from '@worldsibu/convector-core-controller';

import { ParentClass } from './bugrepro.model';

@Controller('bugrepro')
export class BugreproController extends ConvectorController {
  @Invokable()
  public async test(@Param(ParentClass) pc: ParentClass): Promise<any> {
    // The issue isnt that this toJSON doesnt allow the property out, its that its not here on the way in
    if (!pc.Child.childClassName)   {
      // Was set in our test so shouldnt be missing, but it is...?
        // Toggle debug with --inspect-brk
        //pc.Child._childClassName is  set.. ?
      debugger;
    }
    return pc.toJSON();
  }
}

Test


// tslint:disable:no-unused-expression
import { join } from 'path';
import { expect } from 'chai';
import * as uuid from 'uuid/v4';
import { MockControllerAdapter } from '@worldsibu/convector-adapter-mock';
import 'mocha';

import {ChildClass, ParentClass} from '../src/bugrepro.model';
import { BugreproControllerClient } from '../client';

describe('Bugrepro', () => {
    let modelSample: ParentClass;
    let adapter: MockControllerAdapter;
    let bugreproCtrl: BugreproControllerClient;

    before(async () => {
        const now = new Date().getTime();
        modelSample = new ParentClass();
       
        modelSample.name = 'Parent';
        modelSample.Child = new ChildClass();
        modelSample.Child.childClassName = 'Child';
        adapter = new MockControllerAdapter();
        bugreproCtrl = new BugreproControllerClient(adapter);
        await adapter.init([
            {
            version: '*',
            controller: 'BugreproController',
            name: join(__dirname, '..')
            }
        ]);

    });

    it('should return whats passed in', async () => {
        var item = await bugreproCtrl.test(modelSample);
        expect(item.Child.childClassName).to.equal('Child');
    });
});
@diestrin diestrin added bug Something isn't working core Core packages labels Feb 27, 2019
@diestrin
Copy link
Contributor

diestrin commented Feb 27, 2019

Thanks for reporting. This is an issue in the .toJSON method in the core-model package, it's not being recursive on objects. We just need to iterate over each of the properties and normalize the properties.

public toJSON(skipEmpty = false): { [key in keyof T]?: T[key] } {
let protos = [];
let children = this;
// Search through the parents until reach ConvectorModel
do {
children = Object.getPrototypeOf(children);
protos.push(children);
} while (children['__proto__'].constructor.name !== ConvectorModel.name);
// Get all the descriptors for this model
const descriptors = protos.reduce((result, proto) => [
...result,
...Object.keys(proto)
// Map the keys to their [key,propDescriptior]
.map(key => [key, Object.getOwnPropertyDescriptor(proto, key)])
], []);
const base = Object.keys(this).concat('id')
.filter(k => !k.startsWith('_'))
.filter(k => !skipEmpty || this[k] !== undefined || this[k] !== null)
.reduce((result, key) => ({ ...result, [key]: this[key] }), {});
return descriptors
.reduce((result, [key, desc]) => {
const hasGetter = desc && typeof desc.get === 'function';
if (hasGetter) {
// Apply the descriptors to the children class
result[key] = desc.get.call(this);
}
if (skipEmpty && (result[key] === undefined || result[key] === null)) {
delete result[key];
}
return result;
}, base);
}

@friendly-tech
Copy link
Contributor Author

I had a look at that method, and did think should be be recursive - thanks for looking into it for me.

@friendly-tech
Copy link
Contributor Author

Would all required checks + validation also need be recursive in this way.
Im about to start testing that

@diestrin
Copy link
Contributor

diestrin commented Mar 1, 2019

@crazyman1979 I don't think so, toJSON only serealizes the info, so it should be just a matter of removing the lodash in front of the properties only.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working core Core packages
Projects
None yet
Development

No branches or pull requests

2 participants