Bug 258973 - OCLStandardLibraryImpl is neither extensible nor reusable.
Summary: OCLStandardLibraryImpl is neither extensible nor reusable.
Status: CLOSED FIXED
Alias: None
Product: OCL
Classification: Modeling
Component: Core (show other bugs)
Version: 1.2.0   Edit
Hardware: PC Windows Vista
: P3 enhancement (vote)
Target Milestone: M5   Edit
Assignee: Adolfo Sanchez-Barbudo Herrera CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2008-12-16 12:14 EST by Adolfo Sanchez-Barbudo Herrera CLA
Modified: 2011-05-27 02:48 EDT (History)
2 users (show)

See Also:


Attachments
Proposed patch (8.47 KB, patch)
2009-01-08 06:44 EST, Adolfo Sanchez-Barbudo Herrera CLA
no flags Details | Diff
(Final?) Proposed patch (8.09 KB, patch)
2009-01-15 13:38 EST, Adolfo Sanchez-Barbudo Herrera CLA
give.a.damus: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adolfo Sanchez-Barbudo Herrera CLA 2008-12-16 12:14:00 EST
MDT OCL has its Standard Library in an internal package. Since QVTo spec has its own implementation to extend the library with new types and operations, it would be useful for me having that OCLStandardLibraryImpl accessible in somehow:

1. Moving the OCLStandardLibraryImpl to a non-internal package to be able to use that class. 

2. Creating a new extensible UML/EcoreOCLStandardLibraryImpl which would use the correspondent internal OCLStandardLibraryImpl.

Do you have any preference?

Cheers,
Adolfo.
Comment 1 Christian Damus CLA 2008-12-21 10:06:31 EST
My preference is for the former.  Why create more classes?  Of course, we would probably like to clean up the existing class and expose only  as much to subclasses as is really required.  And probably there would be very little public API (mostly protected).

I think you already have a patch for this, is that right?  When you have a patch ready, attach it and assign the bug to me.
Comment 2 Adolfo Sanchez-Barbudo Herrera CLA 2008-12-22 06:20:57 EST
(In reply to comment #1)
> My preference is for the former.  Why create more classes?  Of course, we would
> probably like to clean up the existing class and expose only  as much to
> subclasses as is really required.  And probably there would be very little
> public API (mostly protected).

The main reason is that there are several public static methods which are used by internal classes (TypeResolver, UMLReflection, etc). Most of those methods aren't probably necessary to belong to new API.

One static class (the internal) can be considered as the stdlib builder, and the other (public one) can be considered as the OCLStandardLibrary<C> implementation which only only provides implementation for that interface (and maybe few static methods).

> I think you already have a patch for this, is that right?  When you have a
> patch ready, attach it and assign the bug to me.
> 

Yes I do. However I would like to clarify with Mariano some aspects related to the QVTo stdlib (some issues sent recently), before. There are some unclear points about the OCL stdlib must exactly be extended.

On the other hand, if option 1 is the preferred, I'll have to make an exhahustive of which of those methods are really needed. I want to be quite sure that we don't need any patch fater this one ;).

Can we wait until middle on january for this? API freeze is M6, isn't it?.
Comment 3 Christian Damus CLA 2008-12-22 08:58:12 EST
(In reply to comment #2)
> 
> One static class (the internal) can be considered as the stdlib builder, and
> the other (public one) can be considered as the OCLStandardLibrary<C>
> implementation which only only provides implementation for that interface (and
> maybe few static methods).

Yes, that makes sense.  Not so much harm in a wrapper, I suppose.


> Can we wait until middle on january for this? API freeze is M6, isn't it?.

That should be no problem.  Although, if you post a patch sooner (with the understanding that it is not ready) then I will be more able to provide feed-back in my -- ahem -- copious spare time.  :-)
Comment 4 Adolfo Sanchez-Barbudo Herrera CLA 2008-12-22 10:57:40 EST
> Yes, that makes sense.  Not so much harm in a wrapper, I suppose.

It can be as harmful as we want :D... I mean, we could have the current implementation as the OCLStandardLibraryBuilder(internal), which wouldn't extend OCLStandardLibrary anymore, and all their methods would be static.

The new EcoreOCLStandardLibrary would implement OCLStandardLibrary, and it would use that OCLStandardLibraryBuilder to obtain the types which were built by the latter.

