Wrap SiteWise Asset related API calls in a Data Source #25

gareth-amazon · 2021-12-31T07:40:44Z

Overview

Created a DataSource for SiteWise assets module to use. This scopes down what the asset modules can get at from the raw API.
Now that there is an interface it possible to mock it and test the RequestProcessor
With passing tests the coverage limits can be raised back up to 80%
Changed the base class for RequestProcessWorker to ReplaySubject. This stops an initial 'empty' value from being set to the caller which could be confusing and make programming more complex for the non-list use-case.
Use RequestProcessorWorkerGroup for every request in RequestProcessor, not any simultaneous requests for the same data item will result in only 1 API call.

Tests

Added UTs for Request Processor happy path

Legal

This project is available under the Apache 2.0 License.

NorbertNader · 2022-01-03T23:22:35Z

packages/core/src/asset-modules/sitewise/requestProcessorWorker.ts

@@ -1,4 +1,4 @@
-import { BehaviorSubject, Observable, Subscription } from 'rxjs';
+import { Observable, ReplaySubject, Subscription } from 'rxjs';


How are you using ReplaySubject here? From the documentation I see it is used to record a history of previous values but I don't see that being used here. I think I am missing something.

Ok, I think I missed the part that by default it will return all the previous values if no buffer is provided.

Good call out! I only want it to replay the latest value. I can get that with ReplaySubject(1)

NorbertNader · 2022-01-03T23:28:20Z

packages/core/stencil.config.ts

-        branches: 60,
-        functions: 70,
-        lines: 75,
+        statements: 80,


NorbertNader · 2022-01-03T23:29:55Z

packages/core/src/asset-modules/sitewise/requestProcessor.spec.ts

