Allows to define custom mappings for devices and assets for a tenant group only #61

Yoann-Abbes · 2021-04-01T15:36:52Z

What this PR does

This PR allows to have custom mapping for each tenant group for the assets and devices collections
#28
They are meant to be loaded at application startup

For devices collection, only qos, measures, and metadata fields are meant to be customized.
For assets collection, only metadata field is meant to be customized.

How to test

Functional tests.
Clone KIoTP repo on branch add-custom-mappings
Install plugins and switch to the add-custom-mappings branch on the device-plugin-manager
Define your own mappings in lib/plugins/device-manager/mappings/
Register it in lib/plugins/device-manager/index.ts with the provided methods
Run KIoTP
Create and engine kourou device-manager/engine:create -a group="groupTest" index="test"
Verify the mappings are correctly added.

deviceManagerPlugin.devices.registerQos
deviceManagerPlugin.devices.registerMetadata
deviceManagerPlugin.devices.registerMeasure

deviceManagerPlugin.assets.registerMetadata

Aschen

First round of reviews

Aschen · 2021-04-16T12:07:44Z

features/fixtures/application/app.ts

@@ -10,27 +10,49 @@ const deviceManager = new DeviceManagerPlugin();
 deviceManager.registerDecoder(new DummyTempDecoder());
 deviceManager.registerDecoder(new DummyTempPositionDecoder());

-deviceManager.mappings.devices.measures = {
+deviceManager.devices.registerMeasures({


To me this method could take the measure name as first parameter and then the measure definition.

This could allows to avoid to register 2 times the same measure for example

Aschen · 2021-04-16T12:08:31Z

features/fixtures/application/app.ts

+      value: { type: 'float' },
+    }
+  }
+}, 'astronaut');


Since it's an option, it should be of this form { group: "astronaut" }

lib/DeviceManagerPlugin.ts

Aschen · 2021-04-16T12:11:13Z

lib/DeviceManagerPlugin.ts

+      this.config.mappings.set(tenantGroup, {
+        ...this.config.mappings.get(tenantGroup),
+        assets,
+        'asset-history': assets


What is this? This collection is not included by default in the device manager plugin

lib/DeviceManagerPlugin.ts

Aschen · 2021-04-16T12:12:22Z

lib/DeviceManagerPlugin.ts

+        })
+      }
+    }
+    console.log(this.config.mappings);


Debug leftover

Aschen · 2021-04-16T12:12:36Z

lib/DeviceManagerPlugin.ts

