feat(iotevents): add DetectorModel L2 Construct

[yamatatsu]

This PR is proposed by #17711.

The first step of the roadmap in #17711 is implemented in this PR.

1. implement DetectorModel and State with only required properties
   • It will not be able to have multiple states yet.

If this PR is merged, the simplest detector model can be created as following:

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

[skinny85]

Looks like a good initial attempt @yamatatsu! But we need to iterate on the API a little more before we can merge this in.

[skinny85]

I feel like we need to present an API for these conditions - just writing expressions in strings won't cut it I think.

[yamatatsu]

That's sounds great! 😍
Recently, I knew mapping-tenplate.ts in aws-appsync. I wanna try to design like it! (Or is there any other code I can refer to?)

(And I wanna try to design IoT Rules expressions that we discussed this earlier. I couldn't imagine implementing it at all before but.)

[skinny85]

I think that's the closest similar thing we have for something like this in the CDK currently, yes.

Can you link me to the AWS docs for this functionality? I want to make sure I have a good understanding of the language that's permitted here.

[yamatatsu]

Sure! But, as far as I can find, the only document that explains "what a condition is" is the CloudFormation document.
This CFn document is described that condition is expression returning boolean. The refferences of Expressions are here.

And bellow is examples of condition from Detector model examples .

"condition": "!$variable.noDelay && $variable.enteringNewState"

"condition": "(((($variable.averageTemperature * ($variable.sensorCount - 1)) + $input.temperatureInput.sensorData.temperature) / $variable.sensorCount) < ($variable.desiredTemperature - $variable.allowedError))"

"condition": "currentInput(\"AWS_IoTEvents_Blueprints_Heartbeat_Input\")"

"condition": "timeout(\"awake\")"

Pull request has been modified.

[yamatatsu]

@skinny85

I'm sorry it took me so long to respond.

I sort out the rest issues.

1. add validation that the initialState of the DetectorModel has at least one Event with a non-undefined condition

   • from feat(iotevents): add DetectorModel L2 Construct #18049 (comment)
   • and feat(iotevents): add DetectorModel L2 Construct #18049 (comment)

2. add getCurrentInputCondition() to Input

   • from feat(iotevents): add DetectorModel L2 Construct #18049 (comment)

3. to present an smart API for conditions as mapping-tenplate.ts in aws-appsync instead of just string

   • from feat(iotevents): add DetectorModel L2 Construct #18049 (comment)

These issues is related each I think. That we should add are smart API and validation for it.

I will try to implement these features in this PR.

[skinny85]

Awesome work @yamatatsu! I love the Expression API, looks much cleaner than just mucking around with string expressions.

I have a few notes, but nothing major, mainly decreasing the surface area of the public API of this module.

[skinny85]

I have to say, I find the constant name changes here super confusing 😕. This is a good example. We are writing documentation for a property called onEnterEvents, its type is Event[], and the description says "Specifies the actions that are performed". Doesn't even mention the word "event" anywhere.

Are we missing something here? Should what in this PR is called an Event actually be some sort of Action?

[yamatatsu]

Naming

In CloudFormation, State has OnEnter and OnEnter has only events (typed as Event[]). I easily concated onEnter and events...

If I named it just onEnter, would it be less uncomfortable?

JSDoc

I should fix it!

[yamatatsu]

I fix it in 78fc88c (#18049).

[skinny85]

I actually don't like this method. This is actually a DetectorModel concern, not a State concern, that this validation has to be performed.

Can we remove this method, and perform this validation in DetectorModel by calling toStateJson() on its initialState instead?

[yamatatsu]

If we implement this validation with toStateJson(), It will be as following I think:

const stateJson = props.initialState._toStateJson();

if (!stateJson.onEnter?.events.some(event => event.condition)) {
  throw new Error('Detector Model must have at least one Input with a condition');
}

But we can't get onEnter?.events because onEnter can be cdk.IResolvable, same for events.some() and event.condition.

Should toStateJson() return more exact type that is compatible with CfnDetectorModel.StateProperty instead of CfnDetectorModel.StateProperty itself?

Following is the type CfnDetectorModel.StateProperty that toStateJson() return:

namespace CfnDetectorModel {
  export interface StateProperty {
    readonly onEnter?: CfnDetectorModel.OnEnterProperty | cdk.IResolvable;
    readonly onExit?: CfnDetectorModel.OnExitProperty | cdk.IResolvable;
    readonly onInput?: CfnDetectorModel.OnInputProperty | cdk.IResolvable;
    readonly stateName: string;
  }
}

[skinny85]

OK, I see the problem. I think we could just check whether onEnter is a CfnDetectorModel.OnEnterProperty, and skip the validation if it's an IResolvable. But I don't want you to write all of that code if a simpler alternative is possible here.

So, let's keep _toStateJson() as it is now, but let's make this method @internal, and rename it to _onEnterEventsHaveAtLeastOneCondition().

[skinny85]

Can we make this method @internal? I don't think there's a need to expose it in the public API of this module.

It will require renaming it to _toStateJson().

[skinny85]

I would recommend putting this in a beforeEach() - it's repeated in every test.

Pull request has been modified.

[skinny85]

Looks awesome @yamatatsu. Let's rename that one method in State, and make it @internal, and this will be ready to get merged.

[skinny85]

OK, I see the problem. I think we could just check whether onEnter is a CfnDetectorModel.OnEnterProperty, and skip the validation if it's an IResolvable. But I don't want you to write all of that code if a simpler alternative is possible here.

So, let's keep _toStateJson() as it is now, but let's make this method @internal, and rename it to _onEnterEventsHaveAtLeastOneCondition().

[skinny85]

Thanks for the contribution @yamatatsu!

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
• Commit ID: 2b4301e
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).