Skip to content

Commit

Permalink
fix: only data props on Timeseries and Asset class are enumerable
Browse files Browse the repository at this point in the history
* fix: allow shallow copying for Asset and Timeseries by making resource method props not enumerable
* fix: don't expose httpClient and resourcePath
* test: timeseries filter on external root id fails
  • Loading branch information
polomani committed Sep 16, 2019
1 parent 1c1462d commit a2f412d
Show file tree
Hide file tree
Showing 9 changed files with 210 additions and 132 deletions.
6 changes: 0 additions & 6 deletions src/__tests__/resources/assetList.integration.specs.ts
Expand Up @@ -41,10 +41,4 @@ describe('assetList', () => {
await client.assets.delete(createdAssets.map(asset => ({ id: asset.id })));
await client.events.delete(createdEvents.map(event => ({ id: event.id })));
});

test('json stringify', async () => {
const response = await client.assets.list({ limit: 2 });
const stringRes = JSON.stringify(response.items);
expect(typeof stringRes).toBe('string');
});
});
4 changes: 2 additions & 2 deletions src/__tests__/resources/assets.integration.spec.ts
Expand Up @@ -199,7 +199,7 @@ describe('Asset integration test', () => {
const root2 = { name: 'root-2', externalId: 'root-2' + randomInt() };
const child1 = { name: 'child-1', parentExternalId: root1.externalId };
const child2 = { name: 'child-2', parentExternalId: root2.externalId };
const [createdChild1] = await client.assets.create([
const [createdChild1, createdRoot1] = await client.assets.create([
child1,
root1,
root2,
Expand All @@ -217,7 +217,7 @@ describe('Asset integration test', () => {
await runTestWithRetryWhenFailing(async () => {
const nonRootAssetsUnderRootId = await client.assets
.list({
filter: { root: false, rootIds: [{ externalId: root1.externalId }] },
filter: { root: false, rootIds: [{ id: createdRoot1.id }] },
limit: 2,
})
.autoPagingToArray({ limit: 2 });
Expand Down
45 changes: 43 additions & 2 deletions src/__tests__/resources/assets.unit.spec.ts
@@ -1,17 +1,18 @@
// Copyright 2019 Cognite AS

import * as nock from 'nock';
import { ExternalAssetItem } from '../../';
import CogniteClient from '../../cogniteClient';
import { Node } from '../../graphUtils';
import {
enrichAssetsWithTheirParents,
promiseAllAtOnce,
promiseEachInSequence,
} from '../../resources/assets/assetUtils';
import { ExternalAssetItem } from '../../types/types';
import { mockBaseUrl, setupMockableClient } from '../testUtils';

describe('Asset unit test', () => {
// tslint:disable-next-line:no-big-function
describe('Assets unit test', () => {
let client: CogniteClient;
beforeEach(() => {
client = setupMockableClient();
Expand Down Expand Up @@ -198,4 +199,44 @@ describe('Asset unit test', () => {
});
});
});

describe('class is not polluted with enumerable props', async () => {
const items = [
{
id: 1,
},
{
id: 2,
},
];

beforeEach(() => {
nock.cleanAll();
nock(mockBaseUrl)
.post(new RegExp('/assets/list'))
.thrice()
.reply(200, { items });
});

test('JSON.stringify works', async () => {
const assets = await client.assets.list().autoPagingToArray();
expect(() => JSON.stringify(assets)).not.toThrow();
expect(() => JSON.stringify(assets[0])).not.toThrow();
});

test('change context for asset utility methods', async () => {
const assets = await client.assets.list().autoPagingToArray();
const utilMethod = assets[0].children;
const result = await utilMethod();
const resultAfterBind = await utilMethod.call(null);
expect({ ...result[0] }).toEqual({ ...resultAfterBind[0] });
expect([{ ...result[0] }, { ...result[1] }]).toEqual(items);
});

test('spread operator receives only object data props', async () => {
const assets = await client.assets.list().autoPagingToArray();
expect([{ ...assets[0] }, { ...assets[1] }]).toEqual(items);
expect(Object.assign({}, assets[1])).toEqual(items[1]);
});
});
});
24 changes: 0 additions & 24 deletions src/__tests__/resources/baseResource.unit.spec.ts

This file was deleted.

44 changes: 44 additions & 0 deletions src/__tests__/resources/timeSeriesList.unit.spec.ts
Expand Up @@ -99,4 +99,48 @@ describe('TimeSeriesList class unit test', async () => {
expect(fetchedDatapoints).toHaveLength(3);
expect(fetchedDatapoints[0].datapoints[0].timestamp).toBeDefined();
});

describe('class is not polluted with enumerable props', () => {
const items = [
{
id: 1,
},
{
id: 2,
},
];

beforeEach(() => {
nock.cleanAll();
nock(mockBaseUrl)
.get(new RegExp('/timeseries/'))
.once()
.reply(200, { items });
});

test('JSON.stringify works', async () => {
const timeseries = await client.timeseries.list().autoPagingToArray();
expect(() => JSON.stringify(timeseries)).not.toThrow();
expect(() => JSON.stringify(timeseries[0])).not.toThrow();
});

test('change context for asset utility methods', async () => {
nock(mockBaseUrl)
.post(new RegExp('/timeseries/data/list'))
.twice()
.reply(200, { items });
const timeseries = await client.timeseries.list().autoPagingToArray();
const utilMethod = timeseries[0].getDatapoints;
const result = await utilMethod();
const resultAfterBind = await utilMethod.call(null);
expect({ ...result[0] }).toEqual({ ...resultAfterBind[0] });
expect([{ ...result[0] }, { ...result[1] }]).toEqual(items);
});

test('spread operator receives only object data props', async () => {
const timeseries = await client.timeseries.list().autoPagingToArray();
expect([{ ...timeseries[0] }, { ...timeseries[1] }]).toEqual(items);
expect(Object.assign({}, timeseries[1])).toEqual(items[1]);
});
});
});
4 changes: 2 additions & 2 deletions src/resources/baseResourceApi.ts
Expand Up @@ -54,8 +54,8 @@ export abstract class BaseResourceAPI<
return chunk(items, chunkSize);
}
constructor(
public readonly resourcePath: string,
public readonly httpClient: CDFHttpClient,
private readonly resourcePath: string,
protected readonly httpClient: CDFHttpClient,
private map: MetadataMap
) {}

Expand Down
99 changes: 57 additions & 42 deletions src/resources/classes/asset.ts
Expand Up @@ -7,6 +7,14 @@ import {
FileFilter,
TimeseriesFilter,
} from '../../index';
import {
AssetDescription,
AssetName,
AssetSource,
CogniteExternalId,
CogniteInternalId,
Metadata,
} from '../../types/types';
import { AssetList } from './assetList';
import { BaseResource } from './baseResource';

Expand All @@ -17,36 +25,39 @@ export interface DeleteOptions {
recursive?: boolean;
}
export class Asset extends BaseResource<TypeAsset> implements TypeAsset {
public get id() {
return this.props.id;
}
public get parentId() {
return this.props.parentId;
}
public get name() {
return this.props.name;
}
public get description() {
return this.props.description;
}
public get metadata() {
return this.props.metadata;
}
public get source() {
return this.props.source;
}
public get lastUpdatedTime() {
return this.props.lastUpdatedTime;
}
public get createdTime() {
return this.props.createdTime;
}
public get rootId() {
return this.props.rootId;
}
public id: CogniteInternalId;
public externalId?: CogniteExternalId;
public parentId?: CogniteInternalId;
public name: AssetName;
public description?: AssetDescription;
public metadata?: Metadata;
public source?: AssetSource;
public lastUpdatedTime: Date;
public createdTime: Date;
public rootId: CogniteInternalId;

constructor(client: CogniteClient, props: TypeAsset) {
super(client, props);
super(client);
this.id = props.id;
this.externalId = props.externalId;
this.parentId = props.parentId;
this.name = props.name;
this.description = props.description;
this.metadata = props.metadata;
this.source = props.source;
this.lastUpdatedTime = props.lastUpdatedTime;
this.createdTime = props.createdTime;
this.rootId = props.rootId;

Object.defineProperties(this, {
delete: { value: this.delete.bind(this), enumerable: false },
parent: { value: this.parent.bind(this), enumerable: false },
children: { value: this.children.bind(this), enumerable: false },
subtree: { value: this.subtree.bind(this), enumerable: false },
timeSeries: { value: this.timeSeries.bind(this), enumerable: false },
events: { value: this.events.bind(this), enumerable: false },
files: { value: this.files.bind(this), enumerable: false },
});
}

/**
Expand All @@ -57,7 +68,7 @@ export class Asset extends BaseResource<TypeAsset> implements TypeAsset {
* await asset.delete();
* ```
*/
public delete = async (options: DeleteOptions = {}) => {
public async delete(options: DeleteOptions = {}) {
return this.client.assets.delete(
[
{
Expand All @@ -66,31 +77,31 @@ export class Asset extends BaseResource<TypeAsset> implements TypeAsset {
],
options
);
};
}

/**
* Retrieves the parent of the current asset
* ```js
* const parentAsset = await asset.parent();
* ```
*/
public parent = async () => {
public async parent() {
if (this.parentId) {
const [parentAsset] = await this.client.assets.retrieve([
{ id: this.parentId },
]);
return parentAsset;
}
return null;
};
}

/**
* Returns an AssetList object with all children of the current asset
* ```js
* const children = await asset.children();
* ```
*/
public children = async () => {
public async children() {
const childAssets = await this.client.assets
.list({
filter: {
Expand All @@ -99,7 +110,7 @@ export class Asset extends BaseResource<TypeAsset> implements TypeAsset {
})
.autoPagingToArray({ limit: Infinity });
return new AssetList(this.client, childAssets);
};
}

/**
* Returns the full subtree of the current asset, including the asset itself
Expand All @@ -108,13 +119,13 @@ export class Asset extends BaseResource<TypeAsset> implements TypeAsset {
* const subtree = await asset.subtree();
* ```
*/
public subtree = async (options?: SubtreeOptions) => {
public async subtree(options?: SubtreeOptions) {
const query: SubtreeOptions = options || {};
return this.client.assets.retrieveSubtree(
{ id: this.id },
query.depth || Infinity
);
};
}

/**
* Returns all timeseries for the current asset
Expand All @@ -123,14 +134,14 @@ export class Asset extends BaseResource<TypeAsset> implements TypeAsset {
* const timeSeries = await asset.timeSeries();
* ```
*/
public timeSeries = async (filter: TimeseriesFilter = {}) => {
public async timeSeries(filter: TimeseriesFilter = {}) {
return this.client.timeseries
.list({
...filter,
assetIds: [this.id],
})
.autoPagingToArray({ limit: Infinity });
};
}

/**
* Returns all events for the current asset
Expand All @@ -139,13 +150,13 @@ export class Asset extends BaseResource<TypeAsset> implements TypeAsset {
* const events = await asset.events();
* ```
*/
public events = async (filter: EventFilter = {}) => {
public async events(filter: EventFilter = {}) {
return this.client.events
.list({
filter: { ...filter, assetIds: [this.id] },
})
.autoPagingToArray({ limit: Infinity });
};
}

/**
* Returns all files for the current asset
Expand All @@ -154,11 +165,15 @@ export class Asset extends BaseResource<TypeAsset> implements TypeAsset {
* const files = await asset.files();
* ```
*/
public files = async (filter: FileFilter = {}) => {
public async files(filter: FileFilter = {}) {
return this.client.files
.list({
filter: { ...filter, assetIds: [this.id] },
})
.autoPagingToArray({ limit: Infinity });
};
}

public toJSON() {
return { ...this };
}
}

0 comments on commit a2f412d

Please sign in to comment.