The harm in this case, is that all the references to OCLStandardLibrary.INSTANCE would need to be replaced by the EcoreOCLStandardLibrary.INSTANCE, which I would be obviously very pleased to change ;P.

Anyway, just with the proposed wrapper would suffice to me.

Again, your decision .... The harmful or harmless ?

> 
> 
> > Can we wait until middle on january for this? API freeze is M6, isn't it?.
> 
> That should be no problem.  Although, if you post a patch sooner (with the
> understanding that it is not ready) then I will be more able to provide
> feed-back in my -- ahem -- copious spare time.  :-)
> 

Ok, I'll provide the patch this week. If any further change would be needed due to specification reasons, I'll raise a new bugzilla one. Besides, I believe that we will have to do the hardest work when OCL 2.1 and QVT 1.1 come up.
Comment 5 Adolfo Sanchez-Barbudo Herrera CLA 2009-01-08 06:44:42 EST
Created attachment 121935 [details]
Proposed patch

Well, I have finally sent the harmless solution.

I have provided both, Ecore and UML based implementations.

Cheers,
Adolfo.
Comment 6 Christian Damus CLA 2009-01-10 17:22:31 EST
Hi, Adolfo,

I have a few questions:

  - why define a static INSTANCE?  That isn't extensible
  - why is the getPackage() method static?
  - I noticed, when I hit Shift+Ctrl+F, that the text changed.  Have
     you imported the formatter settings from the ocl.releng
     project?  If you do, then the project-level preferences in the
     OCL projects will find it, so that code formatting in the OCL
     context is consistent.  It won't affect any other projects in
     your workspace ... :-)
     
BTW, I had a brief look at the impact of updating the EcoreEnvironment and UMLEnvironment implementations to add an instance variable for the stdlib, for which subclasses could provide their own implementation (based on your new public classes).  There is still a considerable amount of code that depends on the static stdlib elements, which it would be very hard to change.  So, I'm not sure what the practical benefit of this extensibility is?
 
In any case, this patch looks OK to me.  Is it ready, or do you have more to add?
Comment 7 Adolfo Sanchez-Barbudo Herrera CLA 2009-01-12 06:19:37 EST
(In reply to comment #6)
> Hi, Adolfo,
> 
> I have a few questions:
> 
>   - why define a static INSTANCE?  That isn't extensible

That instance is the only way that an OCL client may use the standard library.
- The OCLStandardLibraryImpl still resides in an internal package.
- The public Ecore/UMLStandardLibrary has a protected constructor, so that just an unique instance (of an extebsible class) exists.

Anyway, for my needs, I don't need that instance becuase I have my own class. So feel free to remove it.

>   - why is the getPackage() method static?

Again that's for allowing a client to get the unique package which the stdlib is built on without having to use the internal implementation. I have worked on the extension several times, and now I'm not requiring the use of the package itself, so that, you can remove it.

>   - I noticed, when I hit Shift+Ctrl+F, that the text changed.  Have
>      you imported the formatter settings from the ocl.releng
>      project?  If you do, then the project-level preferences in the
>      OCL projects will find it, so that code formatting in the OCL
>      context is consistent.  It won't affect any other projects in
>      your workspace ... :-)
> 

Mmm interesting point. Honestly, I'm not familiarized with this kind of preferences. Does this come from any Eclipse development best practices document? If so, could you give me its reference?. I have a little bit idea what the releng project is intended to, however I haven't worked with it and I'd be pleased to learn about it :)

> BTW, I had a brief look at the impact of updating the EcoreEnvironment and
> UMLEnvironment implementations to add an instance variable for the stdlib, for
> which subclasses could provide their own implementation (based on your new
> public classes).  There is still a considerable amount of code that depends on
> the static stdlib elements, which it would be very hard to change.  So, I'm not
> sure what the practical benefit of this extensibility is?

Yep, this has been a big problem for me while working with the stdlib. I fortunately managed to solve all related problems working on the TypeResolver (for example adding new operations to the OCL predefined types) and the new TypeChecker, and this is the simplest patch for OCL Standard Library which I need to solve the QVT library one.

The real benefit is that I have a QVT StandardLibrary which replaces the OCL Standard Library and I don't have to manage the creation/load of all the OCL Standard Library types and operations, which are already managed by internal OCLStandardLibrary implementation.