+
+  it('waits for the AssetSummary', done => {
+    observable.subscribe(result => {
+      expect(result).not.toBeUndefined();


nit: do we need this assertion?

Weirdly, yes. I spent the better part of a day debugging the tests only to find out that I was comparing undefined to undefined and getting passing tests when they should have failed.

NorbertNader · 2022-01-03T23:40:11Z

packages/core/src/data-module/types.d.ts

@@ -102,6 +113,14 @@ type SubscribeToDataStreamsFromPrivate = (
  unsubscribe: () => void;
 };

+export type SiteWiseAssetDataSource = {
+  describeAsset: (input: DescribeAssetCommandInput) => Promise<DescribeAssetCommandOutput>;
+  getPropertyValue: (input: GetAssetPropertyValueCommandInput) => Promise<GetAssetPropertyValueCommandOutput>;


Why not use the data module to get asset property value? https://github.com/awslabs/iot-app-kit/blob/main/packages/core/src/data-sources/site-wise/client/getLatestPropertyDataPoint.ts

The version in the assets code treats all values as if they are attributes, i.e. they do not update over time (this avoids re-requesting things that shouldn't actually change).

gareth-amazon · 2022-01-06T21:12:32Z

packages/core/src/asset-modules/sitewise/requestProcessorWorker.ts

+    error?: ((error: any) => void) | null,
+    complete?: (() => void) | null
+  ): Subscription {
+    throw "deprecated, use addObserver";


addSubscriber

…scribe event

NorbertNader · 2022-01-07T16:45:00Z

packages/core/src/asset-modules/sitewise/requestProcessorWorkerGroup.spec.ts

+      let timeoutID: any;
+      let counter = 0;
+      return new Observable<number>((subscriber) => {
+        timeoutID = setTimeout(function incramenter() {


nit: incrementer

NorbertNader · 2022-01-07T16:58:57Z

packages/core/src/asset-modules/sitewise/requestProcessorWorkerGroup.spec.ts

+        timeoutID = setTimeout(function incramenter() {
+          counter++;
+          subscriber.next(counter);
+          timeoutID = setTimeout(incramenter, 5);


nit: instead of recursion could just be setInterval?

ahh, totally forgot about that, will change.

* Release 1.2.1 (#85) * fix: unsubrscribe data provider on component updates * Release 1.2.1 Co-authored-by: Norbert Nader <nnader@amazon.com> * fix: resolves #83 (#87) * fix: resolves #83 * Update index.ts Co-authored-by: Norbert Nader <nnader@amazon.com> * feat: improve documentation (#90) * Run tests on pull request (#91) * Update reference to code name (#86) AWS-UI internal code name is mentioned in the readme. Co-authored-by: Norbert Nader <Norbert.Nader@gmail.com> * feat: update synchro charts to 3.1.0, update docs (#92) * feat: Improve documentation formatting (#93) * feat: Improve resource explorer docs (#95) * Update README.md (#94) * feat: support fetchMostRecentBeforeStart (#79) * Update documentation & make react-components easier to use (#98) * Utilize lock file in CI (#99) * build(deps): bump async from 2.6.3 to 2.6.4 (#100) Bumps [async](https://github.com/caolan/async) from 2.6.3 to 2.6.4. - [Release notes](https://github.com/caolan/async/releases) - [Changelog](https://github.com/caolan/async/blob/v2.6.4/CHANGELOG.md) - [Commits](caolan/async@v2.6.3...v2.6.4) --- updated-dependencies: - dependency-name: async dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * feat: support auto-assigning colors for certain components (#96) * feat: support auto-assigning colors for certain components * Update package.json * Update cypress to v9, added stricted linting, fixed linting warnings and errors (#104) * Add stylelint and enforce via global package.json (#106) * use most recent synchro-charts version, 4.0.1 (#111) * build(deps): bump eventsource from 1.1.0 to 1.1.1 (#116) Bumps [eventsource](https://github.com/EventSource/eventsource) from 1.1.0 to 1.1.1. - [Release notes](https://github.com/EventSource/eventsource/releases) - [Changelog](https://github.com/EventSource/eventsource/blob/master/HISTORY.md) - [Commits](EventSource/eventsource@v1.1.0...v1.1.1) --- updated-dependencies: - dependency-name: eventsource dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * feat: expand children in resource explorer (#115) * feat: expand children in resource explorer * fix typo * Fix asset module cache * Add logic for fetching hierarchies * Add tests * Improve insert * Fix linting Co-authored-by: Norbert Nader <nnader@amazon.com> * Update README.md (#121) Fix test runner status badge * Bump minor version (#120) * specify react types package version to be 17 (#134) Co-authored-by: Xinyi <xinyxu@amazon.com> * build(deps): bump parse-url from 6.0.0 to 6.0.2 (#142) Bumps [parse-url](https://github.com/IonicaBizau/parse-url) from 6.0.0 to 6.0.2. - [Release notes](https://github.com/IonicaBizau/parse-url/releases) - [Commits](https://github.com/IonicaBizau/parse-url/commits) --- updated-dependencies: - dependency-name: parse-url dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix: Development Quick Start Docs/Scripts (#148) * add yarn bootstrap script to replace lerna bootstrap * specify supported node and yarn engines * simplify development docs and update build steps Co-authored-by: Thomas Juranek <thjurane@amazon.com> * build(deps): bump moment from 2.29.2 to 2.29.4 (#149) Bumps [moment](https://github.com/moment/moment) from 2.29.2 to 2.29.4. - [Release notes](https://github.com/moment/moment/releases) - [Changelog](https://github.com/moment/moment/blob/develop/CHANGELOG.md) - [Commits](moment/moment@2.29.2...2.29.4) --- updated-dependencies: - dependency-name: moment dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix: update @testing-library/react to use create root (#151) Co-authored-by: Norbert Nader <Norbert.Nader@gmail.com> * feat(props): adapt props for synchro-charts (#133) * feat(props): adapt props for synchro-charts * add testing for new props * fix(tests): cleanup tests from devwork * Update changelog * update tests to wait for intercepts Co-authored-by: Norbert Nader <Norbert.Nader@gmail.com> * feat: batch API for historical, aggregated, and latest value data (#137) * fix: resolve float decimal precision issue on round() function. (#160) * fix: update @testing-library/react to use create root * fix: resolve float decimal precision issue on round() function. * Simplify round() function. Remove managing the negative sign separately. Co-authored-by: Norbert Nader <Norbert.Nader@gmail.com> * sync changes with private repo for common files (#150) * sync changes with private repo for common files * Revert "fix: update @testing-library/react to use create root (#151)" This reverts commit 960f53f. Co-authored-by: Xinyi <xinyxu@amazon.com> * Update npm packages repository (#161) * build(deps): bump terser from 4.8.0 to 4.8.1 (#168) Bumps [terser](https://github.com/terser/terser) from 4.8.0 to 4.8.1. - [Release notes](https://github.com/terser/terser/releases) - [Changelog](https://github.com/terser/terser/blob/master/CHANGELOG.md) - [Commits](https://github.com/terser/terser/commits) --- updated-dependencies: - dependency-name: terser dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * remove node version requirement for now until switched internal package to v16 Co-authored-by: Norbert Nader <Norbert.Nader@gmail.com> Co-authored-by: Norbert Nader <nnader@amazon.com> Co-authored-by: db <77755322+diehbria@users.noreply.github.com> Co-authored-by: Mitchell Lee <mitch@evildev.net> Co-authored-by: Bowei Han <boweih@amazon.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Xinyi Xu <sheilaxxy@gmail.com> Co-authored-by: Xinyi <xinyxu@amazon.com> Co-authored-by: Thomas Juranek <thomas@juranek.com> Co-authored-by: Thomas Juranek <thjurane@amazon.com> Co-authored-by: Xiao(Bill) Li <limxiao@amazon.com> Co-authored-by: jmbuss <107281089+jmbuss@users.noreply.github.com>

NorbertNader reviewed Jan 3, 2022

View reviewed changes

packages/core/stencil.config.ts

branches: 60,

functions: 70,

lines: 75,

statements: 80,

Copy link

Contributor

NorbertNader Jan 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

NorbertNader reviewed Jan 3, 2022

View reviewed changes

NorbertNader previously approved these changes Jan 3, 2022

View reviewed changes

NorbertNader reviewed Jan 3, 2022

View reviewed changes

gareth-amazon dismissed NorbertNader’s stale review via d853888 January 5, 2022 23:57

gareth-amazon commented Jan 6, 2022

View reviewed changes

gareth-amazon added 5 commits January 6, 2022 18:14

Wrap SiteWise Asset related API calls in a Data Source

5415b2c

Add Unit Testing for RequestProcessorWorkerGroup

4a6d691

Fix RequestProcessorWorker so it fund the finalizer on the last unsub…

c5094e4

…scribe event

Add test with proof that finalizers run on the worker process

6c4eede

Results of

1c45760

gareth-amazon force-pushed the sitewise-asset-data-source branch from e821596 to 1c45760 Compare January 7, 2022 02:22

NorbertNader previously approved these changes Jan 7, 2022

View reviewed changes

NorbertNader reviewed Jan 7, 2022

View reviewed changes

Fixup

a4c3962

gareth-amazon dismissed NorbertNader’s stale review via a4c3962 January 7, 2022 23:03

Merge branch 'main' into sitewise-asset-data-source

7f80cd2

NorbertNader approved these changes Jan 11, 2022

View reviewed changes

gareth-amazon merged commit b9eabc2 into main Jan 11, 2022

diehbria deleted the sitewise-asset-data-source branch May 5, 2022 16:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrap SiteWise Asset related API calls in a Data Source #25

Wrap SiteWise Asset related API calls in a Data Source #25

gareth-amazon commented Dec 31, 2021

NorbertNader Jan 3, 2022 •

edited

Loading

NorbertNader Jan 3, 2022 •

edited

Loading

gareth-amazon Jan 4, 2022

NorbertNader Jan 3, 2022

NorbertNader Jan 3, 2022

gareth-amazon Jan 4, 2022

NorbertNader Jan 3, 2022

gareth-amazon Jan 4, 2022

gareth-amazon Jan 6, 2022

NorbertNader Jan 7, 2022

gareth-amazon Jan 7, 2022

NorbertNader Jan 7, 2022

gareth-amazon Jan 7, 2022

		@@ -1,4 +1,4 @@
		import { BehaviorSubject, Observable, Subscription } from 'rxjs';
		import { Observable, ReplaySubject, Subscription } from 'rxjs';

Wrap SiteWise Asset related API calls in a Data Source #25

Wrap SiteWise Asset related API calls in a Data Source #25

Conversation

gareth-amazon commented Dec 31, 2021

Overview

Tests

Legal

NorbertNader Jan 3, 2022 • edited Loading

Choose a reason for hiding this comment

NorbertNader Jan 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NorbertNader Jan 3, 2022 •

edited

Loading

NorbertNader Jan 3, 2022 •

edited

Loading