Calculated properties plugin #2146

teallarson · 2021-08-16T21:31:00Z

DONE:

generates an app
generates a source
generates a property
replaces mustached variables from a property's customFunction option, checks for illegal strings (require, env)
Sorts config validations, application, and processing so that calculated properties wait on the properties they reference
Executes the function from the customFunction string in a Node VM instance per profile
saves value returned from customFunction as a new ProfileProperty
recalculates on each run (like all other profileProperties)
Testing coverage:
- cli
- validate
- property-imports for each type (based on query sources)
- property-imports also tests that the function will throw with invalid input

TO DO AFTER COMPLETE (www):

(in progress) Docs
- basics about the tool in code config docs
- important to note:
  - profile property values are parsed into the function as strings
  - we actually use a property's name in the mustache vars, not its id
What's New
Add a calculated property to roo demo -c

evantahler

Nice work so far! Adding some comments for later today

plugins/@grouparoo/calculated-property/src/lib/profileProperty.ts

plugins/@grouparoo/calculated-property/public/templates/property/{{{id}}}.js.template

plugins/@grouparoo/calculated-property/src/lib/profileProperty.ts

plugins/@grouparoo/calculated-property/src/lib/test.ts

plugins/@grouparoo/csv/src/initializers/plugin.ts

plugins/@grouparoo/calculated-property/src/lib/propertyOptions.ts

teallarson · 2021-08-23T22:06:27Z

topological sort with mustache vars works across plugins now
completed unit testing
vm executes the function now instead of just compiling and exporting it

core/src/initializers/codeConfig.ts

core/src/classes/codeConfig.ts

core/src/models/Property.ts

core/src/modules/plugin.ts

core/src/utils/pluginDetails.js

plugins/@grouparoo/calculated-property/src/lib/profileProperty.ts

evantahler · 2021-08-24T16:30:17Z

plugins/@grouparoo/calculated-property/src/lib/profileProperty.ts

+    calculatedPropertyValue = vm.run(
+      `const toRun = ${populatedFunction}; module.exports = toRun();`
+    );


👍 the method is being defined and run in the VM, not our 'real' code

teallarson · 2021-08-24T20:40:47Z

core/src/classes/codeConfig.ts

-    prerequisiteIds.push(
-      ...mustachePrerequisiteIds.map((p) => `property:${p}`)
-    );
+  // - property with mustache dependency


now will sort ANY property with a mustache dependency, not just query source properties

teallarson · 2021-08-24T20:42:53Z

core/src/modules/plugin.ts

  ): Promise<string> {
    if (string.indexOf("{{") < 0) return string;

    const data = await getProfileData(profile);
-    return MustacheUtils.strictlyRender(string, data);
+    if (strict === true) return MustacheUtils.strictlyRender(string, data);
+    return MustacheUtils.render(string, data);


This allows non-strict renders for things like returning mustached properties for calculated properties. That means someone can now do things like calculate if ("{{last_purchase_category}}" !== ""){...}

teallarson · 2021-08-24T20:53:04Z

plugins/@grouparoo/calculated-property/public/templates/property/{{{id}}}.js.template

+  ];
+}
+
+function customFunction () {


The function is written outside of the config object to allow for helpful, human-friendly formatting and is then parsed to a string in the template for the plugin to read.

👍

We'll probably add a link to the docs here once they are ready too

plugins/@grouparoo/calculated-property/src/lib/profileProperty.ts

teallarson · 2021-08-24T20:55:57Z

plugins/@grouparoo/calculated-property/src/lib/profileProperty.ts

+    sandbox: {},
+    argv: [],
+    env: {},
+  });


We're allowing console usage, but keeping everything else locked down. External libraries cannot be imported and neither can env variables.

teallarson · 2021-08-24T21:45:37Z

plugins/@grouparoo/calculated-property/__tests__/import/import-property.ts

