feat: adds reusable actions

[baywet]

This pull request adds action templates.

fixes #33
fixes #136
fixes #270

closes #238

This is another attempt to solve a scale limitation in the current specification. Action templates are better than the previous parameters proposal because:

• They do not introduce a matrix projection, people may use the template multiple times.
• They do not require "declaring" which environment variables to read in advance.
• They limit the string literal replacement to the templates, which does not impact how actions currently work.
• The template reference mechanism works like OpenAPI references.
• The template resolution mechanism can be isolated as a pre-processing step, and the resolved actions can then be fed to the existing code processing actions.
• It introduces a string literal replacement syntax which can be expanded upon for other sources or used at other places in the future.

The pull request is incomplete as it is, it's a draft, I want to collect feedback on the approach before making any further investments.

[ralfhandl]

I like the general direction.

[baywet]

Feedback from the meeting from both @lornajane and @ralfhandl :

• add a peer block to parameters for the environment variables, this allows to set defaults and "declare" which variables are used. But this declaration is optional.
• add a peer "actions" in the components, that's defined as everything except for the target. So people can specify the target on reference. Reusable actions may use action templates in turns.

[ralfhandl]

Mostly wording and capitalization

[baywet]

@ralfhandl @lornajane I pushed another update a couple of minutes ago. I wasn't happy about the whole reusable actions vs action templates kind of thing. After chatting with @mikekistler internally I realized we could simply define a an action template reference object as "you can override anything from the resolved template in the reference" like JSON schema does to some extent. And keep things extra simple.

Let me know what you think!

[baywet]

Sharing some number from the refactor on our internal repos: we're seeing about 30% reduction on average. Of course that number highly depends on how repetitive your Overlay document is.

[mikekistler]

Looks good. 👍

I think this is a valuable addition to Overlays and will help expand their adoption.

I wish we could fix some of the terminology quirks we have discussed -- in particular "Reusable Action Objects" -- but I don't want to make perfect the enemy of the good.

feedback implemented

[handrews]

A few minor comments and questions.

[handrews]

This seems to be missing the field name and type? I see this is true in the other Object where I commented on RFC2119 usage as well? I also don't see a field name in the schema? I am confused here, is this a fixed field, and if so what is its name?

[handrews]

This comment applies to line 218, I apparently missed when selecting lines 🤦

[baywet]

The goal here was to say something along the lines of: "any field that's defined for the action object, you can also use here", without duplicating those. But if you think a copy paste instead would make the reading easier, I do that instead, what do you think?

[ralfhandl]

Please do not duplicate the fields, that will cause trouble later when fields are added to actions and not here.

[baywet]

Thank you for the input. How do we reword this better so what we mean here is obvious then?

[handrews]

Seems like the right general idea, but definition has JSON Schema-ish re-use connotations... you could just call it fields?

[baywet]

Sure! Always happy to get better name suggestions.

[lornajane]

So the goal is that you can re-use something but adjust it for example by setting a more specific description? I see the use case but I think we need to work on how we describe/represent that. I know Ralf doesn't want the fields duplicated but what if we DO later want to add a field there that can't be used here?

[baywet]

Here is my suggestion for this:

• At the reusable action level, we define "fields" instead. And that's just an Action object with two caveats, all fields all optional, any string field may use the string interpolation syntax. Making this change solves two issues: the risk of collision in the future (e.g. what if we want to introduce a description that's for the reusable action itself?) and the "trick" of saying "well you can use any field defined over there" which is odd for sure.
• At the reference level:
  • we could completely do away with the reference object together, and move the $ref, parameterValues fields in the Action Object, and add conditional statements. I already said that I'm not in favour of that, because we have precedents that conditionals are hard to understand for humans and difficult to encode using JSON schema. (e.g. all the query parameters and headers conditional fields in OAD)
  • we could make the same change that Henry suggested for the reusable actions, move everything in fields. But I don't find that ergonomic when you're working next to action objects. (see examples below)
  • we could leave it as is, but it seems to add confusion when reading the specification.
  • we could move this definition (without duplicating the fields themselves) to a "pattern fields" section like Henry suggested before. At least it'd make it a bit more obvious this definition is "not a regular field" but that it "brings several fields along".

Examples

I'm going to be using JSON on purpose to outline the difference in ergonomics between options 2 or 3 and 4 :

keep at the same level

{
  "actions":
   [
     {
       "remove": true,
       "description": "Regular Action object that removes something",
       "target" : "some target"
     },
     {
       "$ref": "#/components/actions/SomeUpdateAction",
       "target": "some target override"
     }
   ]
}

introduce a fields node at the reference level

{
  "actions":
   [
     {
       "remove": true,
       "description": "Regular Action object that removes something",
       "target" : "some target"
     },
     {
       "$ref": "#/components/actions/SomeUpdateAction",
       "fields":
       {
         "target": "some target override"
       }
     }
   ]
}

In this example I find the extra node quite verbose and not very ergonomic when you look at a few actions and reference objects mixed together. Thoughts?

[lornajane]

I've got a lot of questions about the intent here (which I thought we'd already been through, so please accept my apologies for asking it again). I hadn't seen, or perhaps hadn't internalised, the use cases where target is getting rewritten in the reusable action as well as the action bit. I think this is beyond the scope of what I had previously seen and I'm uneasy about how approachable it is.

Also I think we use REQUIRED for fields that are required, rather than marking the ones that are not as Optional - so that needs to be consistent.

[lornajane]

I had thought it was not allowed to include the target. The target has to be with the action, but the action definition can be referred to as one of the reusable items from components. What's the use case for supporting action?

[baywet]

On this instance specifically, I think you've missed a few discussions, either directly here in the PR history, or on the Overlays call. I don't want to re-litigate the whole thing that late in the process. But at a high level:

• We believe that consistency is key, is there really any good reason to allow every field here but target?
• We believe that string interpolation + target are a powerful combination for advanced scenarios, that's actually lead to a huge reduction in my team's context "similar" actions.
• We believe we can also enable simpler scenarios (people who don't want to mess around with string interpolation), by allowing an override at the reference level.

Hopefully that's enough context, let me know if you still have questions or concerns.

[lornajane]

Does it make sense to have remove in a reusable action? If not I am not sure I would refer to it here.

[baywet]

I think having the reusable action ONLY define remove: true probably does not bring a lot of value in itself. However, if the description field and/or the target fields are using string interpolation, then it becomes interesting. We have this scenario for example where we remove a bunch of "preview" enum values across the document. In that case, it's fairly easy and useful to have a reusable action with the 3 fields, 2 templated (description and target)

[lornajane]

So the goal is that you can re-use something but adjust it for example by setting a more specific description? I see the use case but I think we need to work on how we describe/represent that. I know Ralf doesn't want the fields duplicated but what if we DO later want to add a field there that can't be used here?