+        this.config.mappings.set(tenantGroup, {
+          ...this.config.mappings.get(tenantGroup),
+          assets: this.config.mappings.get('shared').assets,
+          'asset-history': this.config.mappings.get('shared').assets


This collection does not exists in the device manager

Aschen · 2021-04-16T12:13:07Z

lib/controllers/EngineController.ts

@@ -52,7 +52,8 @@ export class EngineController {

  async create (request: KuzzleRequest) {
    const index = request.getIndex();
-    const { collections } = await this.engineService.create(index);
+    const tenantGroup = request.input.args.group;


You should use a request params helper intsead

lib/models/CustomProperties.ts

Aschen

Please can you add proper comments to every function with also parameter explanation for developers but also maintainers

lib/models/CustomProperties.ts

Leodau · 2021-04-21T13:17:31Z

Why is the package.lock getting modified?

Leodau

I would need an explanation on why was there the need to explode a functionality, into specific class methods.
Are we trying to build overkill setup functionalities on top of what should be templating?

register-measures
register-metadata
register-qos

Do these modifications imply that if we add new specific custom properties in the future OR new default collections with customizable properties, one would have to reconstruct/setup all this boilerplate by hand?

It seems to me at first glance, that a bad architectural choice was made somewhere.
@Yoann-Abbes @Aschen

doc/1/classes/devices-custom-properties/register-measures/index.md

doc/1/classes/devices-custom-properties/register-qos/index.md

Leodau · 2021-04-23T19:28:18Z

A simple guide to be added and IMO all done!

doc/1/guides/mappings/index.md

Co-authored-by: Michele Leo <[email protected]>

Yoann-Abbes added 2 commits March 25, 2021 17:17

wip [ci skip]

2f29747

wip [ci skip]

3fbc0af

Yoann-Abbes self-assigned this Apr 1, 2021

Yoann-Abbes marked this pull request as draft April 1, 2021 15:36

Yoann-Abbes linked an issue Apr 1, 2021 that may be closed by this pull request

Allows to define custom mappings for devices and assets for a tenant group only #28

Closed

Yoann-Abbes added 16 commits April 13, 2021 10:02

Merge branch '1-dev' into add-custom-mappings

c09ae96

wip [ci skip]

909caa0

Merge branch '1-dev' into add-custom-mappings

c7f4cba

wip [ci skip]

dbe9a20

wip [ci skip]

0d180aa

wip [ci skip]

0db7785

Merge branch '1-dev' into add-custom-mappings

cbbd097

wip [ci skip]

401e900

remove uselss map

f54c12f

comment

cdaac7a

lint

6be922b

fix func tests

3fc906a

com

107cb6b

add test

33d202b

add field text for keyword properties

1e50735

nit

b1009b8

Aschen suggested changes Apr 16, 2021

View reviewed changes

Yoann-Abbes added 2 commits April 16, 2021 15:22

remove asset-history collection

043e5bb

remove asset-history collection from test

16d75cc

Aschen suggested changes Apr 19, 2021

View reviewed changes

Yoann-Abbes added 2 commits April 20, 2021 14:19

@Aschen requested changed

4837eb9

Add doc

fabec15

Yoann-Abbes marked this pull request as ready for review April 20, 2021 13:28

Yoann-Abbes requested a review from Aschen April 20, 2021 13:28

Yoann-Abbes changed the title ~~Able to have custom mappings for each tenant kind~~ Able to have custom mappings for each tenant group Apr 20, 2021

Yoann-Abbes requested a review from Leodau April 20, 2021 16:03

Leodau reviewed Apr 20, 2021

View reviewed changes

lib/models/CustomProperties.ts Show resolved Hide resolved

Leodau reviewed Apr 20, 2021

View reviewed changes

lib/models/CustomProperties.ts Show resolved Hide resolved

Leodau reviewed Apr 20, 2021

View reviewed changes

lib/models/CustomProperties.ts Show resolved Hide resolved

Leodau reviewed Apr 20, 2021

View reviewed changes

lib/models/CustomProperties.ts Show resolved Hide resolved

nit

f25d8d5

Yoann-Abbes requested a review from Leodau April 20, 2021 16:21

package lock

c28ab1f

Leodau suggested changes Apr 22, 2021

View reviewed changes

Shiranuit suggested changes Apr 22, 2021

View reviewed changes

doc/1/classes/devices-custom-properties/register-measures/index.md Outdated Show resolved Hide resolved

doc/1/classes/devices-custom-properties/register-qos/index.md Outdated Show resolved Hide resolved

update doc

07f6231

Yoann-Abbes requested a review from Shiranuit April 23, 2021 01:24

Yoann-Abbes added 2 commits April 23, 2021 07:31

fix merge measures and shared properties

7f1b942

add guard

ba320c1

Add guide

ffb1b91

Yoann-Abbes requested a review from Leodau April 29, 2021 15:31

Leodau suggested changes Apr 29, 2021

View reviewed changes

doc/1/guides/mappings/index.md Outdated Show resolved Hide resolved

doc/1/guides/mappings/index.md Outdated Show resolved Hide resolved

doc/1/guides/mappings/index.md Outdated Show resolved Hide resolved

Yoann-Abbes and others added 3 commits April 29, 2021 19:57

Update doc/1/guides/mappings/index.md

2d0c972

Co-authored-by: Michele Leo <[email protected]>

Update doc/1/guides/mappings/index.md

6563501

Co-authored-by: Michele Leo <[email protected]>

Update doc/1/guides/mappings/index.md

3d73752

Co-authored-by: Michele Leo <[email protected]>

Yoann-Abbes requested a review from Leodau April 29, 2021 17:58

Yoann-Abbes changed the title ~~Able to have custom mappings for each tenant group~~ Allows to define custom mappings for devices and assets for a tenant group only Apr 30, 2021

Shiranuit approved these changes Apr 30, 2021

View reviewed changes

rolljee approved these changes May 3, 2021

View reviewed changes

Leodau merged commit 65fca52 into 1-dev May 4, 2021

Leodau deleted the add-custom-mappings branch May 4, 2021 12:23

Yoann-Abbes mentioned this pull request May 4, 2021

Release 0.2.0 #74

Merged

alexandrebouthinon unassigned Yoann-Abbes Sep 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allows to define custom mappings for devices and assets for a tenant group only #61

Allows to define custom mappings for devices and assets for a tenant group only #61

Yoann-Abbes commented Apr 1, 2021 •

edited

Loading

Aschen left a comment

Aschen Apr 16, 2021

Aschen Apr 16, 2021

Aschen Apr 16, 2021

Aschen Apr 16, 2021

Aschen Apr 16, 2021

Aschen Apr 16, 2021

Aschen left a comment

Leodau commented Apr 21, 2021

Leodau left a comment •

edited

Loading

Leodau commented Apr 23, 2021 •

edited

Loading

Allows to define custom mappings for devices and assets for a tenant group only #61

Allows to define custom mappings for devices and assets for a tenant group only #61

Conversation

Yoann-Abbes commented Apr 1, 2021 • edited Loading

What this PR does

How to test

Aschen left a comment

Choose a reason for hiding this comment

Aschen Apr 16, 2021

Choose a reason for hiding this comment

Aschen Apr 16, 2021

Choose a reason for hiding this comment

Aschen Apr 16, 2021

Choose a reason for hiding this comment

Aschen Apr 16, 2021

Choose a reason for hiding this comment

Aschen Apr 16, 2021

Choose a reason for hiding this comment

Aschen Apr 16, 2021

Choose a reason for hiding this comment

Aschen left a comment

Choose a reason for hiding this comment

Leodau commented Apr 21, 2021

Leodau left a comment • edited Loading

Choose a reason for hiding this comment

Leodau commented Apr 23, 2021 • edited Loading

Yoann-Abbes commented Apr 1, 2021 •

edited

Loading

Leodau left a comment •

edited

Loading

Leodau commented Apr 23, 2021 •

edited

Loading