Maybe for MDT-OCL 2.0 we could consider using the standard library in a non-statical way in all the code, so that an extending OCL standard library fits better in the implementation. I could help in this task ;)

> In any case, this patch looks OK to me.  Is it ready, or do you have more to
> add?
> 

Yes, this patch would suffice. What about asking to Borland's team ?

Cheers,
Adolfo.
Comment 8 Christian Damus CLA 2009-01-12 08:44:11 EST
(In reply to comment #7)
> > 
> >   - why define a static INSTANCE?  That isn't extensible
> 
> That instance is the only way that an OCL client may use the standard library.

Ah, but what is wrong with, e.g., "new EcoreOCLStandardLibrary()" ?  ;-)


> - The OCLStandardLibraryImpl still resides in an internal package.
> - The public Ecore/UMLStandardLibrary has a protected constructor, so that just
> an unique instance (of an extebsible class) exists.

Right, but the constructor could be public.

I'm just concerned about the usefulness of a singleton for a class that is meant to be extended.  Also, the "official" way to access the stdlib is supposed to be through an Environment instance, so that clients needn't know whether it is a singleton or not.


> Anyway, for my needs, I don't need that instance becuase I have my own class.
> So feel free to remove it.
> 
> >   - why is the getPackage() method static?
> 
> Again that's for allowing a client to get the unique package which the stdlib
> is built on without having to use the internal implementation. I have worked on
> the extension several times, and now I'm not requiring the use of the package
> itself, so that, you can remove it.

Well, I won't remove it because the OCL parser, itself, sometimes looks at the stdlib package.  I'll just change it to non-static to align with the accessors for this package's contained classifiers.


> >   - I noticed, when I hit Shift+Ctrl+F, that the text changed.  Have
> >      you imported the formatter settings from the ocl.releng
> >      project?  If you do, then the project-level preferences in the
> >      OCL projects will find it, so that code formatting in the OCL
> >      context is consistent.  It won't affect any other projects in
> >      your workspace ... :-)
> > 
> 
> Mmm interesting point. Honestly, I'm not familiarized with this kind of
> preferences. Does this come from any Eclipse development best practices
> document? If so, could you give me its reference?. I have a little bit idea
> what the releng project is intended to, however I haven't worked with it and
> I'd be pleased to learn about it :)

Well, I don't know whether it is a "best practice" but Eclipse makes these settings shareable in source control specifically so that teams can standardize on them.

Because until recently I have been the OCL team, I standardized on the settings used by the UML2 component because they look good to me.  :-)

Just use the Import button in the Java formatter preference page to import the OCLCodeFormatting.xml file in the org.eclipse.ocl.releng project.  You don't need to set this profile as your (workspace-wide{ default.  It just needs to be available for the project-level settings in the OCL projects to find it.  It won't interfere with your other projects.


> Maybe for MDT-OCL 2.0 we could consider using the standard library in a
> non-statical way in all the code, so that an extending OCL standard library
> fits better in the implementation. I could help in this task ;)

Oo!  certainly, that would be welcome.

I had been hoping to implement the stdlib as a regular EMF-generated package, encapsulating the implementations of all the various operations within the generated code.  The main problems that I see are:

  - Ecore EDataTypes do not support generalization
  - even for the UML stdlib, we would have to implement
     the companion classes for the data types because we
     convert to an Ecore representation in which data types
     can't own operations.  UML2 maps DataType to EClass
     for this reason, so maybe we should just implement
     the entire stdlib as EClasses in both cases?

I welcome ideas for solutions.  :-)


> > In any case, this patch looks OK to me.  Is it ready, or do you have more to
> > add?
> > 
> 
> Yes, this patch would suffice. What about asking to Borland's team ?

I shall copy Radek for their input.

