DCS with namespace target, don't apply decorators on top of the namespace.

[sanketshevkar]

Bug Report 🐛

The following model definition is valid...

@Term("My HR Model")
@Term_description("Reusable data definitions for the HR department.")
namespace acme.hr@1.0.0

Current Behavior

When we apply decorator using dcs targeting the namespace,

{
        "$class": "org.accordproject.decoratorcommands@0.3.0.DecoratorCommandSet",
        "name": "web",
        "version": "1.0.0",
        "commands": [
            {
                "$class": "org.accordproject.decoratorcommands@0.3.0.Command",
                "type": "UPSERT",
                "target": {
                    "$class": "org.accordproject.decoratorcommands@0.3.0.CommandTarget",
                    "namespace": "test@1.0.0"
                },
                "decorator": {
                    "$class": "concerto.metamodel@1.0.0.Decorator",
                    "name": "Form",
                    "arguments": [
                        {
                            "$class": "concerto.metamodel@1.0.0.DecoratorString",
                            "value": "inputType"
                        },
                        {
                            "$class": "concerto.metamodel@1.0.0.DecoratorString",
                            "value": "text"
                        }
                    ]
                }
            }
        ]
    }

We get the following output

namespace test@1.0.0

@Form("inputType","text")
scalar SSN extends String

@Editable
@Form("inputType","text")
concept Person {
  @Custom
  o String firstName
  o String lastName
  o String bio
  o SSN ssn
  o String address1
  o String address2
  o String city
  o String country
  o Integer zip
  o Dictionary dictionary
}

@Form("inputType","text")
map Dictionary {
  o String
  o String
}

@Form("inputType","text")
map Rolodex {
  o String
  o String
}

Expected Behavior

We should get the following output:

@Form("inputType","text")
namespace test@1.0.0

scalar SSN extends String

@Editable
concept Person {
  @Custom
  o String firstName
  o String lastName
  o String bio
  o SSN ssn
  o String address1
  o String address2
  o String city
  o String country
  o Integer zip
  o Dictionary dictionary
}

map Dictionary {
  o String
  o String
}

map Rolodex {
  o String
  o String
}

Possible Solution

1. Breaking change - fix the logic which targets namespace. This would be a breaking change for the users who use namespace targeted dcs to apply decorators to all the decorators in that namespace.
2. Introduce a new property applyToAllDeclarationsInNamespace or something better, which defaults to true. Can be set to false if we don't want to apply to declaration and only at the top of the namespace. Validation could be tricky and would be confusing for users to pickup.
3. New target option target: "something-new", which would target only the namespace,

I'm more inclined towards the first options if we are able check if the impact is minimal or null. Or else the 3rd option, but I feel we will have to come up with some property value for the target that signal that this is for applying decorator at the top of a namespace, which I feel namespace itself conveys the best.

Steps to Reproduce

https://replit.com/@sanketshevkar/AccordProjectConcerto-Decorator-Command-Set

Context (Environment)

Desktop

• OS: [e.g. macOS]
• Browser: [e.g. Chrome, Safari]
• Version: [e.g. 0.22.15]

Detailed Description

Possible Implementation

[dselman]

I lean towards option one as well. Perhaps controlled by a new flag passed to DecoratorManager?

[sanketshevkar]

I lean towards option one as well. Perhaps controlled by a new flag passed to DecoratorManager?

Something like ENABLE_NAMESPACE_TARGET_FIX?

[sanketshevkar]

I'll also like to propose the use of [process.emitWarning()](https://nodejs.org/api/process.html#processemitwarningwarning-type-code-ctor) we can emit [DeprecationWarning](https://nodejs.org/api/process.html#nodejs-warning-names) that can caught, handled or suppressed as needed by the consumer of our APIs. Even NodeJS uses this to issue their deprecation warnings and we can create our own custom warnings. I was not able find if console.warn supports similar features. It just outputs the warning to stderr.

[sanketshevkar]

emitWarning(`Functionality for namespace targeted Decorator Command Sets has beed changed.`, {
    type: 'DeprecationWarning',
    code: 'concerto-dep:001',
    detail: 'See https://concerto.accordproject.org/dcs-target-namespace'
});

[sanketshevkar]

I had an assumption that we could target decorators at Concept, ConceptDeclaration etc types, as well from the metamodel model. But that isn't the case, should we that add that functionality as well? That would be helpful for consumers who would be using the deprecated logic.