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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bringing Umple's APIs to the metamode and runtime #962

Open
vahdat-ab opened this Issue Jan 21, 2017 · 38 comments

Comments

Projects
None yet
4 participants
@vahdat-ab
Member

vahdat-ab commented Jan 21, 2017

Umple generates a hand full of useful APIs for attributes and association. Currently, those methods are generated by UmpleTL. It means they are not available in the runtime model of Umple systems. This limits us regarding many analysis we can do at the runtime and causes to have duplication in UmpleTL's template files. Furthermore, This will be a great base for the future extensions such as using Language Server Protocol.
One of the concerns is how to deal with the executable body of these methods. I think we can create those APIs (their signature at the modeling level) and then tag them as auto-generated. Then code generated will know how to deal with that.
This idea can be extended in an advance form to state machines.

More info about those APIs can be found here

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Jan 21, 2017

Another concern might be the size of the model at the runtime. I think this can be dealt later with some optimization. It might not be even an issue with the current advance in technology.

@TimLethbridge

This comment has been minimized.

Member

TimLethbridge commented Jan 21, 2017

Sounds interesting, but I think explaining what you mean by example would be important. Give an example trivial umple model with two classes, an association and an attribute, and describe the effect of what you would like to see

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Jan 21, 2017

This is in the direction relationship with issue #462

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Jan 30, 2017

Consider the following example,

class A{
  Integer x;
  0..1 -- * B;
}
class B{ 
}

Umple generates API for the attribute x and also the association 0..1 -- * B; The list of APIs for different elements are listed here
These APIs are generated by the different code generators (Java, PHP, etc). It means if a modeling element wants to check a method called void getX() in the class A, the list of methods for the class A will not show it. In these cases, the developer of that element starts to hack the code. The person implements another function which reacts like the one exists in the code generators and produce the list of auto-generated methods (sometimes, it's just a manual list). This brings lots of boiling code. Furthermore, it's hard to update to date all those manual codes with their own hard-coded names. This becomes much worse for cases that we do lots of post checking to make sure the model is valid.
For example, to check if there is no conflict between used-defined methods and auto-generated methods.
By having those methods at the metamodel level, we guarantee that the regular methods such as getAllMethods will always show the right number of methods. Then, a normal iteration on this list can satisfy many post-checkings.
Another example,

class A{
  Integer x;
}
trait T{
  Integer getX();
  void setX();
  void pMethod(){
  //***
  }
}

This example points out that the client of T must have two methods called getX and setX. When the algorithm tries to check the list of methods of the class A, there are not such methods. The algorithm needs to go through a complicated process to figure out if there are some auto-generated methods or not.

I believe this a great step for reducing dependency on code generators and have more concrete metamodel. If Umple wants one day in the future to have its own binary code, so there won't be any issue regarding the APIs.
It also brings many benefits for having a smart compiler at the IDE level (a future roadmap). For example, if users press ctrl+space, they will see those auto-generated methods as methods they can call for a specific class

@katcavers

This comment has been minimized.

Contributor

katcavers commented Feb 5, 2017

@vahdat-ab I have started trying to orient myself with some of the compiler code, and I want to make sure I am on the right track. I found the method analyzeAttribute in UmpleInternalParser_CodeClass.ump. Is this the correct point to identify attributes to add API methods for?

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Feb 7, 2017

@katcavers yes. this is the place Analyzer analyzes the abstract syntax tree

@katcavers

This comment has been minimized.

Contributor

katcavers commented Feb 20, 2017

@vahdat-ab
I found I was getting a whole bunch of errors when I build because duplicate methods are being generated. This makes sense, as my change adds a get method for any attribute to the model, and the generator generates the API methods as well as adds any methods existing in the model; so two get methods with the same name are being added for each attribute.

I remember saying that we wanted the generation of the API methods to remain in the generators. So, I have added an additional type, fAutoAPI, to the Source enum of the Method class and set the get method's source to this. Then, I check in the Java generator class_MethodDeclaration.ump and if the source of the method is fAutoAPI I skip generating this method. Is this the right approach?

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Feb 21, 2017

To me, it looks ok. I wonder if you detected for what reason fAuto is used? I thin it's there for this purpose.
The way I was thinking is this:
At the code generation level, the generator detects an auto-generated method, and then it generates the method along with the necessary implementation.

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Feb 21, 2017

let's go in the way you describe. If we want to do any modification later it will be at the code generation level. You just make sure you do all required checkings at then modeling level.

@katcavers

This comment has been minimized.

Contributor

katcavers commented Feb 21, 2017

Sounds good.I think eventually implementing the auto-generated methods to be generated by the generators makes sense. This would just be a huge change to the code generation side, so it is easier to do this step first.

@katcavers

This comment has been minimized.

Contributor

katcavers commented Feb 24, 2017

I am having an issue with my changes and the Ruby generator. Many of the test cases are failing because empty lines are being inserted. I believe it may be due to the fact that we skip over autogenerated API methods, but somehow a new line is still inserted. For example, I have attached two files; the expected vs actual Ruby code.
The only difference is there are two extra empty lines at the end of the actual output.
actual.txt
expected.txt

@katcavers

This comment has been minimized.

Contributor

katcavers commented Feb 24, 2017

@vahdat-ab I also have two more questions.

First, in our initial discussion for this project, we talked about constructors. Would you be able to expand again on whether I should be creating/modifying constructors when an attribute is encountered in the model?
Second, you also mentioned I should make a method that could remove these autogenerated methods from the model. Would this be a method that removes all the API methods, or a method that could take for example an attribute as a parameter, and delete the API methods for only that attribute?

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Feb 24, 2017

@ahmedvc ----> The issue Resolved

but somehow a new line is still inserted. For example, I have attached two files; the expected vs actual Ruby code.

I wonder if you can comment on this? How can we remove those empty lines generated because of passing some conditions?

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Feb 24, 2017

First, in our initial discussion for this project, we talked about constructors. Would you be able to expand again on whether I should be creating/modifying constructors when an attribute is encountered in the model?

yes. This is a critical issue. You should create a constructor and feed its parameters.

Would this be a method that removes all the API methods, or a method that could take for example an attribute as a parameter, and delete the API methods for only that attribute?

The method should have a parameter such as an attribute or association and then remove related auto-generated APIs. You can also implement a method that removes all of them. This is going to be easy because there is a property that specifies which methods are auto-generated.

@katcavers

This comment has been minimized.

Contributor

katcavers commented Feb 24, 2017

I think I actually just figured out why it's happening. Before, there were no methods in the model for this specific case. But now there are, as I am adding get and set API methods at the model level. In the generator, a check is done on the class being processed is done; if the class has methods, it then iterates through all the methods and at the end appends to the string builder two new lines. For this case, the if statement used to prove false, but now it is true because of the API methods I add at the model level. So even though no code is generated for these methods, those two new lines are added.

@katcavers

This comment has been minimized.

Contributor

katcavers commented Feb 24, 2017

Thanks Vahdat, I will continue working on these.

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Feb 24, 2017

@katcavers great.

@katcavers

This comment has been minimized.

Contributor

katcavers commented Mar 6, 2017

@vahdat-ab For the methods to remove autogenerated methods, I believe it makes sense to add these methods to the UmpleClassifier class in Umple.ump. Do you agree, or is there some other place you had in mind? Thanks!

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Mar 6, 2017

why do yo want to have it in UmpleClassifier?
To me, it makes sense to have because we can have attributes in UmpleClass and UmpleTrait. Do you have other justifications?

@katcavers

This comment has been minimized.

Contributor

katcavers commented Mar 6, 2017

Ah, actually, yes your way does make more sense. I think I was thinking to have it at the higher level of the hierarchy, but UmpleClass is the only one to have attributes, so adding it there is the better way to go.

@katcavers

This comment has been minimized.

Contributor

katcavers commented Mar 19, 2017

@vahdat-ab Hi Vahdat! I have submitted now to my PR the functionality for constructors. I am now going to be moving on to Associations. There are a lot more autogenerated API methods for associations, so I am thinking I should start first with simple get and set again?

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Mar 20, 2017

OK. Thanks. do it as you wish :-)