Thanks!
Comment 9 Radomil Dvorak CLA 2009-01-12 09:49:02 EST
(In reply to comment #8)

Hi, thanks for CC me!

We have not had a need for direct extensibility in terms of java interface of the OCL stdlib. I'm not aware it prevents an MDT OCL based QVT implementation to define the QVT Stdlib. So, I wonder if there is anything I have missed so far.

As Christian says, the access point via environment which we extend does not give any promise on the OCL stdlib being singleton or not.
So I assumed, the QVT Stdlib can rather aggregate the OCL stdlib while we can choose whether to use the singleton way.

I just hope that Adolfo will use OMG mailing list to discuss the QVT Stdlib issues with Mariano ;)
Comment 10 Adolfo Sanchez-Barbudo Herrera CLA 2009-01-13 10:29:56 EST
(In reply to comment #8)
> I'm just concerned about the usefulness of a singleton for a class that is
> meant to be extended.  Also, the "official" way to access the stdlib is
> supposed to be through an Environment instance, so that clients needn't know
> whether it is a singleton or not.

Ok. I'm finally convinced :).

> Oo!  certainly, that would be welcome.

> I had been hoping to implement the stdlib as a regular EMF-generated package,
> encapsulating the implementations of all the various operations within the
> generated code.  The main problems that I see are:
> 
>   - Ecore EDataTypes do not support generalization
>   - even for the UML stdlib, we would have to implement
>      the companion classes for the data types because we
>      convert to an Ecore representation in which data types
>      can't own operations.  UML2 maps DataType to EClass
>      for this reason, so maybe we should just implement
>      the entire stdlib as EClasses in both cases?
> 
> I welcome ideas for solutions.  :-)

I suppose that you mean modeling the stdlib as an .ecore to take adavantage from the EMF generation, dont't you?. The problem is how to simulate the "metalevels" since the (unique) model instance of that possible metamodel must conform to the OCL metamodel. So I could imagine the following for the String M1 predefined primitive type.
- Creating an EClass (which could be called Integer) which extends PrimitiveType.
- Include the required operations on it,
- Generate the classes.
- Configure the values for the generated classes (name=Integer) and ensure that there is a unique instance of them (treat that classes like singletons)

From my point of view, it's a little bit odd doing these things. IMO, from the side of Ecore Metamodel binding, the oclstdlib must be a pure OCLEcore.ecore instance: an oclstdlib.oclecore file, so the corresponding classes in Java are created by the EcoreFactory (not by a new StdlibFactory which creates specialized classes).

Besides, considering EMOF as the metamode, in the OCL specification relates the need of creation of new Classes (at the same M1 level predefined type, with the SAME name of the related predefined DataType) in the OCL Standard Library, in order to specifically solve the fact the EMOF datatypes can't own operations.

It seems that it could be a good solution from the implementation side (having a specific java package for the library, generated classes, etc). However, I'm not sure it's the best solution from the specification side. Anyway, as we already know, the specification doesn't alwasy have the last word, since it may be incorrect.

I rememeber that Radek had its own idea about how to treat standard (and user-defined) libraries. Maybe he could share it's ideas about this :)

(In reply to comment #9)
> 
> We have not had a need for direct extensibility in terms of java interface of
> the OCL stdlib. I'm not aware it prevents an MDT OCL based QVT implementation
> to define the QVT Stdlib. So, I wonder if there is anything I have missed so
> far.

So, I guess you still keep the OCLStandardLibraryImpl in your environments ?

I would like to have the OCL stdlib like an artifact which can be plugged-out from the MDT-OCL implementation (In this case, from the environment) and plug-in my own library implementation. Just using the same patterns applied to artifacts like environments, type resolvers, the type checker, etc would be interesting for language extenders.

The problem right now, is that MDT-OCL implementation was not initially thought to be an extensible language (at least not in terms of its Standard Library). So the MDT-OCL code is highly coupled with the standard library implementation (which could be good for OCL, but not for reusing it for extension), and that's the reason why using a new standardLibrary in the environment, is not completely necessary. Anyway, I hope that the implementation would be more flexible in that way.

Besides, Have you imagined that any predefined type from OCL (a collection or primitive type), would have to be replaced by new instance of its metatypes in the QVTo context. I hope that would never be required by QVTo spec. However, at least the implementation would be sufficient flexible to adopt this kind of changes.

I strongly believe using (extending, integrating, etc) the OCLStandardLibrary (or whatever extension) inside the implementation is not a bad decision. And the first step is having the possibility of extending the default OCL stdlib implementation :)

> 
> As Christian says, the access point via environment which we extend does not
> give any promise on the OCL stdlib being singleton or not.
> So I assumed, the QVT Stdlib can rather aggregate the OCL stdlib while we can
> choose whether to use the singleton way.
> 
> I just hope that Adolfo will use OMG mailing list to discuss the QVT Stdlib
> issues with Mariano ;)
> 

