[WIP] Feature/properties in boundary definitions #113

v-yussupov · 2018-08-03T13:10:04Z

Add Property Definitions for Properties in Boundary Defintions

With this feature, Properties in Boundary Defintions, in addition to the original plain XML specification, can be specified using a PropertyDefintions extension element that is defined on the level of ServiceTemplate.

What's added:

Properties Definition tab on the level of ServiceTemplate (similar to Node Types, etc)
Apart from Winery's KV proeprties, specification of XML elements/types is also possible
Properties are instantiated based on the specified Properties Definition
If no Properties Definition was specified, properties can be specified as in the original implementation
If existing Properties Definition are removed or their type is changed, e.g., from XML element to KV properties, specified properties and property mappings are also removed
Changes of KV Properties Definition (adding or removing new kv pairs) is reflected in both, properties and property mappings
Property Mappings now have a select element allowing to choose defined KV properties (in case KV properties were used in Properties Definition), otherwise an original input field is kept
Ensure that you followed http://eclipse.github.io/winery/dev/ToolChain#github---prepare-pull-request. Especially, we require a single commit
Ensure that the commit message is a good commit message
Ensure to use auto format in all files
Ensure that you appear in NOTICE at Copyright Holders
Tests created for changes
Documentation updated (if needed)
Screenshots added (for UI changes)

Signed-off-by: Vladimir Yussupov <[email protected]>

clarity-bot · 2018-08-03T13:10:42Z

I generated a structure-diff below from pulling c2f8beb on OpenTOSCA:feature/issue-199 into e34186c on OpenTOSCA:master.

Signed-off-by: Vladimir Yussupov <[email protected]>

Vogel612 · 2018-08-03T15:05:05Z

...pse.winery.model.tosca/src/main/java/org/eclipse/winery/model/tosca/TExtensibleElements.java

@@ -154,6 +154,8 @@ protected void setWPDElement(WinerysPropertiesDefinition res) {
    }

    protected void setWPDNamespace(WinerysPropertiesDefinition res) {
+        // bo be overridden by subclasses willing to use getWinerysPropertiesDefinition()


should this be marked as abstract then, to force said overriding? This class is already abstract so we're not giving up anything for that

I was not sure if all TExtensibleElements actually need WPD support, since initially only TEntityType supported them. This is an attempt to reuse the method in both TEntityTypes and TServiceTemplates without duplicating it and at the same time without putting additional requirements on TExtensibleElements.

lharzenetter

On a first sight, it looked good to me. I have some changes though and I will take a deeper look into it next week.
Please also ensure, that Travis completes successfully.

lharzenetter · 2018-08-07T06:23:46Z

...pse.winery.model.tosca/src/main/java/org/eclipse/winery/model/tosca/TExtensibleElements.java

+    }
+
+    protected void setWPDNamespace(WinerysPropertiesDefinition res) {
+        // bo be overridden by subclasses willing to use getWinerysPropertiesDefinition()


I don't think you need to have this "abstract" method. The implementations in both are similar and you could check if the current class is an instance of HasTargetNamespace, and otherwise use a default one

yeah, that's a good solution if a namespace has to be set in the same way for both TEntityType and TServiceTemplate (currently setWPDNamespace is identical, so I assume there's no need for different implementations)

lharzenetter · 2018-08-07T06:24:51Z

