2

I often have my reducers in @ngrx/store to set value to the state, sometimes from a http request

All examples and tutorials I have seen is simply setting the payload to the state, or setting the payload to a key in the state via Object.assign or spreading.

The problem is, how does one validate if the payload is valid? There has been many cases for me where someone provides a bad payload via this.store.dispatch(). It would be in the wrong format, or would be missing a key etc, which would bring in silent bugs as the store's reducer would mess up the state. This is especially true with http requests.

This led to my teams project absolutely LITTERED with null checks everywhere because we cannot trust the store anymore.

How do people validate, or protect their store from bad data?

I have a few ideas from reading:

How can I check that two objects have the same set of property names?

Dolan
  • 1,519
  • 4
  • 16
  • 36
  • I choose to embrace null checks at point of use. The thing is, aside from bad action payloads, store properties are likely to be undefined before an http request completes. – Richard Matsen Apr 10 '18 at 21:56

2 Answers2

1

So people are dispatching invalid action data to the store?

Relying on Typescript to validate the action payload may not be enough. If you want more support try using something like tassign (https://www.npmjs.com/package/tassign) vs Object.assign.

If that's still not enough, you may try validating the data in an effect, once the http request completes and before it's dispatched.

bc1105
  • 1,270
  • 8
  • 8
  • Yes, people are dispatching invalid data to the store, not intentionally of course. TypeScript is not enough, unexpected things happen at runtime. – Dolan Apr 10 '18 at 15:54
1

I use a helper service ObjectShapeComparer to ensure my external config file is of the correct shape before posting it to the store (i.e no properties have been renamed or deleted).

It focuses on types, structure, and missing properties rather than values (so to avoid null checks on store reads you need more functionality). I guess the best thing is it manages the errors and posts them to the store.

Here is the list of tests it passes.

it('should compare a template object to another object and return a list of         
it('should ignore functions', () => {
it('should flag missing properties', () => {
it('should flag differently named properties', () => {
it('should flag all differences', () => {
it('should ignore extra properties on actual', () => {
it('should ignore value differences', () => {
it('should flag type differences (number vs string)', () => {
it('should flag type differences (object vs string)', () => {
it('should flag type differences (object vs array)', () => {
it('should ignore type when template uses undefined value', () => {
it('should ignore property order when no diffs', () => {
it('should ignore property order when diffs', () => {
it('should flag diffs on nested objects', () => {
it('should full qualify nested paths', () => {
it('should flag structure diffs when levels differ', () => {
it('should flag type diffs when levels differ', () => {
it('should flag array diffs', () => {
it('should ignore array ordering', () => {
it('should flag array diffs regardless of array ordering', () => {
it('should flag nested array diffs', () => {

Calling Here is a function which uses it (note I am using angular-redux not ngrx). This is part of a middleware routine which handles http requests.

I guess in ngrx you can incorporate it into an Effect. You would need a mapping of action types to templates.

checkTemplate(data): boolean {
  const results = this.objectShapeComparer.compare(this.configTemplate, data);
  if (results.length === 0) {
    return true;
  }
  results.forEach(result => {
    this.ngRedux.dispatch({
      type: ConfigActions.INITIALIZE_CONFIG_TEMPLATE_ERROR,
      data: result,
    });
  });
  return false;
}

Here is a sample template

this.pageConfigTemplate = {
  pageTitle: '',      // type should be string
  pageDescription: '',
  listTitle: '',
  listWidth: 0,       // type should be number
  badgeUnits: '',
  resultsZoom: ''
};

this.configTemplate = {
  baseDataUrl: '',
  page1Config: {
    filePrefixes: [],     // type should be array    
    numDataPointsForSparkline: 0,
    numInitialFilesToDisplay: 0,
    page: this.pageConfigTemplate   // type should be object
  },
...

ObjectShapeComparer.ts

import { Injectable } from '@angular/core';

@Injectable()
export class ObjectShapeComparer {

  compare(expected, actual): string[] {
    return this.compareObjectShape(expected, actual);
  }

  private compareObjectShape(expected, actual, path = ''): string[] {
    let diffs = [];
    for (const key in expected) {
      // Ignore function properties
      if (!expected.hasOwnProperty(key) || typeof expected[key] === 'function') {
        continue;
      }

      const fullPath = path + (path === '' ? '' : '.') + key;

      // Property exists?
      if (!actual.hasOwnProperty(key)) {
        diffs.push(`Missing property ${fullPath}`);
        continue; // no need to check further when actual is missing
      }

      // Template value = undefined, means no type checking, no nested objects
      const expectedValue = expected[key];
      if (expectedValue === undefined) {
        continue;
      }

      // Types match?
      const expectedType = this.getType(expectedValue);
      const actualValue = actual[key];
      const actualType = this.getType(actualValue);
      if (expectedType !== actualType) {
        diffs.push(`Types differ for property ${fullPath} (${expectedType} vs ${actualType})`);
      }

      // Recurse nested objects and arrays
      diffs = diffs.concat(this.recurse(expectedValue, actualValue, fullPath));
    }
    return diffs;
  }

  private recurse(expectedValue, actualValue, path): string[] {
    let diffs = [];
    const expectedType = this.getType(expectedValue);
    if (expectedType === 'array') {
      diffs = diffs.concat(this.compareArrays(expectedValue, actualValue, path));
    }
    if (expectedType === 'object') {
      diffs = diffs.concat(this.compareObjectShape(expectedValue, actualValue, path));
    }
    return diffs;
  }

  private compareArrays(expectedArray, actualArray, path): string[] {
    let diffs = [];
    if (expectedArray.length === 0 || this.arrayIsPrimitive(expectedArray)) {
      return diffs;
    }

    // Look for expected element anywhere in the actuals array
    const actualKeys = actualArray.map(element => this.getKeys(element).join(','));
    for (let index = 0; index < expectedArray.length; index++) {
      const fullPath = path + '.' + index;
      const expectedElement = expectedArray[index];
      const actualElement = this.actualMatchingExpected(expectedElement, actualArray);
      if (!actualElement) {
        diffs.push(`Missing array element ${fullPath} (keys: ${this.getKeys(expectedElement).join(',')})`);
        continue;
      }
      diffs = diffs.concat(this.recurse(expectedElement, actualElement, fullPath));
    };
    return diffs;
  }

  private getKeys(obj): any[] {
    return typeof obj === 'object' ?
      Object.keys(obj)
        .filter(key => obj.hasOwnProperty(key)) // ignore function properties
        .sort()
      : [];
  }

  private getType(el: any): string {
    return Array.isArray(el) ? 'array' : typeof el;
  }

  private arrayIsPrimitive(array): boolean {
    const arrayType = this.getType(array[0]);
    return arrayType !== 'object' && arrayType !== 'array';
  }

  private actualMatchingExpected(expected, actuals): any {
    const expectedKeys = this.getKeys(expected).join(',');
    const actualKeys = actuals.map(element => this.getKeys(element).join(','));
    const match = actualKeys.indexOf(expectedKeys);
    // tslint:disable-next-line:no-bitwise
    return (~match) ? actuals[match] : null;
  }

}
Richard Matsen
  • 20,671
  • 3
  • 43
  • 77