@TimLethbridge

This comment has been minimized.

Member

TimLethbridge commented Mar 20, 2017

For associations we have get, set and add (among many). I wouldn't add set unless I also added add, otherwise it will be a bit off-balance; set is used for '1' ended associations and add for '*'.

@katcavers

This comment has been minimized.

Contributor

katcavers commented Mar 21, 2017

Thanks for pointing that out Tim! I will follow your advice.

@katcavers

This comment has been minimized.

Contributor

katcavers commented Mar 22, 2017

@vahdat-ab I wanted to give you an update. I am testing out and trying to figure out how I am going to handle associations. I have realized that I will have to handle them a bit differently than attributes. For attributes, I was able to add the autogenerated methods right away, because the class they were associated with had already been added to the model. However, for associations both ends of the association may not have been added to the model yet, and so I am not able to add the autogenerated methods from analyzeAssociation(). What I am thinking I will do is add them when postTokenClassAnalysis() is called. This way, I will be able to access all the classes I need. Another option is to add some at the analyzeAssociationLevel, and then create a list similar to unlinkedAssociationVariables() where I keep track of the ends for which I need to generate methods, and then generate them at the postTokenClassAnalysis().

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Mar 22, 2017

Putting the code in postTokenClassAnalysis() is the right place. This makes it more module and predictable. Even, you could do the same for attributes :-).
NOTE: The way you implemented attributes is faster than putting it in postTokenClassAnalysis(). However, it is not modular.

@katcavers

This comment has been minimized.

Contributor

katcavers commented Mar 29, 2017

@vahdat-ab Hi Vahdat, I am currently trying to make tests for the autogenerated association methods. After our meeting last Friday, I went and looked at all the tests there are for the generators. This really opened my eyes to how many cases there are. I am looking at the tests in cruise.umple.test.implementaion (ManyToManySubclassTest, all the way down to UnidirectionalOptionalOneTest). It's a huge number of tests! Up until now, my approach has been to parse an umple file, get the UmpleClass object, and assert that it has the correct methods (an example of which you could find here: bf008d7).