...in/java/org/eclipse/winery/repository/rest/resources/entitytemplates/PropertiesResource.java

 public class PropertiesResource {

    private static final Logger LOGGER = LoggerFactory.getLogger(PropertiesResource.class);

    private AbstractComponentInstanceResource res;
    private TEntityTemplate template;

+    public PropertiesResource() {


Why do you need this?

now it's not needed, I'll remove it (I first made BoundaryDefinitionsPropertiesResource to be a subclass of PropertiesResource and there I needed a default constructor, however I changed it later)

lharzenetter · 2018-08-07T06:29:02Z

...a/org/eclipse/winery/repository/rest/resources/servicetemplates/ServiceTemplateResource.java

@@ -119,6 +120,11 @@ public BoundaryDefinitionsResource getBoundaryDefinitionsResource() {
        return new BoundaryDefinitionsResource(this, boundaryDefinitions);
    }

+    @Path("propertiesdefinition/")


Please no trailing slash

all other subresources (topologytemplate, plans, ...) in ServiceTemplateResource use trailing slashes, I just kept it consistent with the used convention

lharzenetter · 2018-08-07T06:35:57Z

...p/instance/serviceTemplates/boundaryDefinitions/propertyMappings/propertyMappings.service.ts


-        return this.http
+        /*return this.http


Please remove comments

lharzenetter · 2018-08-07T06:37:53Z

...in/java/org/eclipse/winery/repository/rest/resources/entitytemplates/PropertiesResource.java

 public class PropertiesResource {

    private static final Logger LOGGER = LoggerFactory.getLogger(PropertiesResource.class);

    private AbstractComponentInstanceResource res;
    private TEntityTemplate template;

+    public PropertiesResource() {


Why do you need this?

lharzenetter · 2018-08-07T06:38:06Z

...a/org/eclipse/winery/repository/rest/resources/servicetemplates/ServiceTemplateResource.java

@@ -119,6 +120,11 @@ public BoundaryDefinitionsResource getBoundaryDefinitionsResource() {
        return new BoundaryDefinitionsResource(this, boundaryDefinitions);
    }

+    @Path("propertiesdefinition/")


No trailing slashes

lharzenetter · 2018-08-07T18:54:52Z

...pse.winery.model.tosca/src/main/java/org/eclipse/winery/model/tosca/TExtensibleElements.java

+        }
+    }
+
+    protected void setWPDNamespace(WinerysPropertiesDefinition res) {


You could actually implement it on this level since the methods are identical. To be able to use the getTargetNamespace method, you can check if the current object is an instance of HasTargetNamespace and cast it. Otherwise, you could use a default namespace.

Signed-off-by: Vladimir Yussupov <[email protected]>

miwurster · 2019-11-22T10:20:29Z

Hi @v-yussupov, what needs to be done here for the final merge?

v-yussupov · 2019-11-22T10:45:37Z

@miwurster actually, I'm not sure if it's still needed. I think initially Lukas planned to have this feature for the paper, but eventually it was not used. @lharzenetter, is it still needed?

miwurster · 2020-02-14T05:09:15Z

@lharzenetter please revise, finalize and merge this PR ... otherwise, close it.

v-yussupov added 4 commits July 19, 2018 14:59

Add initial implementation

f2fbee3

Signed-off-by: Vladimir Yussupov <[email protected]>

Merge remote-tracking branch 'upstream/master' into feature/issue-199

c7dd8a2

Signed-off-by: Vladimir Yussupov <[email protected]>

Improve initial implementation

9d68b3d

Signed-off-by: Vladimir Yussupov <[email protected]>

Merge remote-tracking branch 'upstream/master' into feature/issue-199

7288da7

Signed-off-by: Vladimir Yussupov <[email protected]>

v-yussupov requested a review from lharzenetter August 3, 2018 13:10

koppor added the in progress label Aug 3, 2018

Remove unused imports

5aad601

Signed-off-by: Vladimir Yussupov <[email protected]>

Vogel612 reviewed Aug 3, 2018

View reviewed changes

lharzenetter changed the title ~~[WIP] Feature/issue 199~~ [WIP] Feature/properties in boundary definitions Aug 4, 2018

lharzenetter requested changes Aug 7, 2018

View reviewed changes

Add requested changes

c2f8beb

Signed-off-by: Vladimir Yussupov <[email protected]>

lharzenetter force-pushed the master branch from e34186c to 50b9f95 Compare August 24, 2018 12:18

miwurster assigned v-yussupov Nov 22, 2019

miwurster closed this Feb 14, 2020

miwurster reopened this Feb 14, 2020

lharzenetter added 2 commits February 14, 2020 10:18

Merge branch 'master' into feature/issue-199

1b974b6

fix missing import

5f2e739

lharzenetter changed the base branch from master to main December 15, 2021 12:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Feature/properties in boundary definitions #113

[WIP] Feature/properties in boundary definitions #113

v-yussupov commented Aug 3, 2018

clarity-bot bot commented Aug 3, 2018 •

edited

Loading

Vogel612 Aug 3, 2018

v-yussupov Aug 3, 2018

lharzenetter left a comment •

edited

Loading

lharzenetter Aug 7, 2018

v-yussupov Aug 8, 2018

lharzenetter Aug 7, 2018

v-yussupov Aug 8, 2018

lharzenetter Aug 7, 2018

v-yussupov Aug 8, 2018

lharzenetter Aug 7, 2018

v-yussupov Aug 8, 2018

lharzenetter Aug 7, 2018

lharzenetter Aug 7, 2018

lharzenetter Aug 7, 2018

miwurster commented Nov 22, 2019

v-yussupov commented Nov 22, 2019

miwurster commented Feb 14, 2020 •

edited

Loading

[WIP] Feature/properties in boundary definitions #113

Are you sure you want to change the base?

[WIP] Feature/properties in boundary definitions #113

Conversation

v-yussupov commented Aug 3, 2018

clarity-bot bot commented Aug 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lharzenetter left a comment • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miwurster commented Nov 22, 2019

v-yussupov commented Nov 22, 2019

miwurster commented Feb 14, 2020 • edited Loading

clarity-bot bot commented Aug 3, 2018 •

edited

Loading

lharzenetter left a comment •

edited

Loading

miwurster commented Feb 14, 2020 •

edited

Loading