roboconf / roboconf-platform

The core modules and the platform
Apache License 2.0
35 stars 11 forks source link

Prefix required for inherited attribute of component has no sens #420

Closed cdeneux closed 8 years ago

cdeneux commented 9 years ago

According to the following graph

PetalsJBIComponent {
    installer: script;
    exports: componentId = my-component-id;
}

PetalsSL {
    extends: PetalsJBIComponent;
    exports: componentType = SL;
}

PetalsSLPostgreSQL {
extends: PetalsSL;
exports: present = true;
}

My instances must be written as:

instance of PetalsContainer-VM {
name: container-bootstrap-vm;

instance of PetalsContainerBootstrap {
    name: container-bootstrap-node;

    PetalsContainerBootstrap.containerId: container-bootstrap-node;
    domainName: roboconf-demo;
    subdomainName: roboconf-demo-1;

    instance of PetalsSLPostgreSQL {
        name: petals-sl-postgresql;

        # TODO: Why PetalsJBIComponent must be used ? We should use only 'componentId'
        PetalsJBIComponent.componentId: petals-sl-postgresql-9.4-1201-jdbc4;
    }
}

I don't understand why I must use PetalsJBIComponent.componentId when defining the instance PetalsSLPostgreSQL. The prefix PetalsJBIComponent has no sens.

Moreover, with others Roboconf components, I must define instances where the same attribute must be declared twice with the same value but with different prefix, as:

instance of PetalsSEActiviti {
        name: petals-se-activiti-1;

        PetalsJBIComponent.componentId: petals-se-activiti;
        PetalsSEActiviti.componentId: petals-se-activiti;

        instance of SU-Activiti-VacationRequest {
            name: su-activiti-vacationService-provide;
        }
}

As the components are inherited, it has no sens to use a prefix to declare attributes. An inherited attribute has always the value defined at the deepest inherited level.

vincent-zurczak commented 8 years ago

According to this documentation, it looks like a bug.

Using componentId should override all the componentId variables, no matter their prefix. I will test your graph portion.

vincent-zurczak commented 8 years ago

Ho! Wait! We are talking about instances. OK... :smiling_imp:

vincent-zurczak commented 8 years ago

OK. I changed the resolution policy.

  1. First, during validation, when we notice an instance variable has no prefix while it could mean several component ones, we add a warning and not an error anymore.
  2. When we resolve exported variables and that no prefix is present for "ambiguous" overriding, we override all the component variables. And we also add the default variable.

I will update the documentation about it.

vincent-zurczak commented 8 years ago

The documentation was updated: http://roboconf.net/en/user-guide-snapshot/inheritance-and-variables.html

cdeneux commented 8 years ago

The warning is disturbing. When building, we are thinking that something is wrong but not.

vincent-zurczak commented 8 years ago

By definition, a warning is something one should care about. Something that could be an error. ;)