Of course, I hope that the efforts we have been investing here, have an immediate result in the next realeses of both OCL/QVT spec :D. I have no doubts about we both have an acceptable implementations of the spec (mine for my phd, and your for the official Eclipse project ;) ).

BTW there are some issues related to QVTo Stdlib in the OMG mailing list, which haven't had too much (actually, any) feedback. Maybe you could send some comments there, so that the discussion were activated there.

Cheers,
Adolfo.
Comment 11 Christian Damus CLA 2009-01-14 22:59:43 EST
(In reply to comment #10)
> I suppose that you mean modeling the stdlib as an .ecore to take adavantage
> from the EMF generation, dont't you?. The problem is how to simulate the
> "metalevels" since the (unique) model instance of that possible metamodel must
> conform to the OCL metamodel. So I could imagine the following for the String

Yes, I suppose the question is what will the code generator think of instances of its metaclasses as extended by OCL?  Good question.  Certainly, it will not recognize the metaclasses that directly extend EClassifier ...


> M1 predefined primitive type.
> - Creating an EClass (which could be called Integer) which extends
> PrimitiveType.
> - Include the required operations on it,

This is just like the "shadow classes" of the ungainly EMOF adaptation strategy defined in Chapter 13 of the spec.


> Besides, considering EMOF as the metamode, in the OCL specification relates the
> need of creation of new Classes (at the same M1 level predefined type, with the
> SAME name of the related predefined DataType) in the OCL Standard Library, in
> order to specifically solve the fact the EMOF datatypes can't own operations.

Oo, I had the same thought.  Don't you just hate these extra classes?  Why did EMOF make the mistake of not allowing operations on data types?  Of what use are data types without operations, anyway?  Argh.


> It seems that it could be a good solution from the implementation side (having
> a specific java package for the library, generated classes, etc). However, I'm
> not sure it's the best solution from the specification side. Anyway, as we
> already know, the specification doesn't alwasy have the last word, since it may
> be incorrect.

That's right.  And, clearly, my ideas on the subject so far are quite early and not much developed.  You raise some very good points.  This is a good subject for discussion on the mailing list.  Perhaps you would like to formalize some design proposal that we can use as a starting point for the discussion?  Perhaps in collaboration with Radek, who you suggest has thoughts on the subject already?


On a more practical question, am I waiting for an updated patch on this bug?  It seems that you and Radek have very different requirements in this area ...

Comment 12 Adolfo Sanchez-Barbudo Herrera CLA 2009-01-15 11:35:35 EST
> That's right.  And, clearly, my ideas on the subject so far are quite early and
> not much developed.  You raise some very good points.  This is a good subject
> for discussion on the mailing list.  Perhaps you would like to formalize some
> design proposal that we can use as a starting point for the discussion? 
> Perhaps in collaboration with Radek, who you suggest has thoughts on the
> subject already?

I have found the link about our discussion (nearly one year ago >.<)

http://dev.eclipse.org/newslists/news.eclipse.modeling.m2m/msg02380.html

I don't know if Radek has changed his mind about all those ideas ;P. I'll definitely open a discussion in the mdt.ocl-dev mail list. 

> 
> 
> On a more practical question, am I waiting for an updated patch on this bug? 

For me, the proposed patch with the changes you mentioned is good for me.

> It seems that you and Radek have very different requirements in this area ...
> 

Definitely, since it seems we have different design approaches... 

If it is not an inconvenience, I would like to have this enhancement, since along my code, I uniformily access the QVToStandardLibrary, which provides both QVT and OCL types, by Environment.getOclStandardLibrary contract.

Cheers,
Adolfo.
Comment 13 Adolfo Sanchez-Barbudo Herrera CLA 2009-01-15 13:38:15 EST
Created attachment 122705 [details]
(Final?) Proposed patch

New patch which includes the changes commented above.
Comment 14 Christian Damus CLA 2009-01-19 09:13:46 EST
Committed the patch to HEAD (1.3 branch).  Thanks, Adolfo!
Comment 15 Ed Willink CLA 2011-05-27 02:48:32 EDT
Closing after over 18 months in resolved state.