+  test("it evaluates date strings as expected", async () => {
+    const fn = `() => {
+          const date = new Date("{{lastLoginAt.iso}}");
+          return date.toISOString();


important to note in docs that we return date properties as an object with multiple date formats!

teallarson · 2021-08-25T16:28:39Z

core/src/utils/pluginDetails.js

@@ -55,6 +55,7 @@ function getPluginManifest() {
      grouparoo: { plugins: [] },
    },
    plugins: [],
+    missingPlugins: [],


for logging purposes

teallarson · 2021-08-25T16:36:16Z

plugins/@grouparoo/calculated-property/src/lib/profileProperty.ts

+      profile,
+      false
+    );
+    //fail at every level if someone tries to require a library... this should never be allowed to hit vm.run


This means that running roo validate or roo apply will fail. It fails a bit more abruptly than with other config errors, but I think that's warranted. We don't want anything questionable attempting to be executed. This is one area not covered in our test suite because I couldn't get it to test without throwing the error straight to the console. Open to suggestions there or leaving as is!

I think this can be tested! Wound't this throw in the way you are already testing in __tests__/import/import-property.ts ?

To confirm - this check is to "fail fast" with grouparoo validate if you try to require something or read process.env. Otherwise, the function would be valid, but produce results you don't expect as we block / null out those at runtime.

We can test that process.env is empty with a test like

const fn = `() => { return process.env.DATABASE_URL }`;

and show that it's undefined which might be a good idea.

It wasn't throwing the exact same way, but I've changed it to now

teallarson · 2021-08-25T17:41:48Z

Known limitation: if I generate a property, then roo run, change the function, then roo run again, it will appear to be stuck (does stuff, but # of profiles and imports doesn't change) for a decent chunk of time (>1-2 mins) then eventually will run and recalculate. This is using our 1000 sample profiles, so could be very slow with larger data sets if someone changed their config.

evantahler · 2021-08-25T17:46:14Z

Known limitation: if I generate a property, then roo run, change the function, then roo run again, it will appear to be stuck (does stuff, but # of profiles and imports doesn't change) for a decent chunk of time (>1-2 mins) then eventually will run and recalculate. This is using our 1000 sample profiles, so could be very slow with larger data sets if someone changed their config.

That's interesting! Can you please write this up? I wonder if it works that way for all changed properties (and not just this new plugin). A changed Property does make an internal run...

teallarson · 2021-08-25T17:55:18Z

Known limitation: if I generate a property, then roo run, change the function, then roo run again, it will appear to be stuck (does stuff, but # of profiles and imports doesn't change) for a decent chunk of time (>1-2 mins) then eventually will run and recalculate. This is using our 1000 sample profiles, so could be very slow with larger data sets if someone changed their config.

That's interesting! Can you please write this up? I wonder if it works that way for all changed properties (and not just this new plugin). A changed Property does make an internal run...

Hmm. Yeah, it's not a calculated properties only thing! Adding to tracker.

evantahler

This is looking great! I think there are few things more to test and a little cleanup... and then this is ready to be merged 🎉!

plugins/@grouparoo/calculated-property/__tests__/import/import-property.ts

evantahler · 2021-08-25T17:55:39Z

plugins/@grouparoo/calculated-property/public/templates/property/{{{id}}}.js.template

+  ];
+}
+
+function customFunction () {


👍

We'll probably add a link to the docs here once they are ready too

plugins/@grouparoo/calculated-property/src/lib/propertyOptions.ts

evantahler · 2021-08-25T17:59:14Z

plugins/@grouparoo/calculated-property/src/lib/profileProperty.ts

+    sandbox: {},
+    argv: [],
+    env: {},
+  });


evantahler · 2021-08-25T18:02:05Z

plugins/@grouparoo/calculated-property/src/lib/profileProperty.ts

+      profile,
+      false
+    );
+    //fail at every level if someone tries to require a library... this should never be allowed to hit vm.run


I think this can be tested! Wound't this throw in the way you are already testing in __tests__/import/import-property.ts ?

evantahler · 2021-08-25T18:02:20Z

plugins/@grouparoo/calculated-property/src/lib/profileProperty.ts

+      profile,
+      false
+    );
+    //fail at every level if someone tries to require a library... this should never be allowed to hit vm.run


To confirm - this check is to "fail fast" with grouparoo validate if you try to require something or read process.env. Otherwise, the function would be valid, but produce results you don't expect as we block / null out those at runtime.

We can test that process.env is empty with a test like

const fn = `() => { return process.env.DATABASE_URL }`;

and show that it's undefined which might be a good idea.

evantahler · 2021-08-25T21:01:14Z

plugins/@grouparoo/calculated-property/src/lib/profileProperty.ts

@@ -20,7 +20,7 @@ async function calculateProfilePropertyValue(
    );

    //fail at every level if someone tries to require a library... this should never be allowed to hit vm.run
-    const illegalStrings = [`require(`, `process.env`];
+    const illegalStrings = [`require(`, `process.env`, `async`];


lol, that's one way to do it!

We will need to get async functions working in the next version of this (when we want to support fetch, etc) but I guess we don't need it now

teallarson added the enhancement New feature or request label Aug 16, 2021

evantahler reviewed Aug 17, 2021

View reviewed changes

teallarson force-pushed the calculated-properties-plugin branch 2 times, most recently from c2f014d to 3cf2682 Compare August 23, 2021 20:59

evantahler reviewed Aug 24, 2021

View reviewed changes

Teal Larson and others added 24 commits August 24, 2021 11:27

Add App, Source for calculated property

208812d

calculated app, source. Handle load error.

3da886b

template fix, package.json fix

cf550b9

config template for property

708f38c

plugin connectionMethod

36bb75e

property options method

4c9c9e8

added todo

30e906f

parse list of propertyOptions

48ae493

fix required property object structure

c2f850e

added vm2/node vm

c4ecc3e

move todo to pr

1a44222

add tests file for ci build

caeedb9

basic validation on vm function

10eef9d

tidying up

1405535

start order, mustache vars started

1dcdf43

move to mustache vars

f4280d8

remove comments

a4a1aea

allow non-strict rendering w mustache vars

69fc9b3

cli tests

f5216d4

cleaning up

075d63d

Updating things automatically

7f23883

rebase successful

21a77ff

customFunction is a mustache string

0086106

topological check for mustache options

a3456de

teallarson commented Aug 24, 2021

View reviewed changes

tidying test files

b463fc5

teallarson commented Aug 24, 2021

View reviewed changes

plugins/@grouparoo/calculated-property/src/lib/profileProperty.ts Show resolved Hide resolved

teallarson commented Aug 24, 2021

View reviewed changes

fail validation if customFunction has require(

668d6b7

teallarson commented Aug 24, 2021

View reviewed changes

Teal Larson added 2 commits August 24, 2021 14:57

undo codeConfig initializer startPriority change

d10fa47

set illegalStrings for calculated properties

15a4bf8

teallarson marked this pull request as ready for review August 25, 2021 16:20

evantahler linked an issue Aug 25, 2021 that may be closed by this pull request

Feature Request: Combined Profile Properties #780

Closed

teallarson commented Aug 25, 2021

View reviewed changes

teallarson requested review from evantahler and pedroslopez August 25, 2021 16:36

cleaning up docs

810a844

evantahler suggested changes Aug 25, 2021

View reviewed changes

Teal Larson added 3 commits August 25, 2021 11:41

better validation for illegalStrings/empty fns

7d8357e

ban async, add testing

ea135ce

textarea display for ui

fa773bc

teallarson requested a review from evantahler August 25, 2021 20:12

evantahler approved these changes Aug 25, 2021

View reviewed changes

image fix

c699035

teallarson merged commit 5d13788 into main Aug 25, 2021

teallarson deleted the calculated-properties-plugin branch August 25, 2021 22:12

evantahler mentioned this pull request Aug 31, 2021

Feature Request: Ability to add custom code to our property configs #1721

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Calculated properties plugin #2146

Calculated properties plugin #2146

teallarson commented Aug 16, 2021 •

edited

Loading

evantahler left a comment

teallarson commented Aug 23, 2021

evantahler Aug 24, 2021

teallarson Aug 24, 2021

teallarson Aug 24, 2021

teallarson Aug 24, 2021

evantahler Aug 25, 2021

teallarson Aug 24, 2021

evantahler Aug 25, 2021

teallarson Aug 24, 2021

teallarson Aug 25, 2021

teallarson Aug 25, 2021 •

edited

Loading

evantahler Aug 25, 2021

evantahler Aug 25, 2021

teallarson Aug 25, 2021 •

edited

Loading

teallarson commented Aug 25, 2021 •

edited

Loading

evantahler commented Aug 25, 2021

teallarson commented Aug 25, 2021

evantahler left a comment

evantahler Aug 25, 2021

evantahler Aug 25, 2021

evantahler Aug 25, 2021

evantahler Aug 25, 2021

evantahler Aug 25, 2021

Calculated properties plugin #2146

Calculated properties plugin #2146

Conversation

teallarson commented Aug 16, 2021 • edited Loading

evantahler left a comment

Choose a reason for hiding this comment

teallarson commented Aug 23, 2021

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

teallarson Aug 25, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teallarson Aug 25, 2021 • edited Loading

Choose a reason for hiding this comment

teallarson commented Aug 25, 2021 • edited Loading

evantahler commented Aug 25, 2021

teallarson commented Aug 25, 2021

evantahler left a 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

teallarson commented Aug 16, 2021 •

edited

Loading

teallarson Aug 25, 2021 •

edited

Loading

teallarson Aug 25, 2021 •

edited

Loading

teallarson commented Aug 25, 2021 •

edited

Loading