This approach was fine for attributes, because the number of cases was much smaller. I could still use this approach for associations, but there are so many more cases/tests, and so many more methods that I have to check for. Do you have any suggestions/ideas of what I could do to make testing more efficient?

I want to write tests first, as I think this will make the implementation go much faster. Thanks!

@TimLethbridge

This comment has been minimized.

Member

TimLethbridge commented Mar 29, 2017

Yes, as I mentioned in the meeting last Friday, the API generated for associations is considerating, and the logic for determine what will be generated is non-trivial (there are I think about 14 cominations of multiplicity, but there are also other factors that change the API). This took a whole PhD (Andrew Forward) and has been evolved since then to add features and fix bugs.

I think that doing this manually will be a challenge. The logic for the set of methods to be generated is already encapsulated in the generator; you would either need to reverse engineer this and hence create a module that somehow duplicates it, although the module will not be as complex as the generators since you do not care about the contents of the bodies.

In the long run it will be necessary to constantly verify that whatever is generated matches the signatures found in the model. Otherwise changes may result in mismatches creeping in. We can take advantage of that by right now starting wtth that verifier: Instrument the Java generator to emit a message (only in a debugging mode) when it generates a method that does not have a corresponding method in the model, and vice-versa. i.e. it checks methods off lists are they are generated and then spits out the difference.

Next you start adding the methods to the model and watch the above list of messages about mismatches getting smaller and smaller. You won't be able to be perfect in the week or so you have left, but you can get part way, and leave it for another student to finish.

@katcavers

This comment has been minimized.

Contributor

katcavers commented Mar 29, 2017

@TimLethbridge Thanks for the clarification Tim. I will start doing as you have suggested. Do you know of anywhere in the Umple project where something similar, in terms of debugging trace statements, has been done that I could take a look at?

@katcavers

This comment has been minimized.

Contributor

katcavers commented Apr 2, 2017

@vahdat-ab After looking more into the Trace capabilities (I looked at both the code and user manual), I do not think I will be able to use it. While many of the ideas are shared between Trace elements and what I need, I think that I would be using the Umple Trace in a way it was not necessarily intended, and would have to make changes to it that would not be in line with its purpose. I am going to go forward with creating my own implementation of debug statements to verify autogenerated methods.

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Apr 3, 2017

@katcavers

This comment has been minimized.

Contributor

katcavers commented Apr 4, 2017

@vahdat-ab Hi Vahdat, I have finished putting in my debug statements and am now setting up tests for the autogenerated associations. However, I noticed there are a large number of files in the UmpleTL folder of UmpleToJava which do not seem to be used. For example, association_SetOneToOptionalN.ump. I did a search for their code in the project in Eclipse (including generated code), and nothing came up. I want to make sure that I am not missing anything as I will be relying somewhat on these debug statements. Are these just old files that were once used, and just haven't been deleted? Or are they being used somewhere other than the generator?

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Apr 4, 2017

@katcavers I'm not aware of them.
@TimLethbridge might know about them.

@TimLethbridge

This comment has been minimized.

Member

TimLethbridge commented Apr 4, 2017

Hmmm. I don't know. You say 'a large number'. If it was a few I would think they might have been left behind. But I would be rather surprised if it is a large number. One strategy would be to delete them temporarily and do a full build and see what happens! remember that these files are compiled separately; then the generated code is transferred from their directory to the general umple generated code directory.

@katcavers

This comment has been minimized.

Contributor

katcavers commented Apr 5, 2017

@TimLethbridge @vahdat-ab After your comments, I realized last night that I had missed files used in JavaSpecGenerator - I went and added the debug statements to the files there, and the number of unused files is a small one now. Oops! And thanks! :)

@katcavers

This comment has been minimized.

Contributor

katcavers commented Apr 14, 2017

@TimLethbridge @vahdat-ab Documentation has been added: https://github.com/umple/umple/wiki/Adding-Autogenerated-Methods-at-the-Model-Level

I am in the middle of exams, so I will keep adding to it as I have time and think of what else I should write.

@katcavers

This comment has been minimized.

Contributor

katcavers commented Apr 16, 2017

NOTE: Read above wiki page. All work for associations has been done in branch issue962.

@ZainabAlShowely

This comment has been minimized.

Contributor

ZainabAlShowely commented Jul 6, 2018

I will attempt to replace the previous testing method that was implemented with a second one, both are explained on the wiki page.

TimLethbridge added a commit that referenced this issue Jul 18, 2018

Merge pull request #1336 from umple/Issue_962
Progress on #962 Adds some associations autogenerated methods to the model and adds testing

TimLethbridge added a commit that referenced this issue Aug 29, 2018

Merge pull request #1356 from umple/Issue_962
Progress on #962 Adds autogenerated methods to the model for associations, attributes and state machines
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment