New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ambiguous methods created for the Creation Factory in the Mgr class #24

Closed
berezovskyi opened this Issue Aug 29, 2018 · 11 comments

Comments

Projects
None yet
2 participants
@berezovskyi
Copy link
Member

berezovskyi commented Aug 29, 2018

The following model

image

leads to the generation of the following code:

image

I have no simple workaround for this 馃

@berezovskyi

This comment has been minimized.

Copy link
Member Author

berezovskyi commented Aug 29, 2018

@jadelkhoury you were asking to separate "good to fix" from "need to fix" bugs. I have added the label "blocker" to this bug to indicate it needs to be fixed. Will that do?

@jadelkhoury

This comment has been minimized.

Copy link
Contributor

jadelkhoury commented Aug 29, 2018

@jadelkhoury you were asking to separate "good to fix" from "need to fix" bugs. I have added the label >"blocker" to this bug to indicate it needs to be fixed. Will that do?

It's one component that helps. But We need to have a process in place for your project.

@jadelkhoury

This comment has been minimized.

Copy link
Contributor

jadelkhoury commented Aug 29, 2018

@berezovskyi The generator is not fool-proof and that's an issue I am aware of.
The generator does (stupidly) follow the model and produce a method for each such CreationFactory in the model.
One reason to have multiple CreationFactories for the same resource PlanExecutionRequest is because one has different parameters for such creations. If that is hte cause, different method signatures will be produced.
But in your case, it seems that you have the same signiture. So what is the purpose of multiple creationFactories for the same resource?
Can you possible just have one? Maybe that CF is separate/independant from the other services and/or ServiceProviders?

@jadelkhoury

This comment has been minimized.

Copy link
Contributor

jadelkhoury commented Aug 29, 2018

I guess a more robust generation would produce a single method for all these CF capabilities.
But I don't think that would help with your String paramter beltId, so it seems to be that you somehow want different methods for these differnet CFs?

@jadelkhoury

This comment has been minimized.

Copy link
Contributor

jadelkhoury commented Aug 29, 2018

and here's a workaround for the time being.

Set the "Creation URI" of the 3 CreationFactories to be the following (in some random choice)

  • create
  • create/{aaa}
  • create/{aaa}/{bbb}

Generate and note that each creation method in the manager will have a different signature now 馃憤

@berezovskyi

This comment has been minimized.

Copy link
Member Author

berezovskyi commented Aug 29, 2018

@berezovskyi

This comment has been minimized.

Copy link
Member Author

berezovskyi commented Aug 29, 2018

berezovskyi added a commit to EricssonResearch/scott-eu that referenced this issue Aug 30, 2018

Apply workaround for identical (cap,shape) pairs
See eclipse/lyo.designer#24 for more info

Signed-off-by: Andrew Berezovskyi <andriib@kth.se>
@berezovskyi

This comment has been minimized.

Copy link
Member Author

berezovskyi commented Aug 30, 2018

Thanks @jadelkhoury, the signatures indeed do not clash any more. But then, I have to insert some random strings for those path parameters...

How hard would it be to apply a correct fix? I have traced the name of the creation factory method in the adaptor manager class to the following query:

creationMethodName(aCreationFactory: CreationFactory)

In other queries, there are already calls to aCreationFactory.eContainer(Service) and I think we can get the SP the same way, e.g.

aCreationFactory.eContainer(Service).eContainer(ServiceProvider)

Actually, there is a query containingServiceProvider, which does the same. Why do we need to call oclAsType method, by the way?

I was not able to find the query that would return the SP namespace, but the closest I got was this:

aService.containingServiceProvider().instanceURI()

Jad, what do you think?

@jadelkhoury

This comment has been minimized.

Copy link
Contributor

jadelkhoury commented Aug 30, 2018

I am not sure how accessing the containing ServiceProvider be of help here.

If I am to take your example above, my suggestion is to add some intelligence to the generator so that the 1st and 3rd methods (which are exactly the same) are only generated once.
Note that this does not solve the problem completly since the 2nd method still has the same Java method signature. But in that case, I think there is something fundamentally wrong with the model that the generator should not hide? (unless I misunderstood it).

That is, the generator avoids the duplication of methods if they have exactly the same set of parameter names, as well as return type.
This is NOT the same as having the same Java signature (where only number and types of paramters matter).

@jadelkhoury

This comment has been minimized.

Copy link
Contributor

jadelkhoury commented Aug 30, 2018

But if it can be motivated that the 2nd method is indeed different from the 1st/3rd method, then we need to find another solution.

@jadelkhoury

This comment has been minimized.

Copy link
Contributor

jadelkhoury commented Aug 30, 2018

Alternative (ugly?) solution:
if there are more than 1 CF for a particular resource type, we name each method "createPlanExecutionRequestForCreationFactory1". Where "CreationFactory1" is actually the name of hte CreationFactory. So in your case, you will need to give the factories different names yourself.

What I like with this approach is that the method names are quite clean and unchanged from current behavioru for 99% of the cases where there is only 1 CF per method.

jadelkhoury added a commit that referenced this issue Sep 5, 2018

closes #24: Ambiguous methods created for the Creation Factory in the
Mgr class

The problem occur when there are 1+ capabilities (such as
CreationFactory) within an adaptor that manage the same (set of)
resources. This results in multiple methods in the AdaptorManager that
have the same signature.
To remedy, for such 1+ capabilities, append the name of the capability
to the method. For example, a creation factory - with title CF1 - on
Requirement will result in method called "createRequirementForCF1"

This change also made it necessary to have a additional method in
AdaptorManager to handle the creation method from a CreationDialog
(previously, both CreationDialog and CreationFactory) shared the same
createResource method in Manager.

@berezovskyi berezovskyi added this to the 2.4.0 milestone Nov 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment