You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Alex Huang <Al...@citrix.com> on 2013/08/22 19:58:46 UTC

[DESIGN] Access entities directly...

Hi Everyone,

I've been doing a lot of refactoring and I noticed the following type of code in our code base.

Interface VpcService {
  Vpc getVpc(long id);
  PrivateGateway getPrviateGateway(long id);
}

Interface VpcManager extends VpcService {
...
}

Class VpcManager implements VpcManager {
  Public Vpc getVpc(long id) {
       Return _vpcDao.findById(id);
  }
  Public PrivateGateway getPrivateGateway(long id) {
       Return _privateGateway.findById(id);
  }
}

CloudStack was specifically written so people don't have to do this.  It's just useless lines that makes following code harder.

I know most schools teach these abstraction concepts and there are valid uses but they don't apply in cloudstack.  There's certainly a school of thought that you need to guard your entities from outside developers.  In cloudstack, that fence is at the process boundary of cloudstack or, iow, that fence is cloudstack's over the wire api.  Once code got past that layer, code is collaborating and there's no need for that fence.  Now, this doesn't mean we are advocating all code can modify all entities at will.  Manager is here to manage the lifecycle and changes to the entities they manage.  However, it is not there to provide access.  Consumers of your code should know by convention to use the entity interface instead of the VO objects and leave modifications to that manager.  So here's some general things to think about.

If you are writing code for CloudStack's core orchestration work:
- Write your managers to manage entities, not provide access
- Entity interfaces are for consummation so it shouldn't have set methods on them.
- Entity interfaces should be in cloud-api which defines CloudStack's sdk.
- CloudStack's core  VO and DAO are in cloud-engine-schema.   This forms the schema for CloudStack.  Note that this is for the core objects CloudStack manages and exposes.  If you're writing a plugin and the plugin has its own DB, there's no need to put that into cloud-engine-schema.

If you are writing code for plugins:
- If you need to modify certain entities in cloudstack, you can add dependency to cloud-engine-schema and have access to the vo and daos.  Make sure you really need to do this though.
- Never assume an interface can be cast down to the VO.
- If you are consuming an entity, use the interface not the VO.  You can use EntityManager to do this.  For example, any code can do the following after declaring dependency on the cloud-api package.

@Inject
EntityManager _entityMgr;

Vpc vpc = _entityMgr.findById(Vpc.class, vpcId);
PrivateGateway pg = _entityMgr.findById(PrivateGateway.class, gatewayId);

--Alex

Re: [DESIGN] Access entities directly...

Posted by Amit Das <am...@cloudbyte.com>.
Thanks Alex for clearing out my confusion. I intend to modify certain
fields only.

One more query regarding the injection of cloudstack Dao vs cloudstack Mgr
classes in the plugin code base.
Should we not use Mgr classes for all the CRUD operations instead of using
the Dao classes ?
By using Mgr for CRUD ops, we delegate the transaction & other management
concerns to the service layer.

Regards,
Amit
*CloudByte Inc.* <http://www.cloudbyte.com/>


On Wed, Sep 11, 2013 at 9:52 PM, Alex Huang <Al...@citrix.com> wrote:

> Hi Amit,
>
> I would say make sure you leave the life cycle management to cloudstack's
> storage orchestration.  Otherwise, if you're just updating certain fields,
> you should be able to directly retrieve the VO and modify and persist it.
>  You shouldn't have to go through the o-t-w api, which is what you listed
> here.
>
> Maybe you can give us an idea what you are planning to modify.
>
> --Alex
>
> > -----Original Message-----
> > From: Amit Das [mailto:amit.das@cloudbyte.com]
> > Sent: Wednesday, September 11, 2013 5:53 AM
> > To: dev@cloudstack.apache.org
> > Subject: Re: [DESIGN] Access entities directly...
> >
> > Hi All,
> >
> > I am in process of implementing a storage plugin and would require more
> > clarity on this front.
> >
> > I need to update the 'Volume' as well as the 'StoragePool' in a new
> > implementing class of PrimaryDataStoreDriver (createAsync method to be
> > exact).
> >
> > Should following method invocations be used to update above entities.
> > - VolumeManager's updateVolume(UpdateVolumeCmd updateVolumeCmd)
> > - StorageManager's updateStoragePool(UpdateStoragePoolCmd cmd)
> >
> > In other words, do I need to construct UpdateVolumeCmd &
> > UpdateStoragePoolCmd objects in the storage plugin code ?
> >
> > Regards,
> > Amit
> > *CloudByte Inc.* <http://www.cloudbyte.com/>
> >
> >
> > On Fri, Aug 23, 2013 at 2:10 AM, Darren Shepherd <
> > darren.s.shepherd@gmail.com> wrote:
> >
> > > Sorry clicked something that send that last email...
> > >
> > > To finish
> > >
> > > if ( obj instance VMInstanceVO )
> > >   return (VMInstanceVO)obj
> > > else
> > >   return findByIdIncludingRemoved(obj.getId())
> > >
> > >
> > > Just a thought
> > >
> > > Darren
> > >
> > >
> > >
> > > On Thu, Aug 22, 2013 at 1:38 PM, Darren Shepherd <
> > > darren.s.shepherd@gmail.com> wrote:
> > >
> > > > These are great points Alex is making here and I was just noticing
> > > > the same thing.
> > > >
> > > > One if the things I've noticed is that most managers inject a lot of
> > > > DAOs.  When you look at the code usually about half those DAOs are
> > > > needed only for findById.  So if you are consuming an object use the
> > > > interface and use entity manager.  Only if you need a specialized
> > > > findBy[SomeSpecialCriteria] should you actually inject the VO's DAO.
> > > >
> > > > The problem with injecting a lot of DAO is the injected members
> > > > given you a quick look at what are the logical dependencies of the
> > > > class/manager.  So if you see 15 DAOs you then have to assume that
> > > > that class may modify up to 15 types of entities.
> > > >
> > > > In other projects that have a similar DAO usage style as CloudStack,
> > > > it's been helpful to actually define the DAOs as returning
> > > > jnterfaces only.  We may consider such a pattern, or some variation
> > > > of it.  The reason I say this, is that if you need to get a VM by
> > > > instance name your going to call the vmDao.findVMByInstanceName()
> > > > which returns a VO.  So two problems.  1) The temptation is to use a
> > > > VO in the code and not the interface 2) The class, which is just a
> > > > consumer now has access to a DAO that can write modify.
> > > >
> > > > So one possible pattern could be to seperate the interfaces into
> > > > VMInstanceAccessDAO and VMInstanceDAO where the access interface
> > has
> > > > all find and list methods that return "? extend VMInstance" and the
> > > > other interface has the modify methods.  To make this fully work,
> > > > you would probably have to add a getVO() method to the GenericDao
> > > > that will translate an inteface to a VO.  So basically
> > > >
> > > > if ( obj instance VMInstanceVO )
> > > >
> > > >
> > > > Darren
> > > >
> > > > On Aug 22, 2013, at 10:58 AM, Alex Huang <Al...@citrix.com>
> > wrote:
> > > >
> > > > > Hi Everyone,
> > > > >
> > > > > I've been doing a lot of refactoring and I noticed the following
> > > > > type
> > > of
> > > > code in our code base.
> > > > >
> > > > > Interface VpcService {
> > > > >  Vpc getVpc(long id);
> > > > >  PrivateGateway getPrviateGateway(long id); }
> > > > >
> > > > > Interface VpcManager extends VpcService { ...
> > > > > }
> > > > >
> > > > > Class VpcManager implements VpcManager {  Public Vpc getVpc(long
> > > > > id) {
> > > > >       Return _vpcDao.findById(id);  }  Public PrivateGateway
> > > > > getPrivateGateway(long id) {
> > > > >       Return _privateGateway.findById(id);  } }
> > > > >
> > > > > CloudStack was specifically written so people don't have to do
> this.
> > > >  It's just useless lines that makes following code harder.
> > > > >
> > > > > I know most schools teach these abstraction concepts and there are
> > > valid
> > > > uses but they don't apply in cloudstack.  There's certainly a school
> > > > of thought that you need to guard your entities from outside
> > > > developers.  In cloudstack, that fence is at the process boundary of
> > > > cloudstack or, iow, that fence is cloudstack's over the wire api.
> > > > Once code got past that layer, code is collaborating and there's no
> > > > need for that fence.  Now,
> > > this
> > > > doesn't mean we are advocating all code can modify all entities at
> will.
> > > >  Manager is here to manage the lifecycle and changes to the entities
> > > > they manage.  However, it is not there to provide access.  Consumers
> > > > of your code should know by convention to use the entity interface
> > > > instead of the VO objects and leave modifications to that manager.
> > > > So here's some
> > > general
> > > > things to think about.
> > > > >
> > > > > If you are writing code for CloudStack's core orchestration work:
> > > > > - Write your managers to manage entities, not provide access
> > > > > - Entity interfaces are for consummation so it shouldn't have set
> > > > methods on them.
> > > > > - Entity interfaces should be in cloud-api which defines
> > > > > CloudStack's
> > > > sdk.
> > > > > - CloudStack's core  VO and DAO are in cloud-engine-schema.   This
> > > forms
> > > > the schema for CloudStack.  Note that this is for the core objects
> > > > CloudStack manages and exposes.  If you're writing a plugin and the
> > > plugin
> > > > has its own DB, there's no need to put that into cloud-engine-schema.
> > > > >
> > > > > If you are writing code for plugins:
> > > > > - If you need to modify certain entities in cloudstack, you can
> > > > > add
> > > > dependency to cloud-engine-schema and have access to the vo and daos.
> > >  Make
> > > > sure you really need to do this though.
> > > > > - Never assume an interface can be cast down to the VO.
> > > > > - If you are consuming an entity, use the interface not the VO.
> > > > > You
> > > can
> > > > use EntityManager to do this.  For example, any code can do the
> > > > following after declaring dependency on the cloud-api package.
> > > > >
> > > > > @Inject
> > > > > EntityManager _entityMgr;
> > > > >
> > > > > Vpc vpc = _entityMgr.findById(Vpc.class, vpcId); PrivateGateway pg
> > > > > = _entityMgr.findById(PrivateGateway.class,
> > > gatewayId);
> > > > >
> > > > > --Alex
> > > >
> > >
>

RE: [DESIGN] Access entities directly...

Posted by Alex Huang <Al...@citrix.com>.
Hi Amit,

I would say make sure you leave the life cycle management to cloudstack's storage orchestration.  Otherwise, if you're just updating certain fields, you should be able to directly retrieve the VO and modify and persist it.  You shouldn't have to go through the o-t-w api, which is what you listed here.

Maybe you can give us an idea what you are planning to modify.  

--Alex

> -----Original Message-----
> From: Amit Das [mailto:amit.das@cloudbyte.com]
> Sent: Wednesday, September 11, 2013 5:53 AM
> To: dev@cloudstack.apache.org
> Subject: Re: [DESIGN] Access entities directly...
> 
> Hi All,
> 
> I am in process of implementing a storage plugin and would require more
> clarity on this front.
> 
> I need to update the 'Volume' as well as the 'StoragePool' in a new
> implementing class of PrimaryDataStoreDriver (createAsync method to be
> exact).
> 
> Should following method invocations be used to update above entities.
> - VolumeManager's updateVolume(UpdateVolumeCmd updateVolumeCmd)
> - StorageManager's updateStoragePool(UpdateStoragePoolCmd cmd)
> 
> In other words, do I need to construct UpdateVolumeCmd &
> UpdateStoragePoolCmd objects in the storage plugin code ?
> 
> Regards,
> Amit
> *CloudByte Inc.* <http://www.cloudbyte.com/>
> 
> 
> On Fri, Aug 23, 2013 at 2:10 AM, Darren Shepherd <
> darren.s.shepherd@gmail.com> wrote:
> 
> > Sorry clicked something that send that last email...
> >
> > To finish
> >
> > if ( obj instance VMInstanceVO )
> >   return (VMInstanceVO)obj
> > else
> >   return findByIdIncludingRemoved(obj.getId())
> >
> >
> > Just a thought
> >
> > Darren
> >
> >
> >
> > On Thu, Aug 22, 2013 at 1:38 PM, Darren Shepherd <
> > darren.s.shepherd@gmail.com> wrote:
> >
> > > These are great points Alex is making here and I was just noticing
> > > the same thing.
> > >
> > > One if the things I've noticed is that most managers inject a lot of
> > > DAOs.  When you look at the code usually about half those DAOs are
> > > needed only for findById.  So if you are consuming an object use the
> > > interface and use entity manager.  Only if you need a specialized
> > > findBy[SomeSpecialCriteria] should you actually inject the VO's DAO.
> > >
> > > The problem with injecting a lot of DAO is the injected members
> > > given you a quick look at what are the logical dependencies of the
> > > class/manager.  So if you see 15 DAOs you then have to assume that
> > > that class may modify up to 15 types of entities.
> > >
> > > In other projects that have a similar DAO usage style as CloudStack,
> > > it's been helpful to actually define the DAOs as returning
> > > jnterfaces only.  We may consider such a pattern, or some variation
> > > of it.  The reason I say this, is that if you need to get a VM by
> > > instance name your going to call the vmDao.findVMByInstanceName()
> > > which returns a VO.  So two problems.  1) The temptation is to use a
> > > VO in the code and not the interface 2) The class, which is just a
> > > consumer now has access to a DAO that can write modify.
> > >
> > > So one possible pattern could be to seperate the interfaces into
> > > VMInstanceAccessDAO and VMInstanceDAO where the access interface
> has
> > > all find and list methods that return "? extend VMInstance" and the
> > > other interface has the modify methods.  To make this fully work,
> > > you would probably have to add a getVO() method to the GenericDao
> > > that will translate an inteface to a VO.  So basically
> > >
> > > if ( obj instance VMInstanceVO )
> > >
> > >
> > > Darren
> > >
> > > On Aug 22, 2013, at 10:58 AM, Alex Huang <Al...@citrix.com>
> wrote:
> > >
> > > > Hi Everyone,
> > > >
> > > > I've been doing a lot of refactoring and I noticed the following
> > > > type
> > of
> > > code in our code base.
> > > >
> > > > Interface VpcService {
> > > >  Vpc getVpc(long id);
> > > >  PrivateGateway getPrviateGateway(long id); }
> > > >
> > > > Interface VpcManager extends VpcService { ...
> > > > }
> > > >
> > > > Class VpcManager implements VpcManager {  Public Vpc getVpc(long
> > > > id) {
> > > >       Return _vpcDao.findById(id);  }  Public PrivateGateway
> > > > getPrivateGateway(long id) {
> > > >       Return _privateGateway.findById(id);  } }
> > > >
> > > > CloudStack was specifically written so people don't have to do this.
> > >  It's just useless lines that makes following code harder.
> > > >
> > > > I know most schools teach these abstraction concepts and there are
> > valid
> > > uses but they don't apply in cloudstack.  There's certainly a school
> > > of thought that you need to guard your entities from outside
> > > developers.  In cloudstack, that fence is at the process boundary of
> > > cloudstack or, iow, that fence is cloudstack's over the wire api.
> > > Once code got past that layer, code is collaborating and there's no
> > > need for that fence.  Now,
> > this
> > > doesn't mean we are advocating all code can modify all entities at will.
> > >  Manager is here to manage the lifecycle and changes to the entities
> > > they manage.  However, it is not there to provide access.  Consumers
> > > of your code should know by convention to use the entity interface
> > > instead of the VO objects and leave modifications to that manager.
> > > So here's some
> > general
> > > things to think about.
> > > >
> > > > If you are writing code for CloudStack's core orchestration work:
> > > > - Write your managers to manage entities, not provide access
> > > > - Entity interfaces are for consummation so it shouldn't have set
> > > methods on them.
> > > > - Entity interfaces should be in cloud-api which defines
> > > > CloudStack's
> > > sdk.
> > > > - CloudStack's core  VO and DAO are in cloud-engine-schema.   This
> > forms
> > > the schema for CloudStack.  Note that this is for the core objects
> > > CloudStack manages and exposes.  If you're writing a plugin and the
> > plugin
> > > has its own DB, there's no need to put that into cloud-engine-schema.
> > > >
> > > > If you are writing code for plugins:
> > > > - If you need to modify certain entities in cloudstack, you can
> > > > add
> > > dependency to cloud-engine-schema and have access to the vo and daos.
> >  Make
> > > sure you really need to do this though.
> > > > - Never assume an interface can be cast down to the VO.
> > > > - If you are consuming an entity, use the interface not the VO.
> > > > You
> > can
> > > use EntityManager to do this.  For example, any code can do the
> > > following after declaring dependency on the cloud-api package.
> > > >
> > > > @Inject
> > > > EntityManager _entityMgr;
> > > >
> > > > Vpc vpc = _entityMgr.findById(Vpc.class, vpcId); PrivateGateway pg
> > > > = _entityMgr.findById(PrivateGateway.class,
> > gatewayId);
> > > >
> > > > --Alex
> > >
> >

Re: [DESIGN] Access entities directly...

Posted by Amit Das <am...@cloudbyte.com>.
Thanks Mike.

I referred to SolidFire's plugin implementation & found some differences
with the recommendations in this email thread.

Alex's email talks about using entity interface e.g. Volume than a VO class
e.g. VolumeVO.
However, both of our plugins require a mutable object (w.r.t volume,
storagepool, etc.) & hence the confusion regarding the best practices.

In addition, I also want to know if it is appropriate to inject DaoImpl
classes in the plugin codebase for CRUD operations ?
Will it not be better to inject Mgr classes instead for these CRUD
operations ?


Regards,
Amit
*CloudByte Inc.* <http://www.cloudbyte.com/>


On Wed, Sep 11, 2013 at 8:43 PM, Mike Tutkowski <
mike.tutkowski@solidfire.com> wrote:

> Hey Amit,
>
> We might have already chatted a bit about this, but feel free to examine
> the SolidFire plug-in as it likely handles what you're referring to here.
>
> Talk to you later
>
>
> On Wed, Sep 11, 2013 at 6:52 AM, Amit Das <am...@cloudbyte.com> wrote:
>
> > Hi All,
> >
> > I am in process of implementing a storage plugin and would require more
> > clarity on this front.
> >
> > I need to update the 'Volume' as well as the 'StoragePool' in a new
> > implementing class of PrimaryDataStoreDriver (createAsync method to be
> > exact).
> >
> > Should following method invocations be used to update above entities.
> > - VolumeManager's updateVolume(UpdateVolumeCmd updateVolumeCmd)
> > - StorageManager's updateStoragePool(UpdateStoragePoolCmd cmd)
> >
> > In other words, do I need to construct UpdateVolumeCmd &
> > UpdateStoragePoolCmd objects in the storage plugin code ?
> >
> > Regards,
> > Amit
> > *CloudByte Inc.* <http://www.cloudbyte.com/>
> >
> >
> > On Fri, Aug 23, 2013 at 2:10 AM, Darren Shepherd <
> > darren.s.shepherd@gmail.com> wrote:
> >
> > > Sorry clicked something that send that last email...
> > >
> > > To finish
> > >
> > > if ( obj instance VMInstanceVO )
> > >   return (VMInstanceVO)obj
> > > else
> > >   return findByIdIncludingRemoved(obj.getId())
> > >
> > >
> > > Just a thought
> > >
> > > Darren
> > >
> > >
> > >
> > > On Thu, Aug 22, 2013 at 1:38 PM, Darren Shepherd <
> > > darren.s.shepherd@gmail.com> wrote:
> > >
> > > > These are great points Alex is making here and I was just noticing
> the
> > > > same thing.
> > > >
> > > > One if the things I've noticed is that most managers inject a lot of
> > > > DAOs.  When you look at the code usually about half those DAOs are
> > > > needed only for findById.  So if you are consuming an object use the
> > > > interface and use entity manager.  Only if you need a specialized
> > > > findBy[SomeSpecialCriteria] should you actually inject the VO's DAO.
> > > >
> > > > The problem with injecting a lot of DAO is the injected members given
> > > > you a quick look at what are the logical dependencies of the
> > > > class/manager.  So if you see 15 DAOs you then have to assume that
> > > > that class may modify up to 15 types of entities.
> > > >
> > > > In other projects that have a similar DAO usage style as CloudStack,
> > > > it's been helpful to actually define the DAOs as returning jnterfaces
> > > > only.  We may consider such a pattern, or some variation of it.  The
> > > > reason I say this, is that if you need to get a VM by instance name
> > > > your going to call the vmDao.findVMByInstanceName() which returns a
> > > > VO.  So two problems.  1) The temptation is to use a VO in the code
> > > > and not the interface 2) The class, which is just a consumer now has
> > > > access to a DAO that can write modify.
> > > >
> > > > So one possible pattern could be to seperate the interfaces into
> > > > VMInstanceAccessDAO and VMInstanceDAO where the access interface has
> > > > all find and list methods that return "? extend VMInstance" and the
> > > > other interface has the modify methods.  To make this fully work, you
> > > > would probably have to add a getVO() method to the GenericDao that
> > > > will translate an inteface to a VO.  So basically
> > > >
> > > > if ( obj instance VMInstanceVO )
> > > >
> > > >
> > > > Darren
> > > >
> > > > On Aug 22, 2013, at 10:58 AM, Alex Huang <Al...@citrix.com>
> > wrote:
> > > >
> > > > > Hi Everyone,
> > > > >
> > > > > I've been doing a lot of refactoring and I noticed the following
> type
> > > of
> > > > code in our code base.
> > > > >
> > > > > Interface VpcService {
> > > > >  Vpc getVpc(long id);
> > > > >  PrivateGateway getPrviateGateway(long id);
> > > > > }
> > > > >
> > > > > Interface VpcManager extends VpcService {
> > > > > ...
> > > > > }
> > > > >
> > > > > Class VpcManager implements VpcManager {
> > > > >  Public Vpc getVpc(long id) {
> > > > >       Return _vpcDao.findById(id);
> > > > >  }
> > > > >  Public PrivateGateway getPrivateGateway(long id) {
> > > > >       Return _privateGateway.findById(id);
> > > > >  }
> > > > > }
> > > > >
> > > > > CloudStack was specifically written so people don't have to do
> this.
> > > >  It's just useless lines that makes following code harder.
> > > > >
> > > > > I know most schools teach these abstraction concepts and there are
> > > valid
> > > > uses but they don't apply in cloudstack.  There's certainly a school
> of
> > > > thought that you need to guard your entities from outside developers.
> >  In
> > > > cloudstack, that fence is at the process boundary of cloudstack or,
> > iow,
> > > > that fence is cloudstack's over the wire api.  Once code got past
> that
> > > > layer, code is collaborating and there's no need for that fence.
>  Now,
> > > this
> > > > doesn't mean we are advocating all code can modify all entities at
> > will.
> > > >  Manager is here to manage the lifecycle and changes to the entities
> > they
> > > > manage.  However, it is not there to provide access.  Consumers of
> your
> > > > code should know by convention to use the entity interface instead of
> > the
> > > > VO objects and leave modifications to that manager.  So here's some
> > > general
> > > > things to think about.
> > > > >
> > > > > If you are writing code for CloudStack's core orchestration work:
> > > > > - Write your managers to manage entities, not provide access
> > > > > - Entity interfaces are for consummation so it shouldn't have set
> > > > methods on them.
> > > > > - Entity interfaces should be in cloud-api which defines
> CloudStack's
> > > > sdk.
> > > > > - CloudStack's core  VO and DAO are in cloud-engine-schema.   This
> > > forms
> > > > the schema for CloudStack.  Note that this is for the core objects
> > > > CloudStack manages and exposes.  If you're writing a plugin and the
> > > plugin
> > > > has its own DB, there's no need to put that into cloud-engine-schema.
> > > > >
> > > > > If you are writing code for plugins:
> > > > > - If you need to modify certain entities in cloudstack, you can add
> > > > dependency to cloud-engine-schema and have access to the vo and daos.
> > >  Make
> > > > sure you really need to do this though.
> > > > > - Never assume an interface can be cast down to the VO.
> > > > > - If you are consuming an entity, use the interface not the VO.
>  You
> > > can
> > > > use EntityManager to do this.  For example, any code can do the
> > following
> > > > after declaring dependency on the cloud-api package.
> > > > >
> > > > > @Inject
> > > > > EntityManager _entityMgr;
> > > > >
> > > > > Vpc vpc = _entityMgr.findById(Vpc.class, vpcId);
> > > > > PrivateGateway pg = _entityMgr.findById(PrivateGateway.class,
> > > gatewayId);
> > > > >
> > > > > --Alex
> > > >
> > >
> >
>
>
>
> --
> *Mike Tutkowski*
> *Senior CloudStack Developer, SolidFire Inc.*
> e: mike.tutkowski@solidfire.com
> o: 303.746.7302
> Advancing the way the world uses the
> cloud<http://solidfire.com/solution/overview/?video=play>
> *™*
>

Re: [DESIGN] Access entities directly...

Posted by Mike Tutkowski <mi...@solidfire.com>.
Hey Amit,

We might have already chatted a bit about this, but feel free to examine
the SolidFire plug-in as it likely handles what you're referring to here.

Talk to you later


On Wed, Sep 11, 2013 at 6:52 AM, Amit Das <am...@cloudbyte.com> wrote:

> Hi All,
>
> I am in process of implementing a storage plugin and would require more
> clarity on this front.
>
> I need to update the 'Volume' as well as the 'StoragePool' in a new
> implementing class of PrimaryDataStoreDriver (createAsync method to be
> exact).
>
> Should following method invocations be used to update above entities.
> - VolumeManager's updateVolume(UpdateVolumeCmd updateVolumeCmd)
> - StorageManager's updateStoragePool(UpdateStoragePoolCmd cmd)
>
> In other words, do I need to construct UpdateVolumeCmd &
> UpdateStoragePoolCmd objects in the storage plugin code ?
>
> Regards,
> Amit
> *CloudByte Inc.* <http://www.cloudbyte.com/>
>
>
> On Fri, Aug 23, 2013 at 2:10 AM, Darren Shepherd <
> darren.s.shepherd@gmail.com> wrote:
>
> > Sorry clicked something that send that last email...
> >
> > To finish
> >
> > if ( obj instance VMInstanceVO )
> >   return (VMInstanceVO)obj
> > else
> >   return findByIdIncludingRemoved(obj.getId())
> >
> >
> > Just a thought
> >
> > Darren
> >
> >
> >
> > On Thu, Aug 22, 2013 at 1:38 PM, Darren Shepherd <
> > darren.s.shepherd@gmail.com> wrote:
> >
> > > These are great points Alex is making here and I was just noticing the
> > > same thing.
> > >
> > > One if the things I've noticed is that most managers inject a lot of
> > > DAOs.  When you look at the code usually about half those DAOs are
> > > needed only for findById.  So if you are consuming an object use the
> > > interface and use entity manager.  Only if you need a specialized
> > > findBy[SomeSpecialCriteria] should you actually inject the VO's DAO.
> > >
> > > The problem with injecting a lot of DAO is the injected members given
> > > you a quick look at what are the logical dependencies of the
> > > class/manager.  So if you see 15 DAOs you then have to assume that
> > > that class may modify up to 15 types of entities.
> > >
> > > In other projects that have a similar DAO usage style as CloudStack,
> > > it's been helpful to actually define the DAOs as returning jnterfaces
> > > only.  We may consider such a pattern, or some variation of it.  The
> > > reason I say this, is that if you need to get a VM by instance name
> > > your going to call the vmDao.findVMByInstanceName() which returns a
> > > VO.  So two problems.  1) The temptation is to use a VO in the code
> > > and not the interface 2) The class, which is just a consumer now has
> > > access to a DAO that can write modify.
> > >
> > > So one possible pattern could be to seperate the interfaces into
> > > VMInstanceAccessDAO and VMInstanceDAO where the access interface has
> > > all find and list methods that return "? extend VMInstance" and the
> > > other interface has the modify methods.  To make this fully work, you
> > > would probably have to add a getVO() method to the GenericDao that
> > > will translate an inteface to a VO.  So basically
> > >
> > > if ( obj instance VMInstanceVO )
> > >
> > >
> > > Darren
> > >
> > > On Aug 22, 2013, at 10:58 AM, Alex Huang <Al...@citrix.com>
> wrote:
> > >
> > > > Hi Everyone,
> > > >
> > > > I've been doing a lot of refactoring and I noticed the following type
> > of
> > > code in our code base.
> > > >
> > > > Interface VpcService {
> > > >  Vpc getVpc(long id);
> > > >  PrivateGateway getPrviateGateway(long id);
> > > > }
> > > >
> > > > Interface VpcManager extends VpcService {
> > > > ...
> > > > }
> > > >
> > > > Class VpcManager implements VpcManager {
> > > >  Public Vpc getVpc(long id) {
> > > >       Return _vpcDao.findById(id);
> > > >  }
> > > >  Public PrivateGateway getPrivateGateway(long id) {
> > > >       Return _privateGateway.findById(id);
> > > >  }
> > > > }
> > > >
> > > > CloudStack was specifically written so people don't have to do this.
> > >  It's just useless lines that makes following code harder.
> > > >
> > > > I know most schools teach these abstraction concepts and there are
> > valid
> > > uses but they don't apply in cloudstack.  There's certainly a school of
> > > thought that you need to guard your entities from outside developers.
>  In
> > > cloudstack, that fence is at the process boundary of cloudstack or,
> iow,
> > > that fence is cloudstack's over the wire api.  Once code got past that
> > > layer, code is collaborating and there's no need for that fence.  Now,
> > this
> > > doesn't mean we are advocating all code can modify all entities at
> will.
> > >  Manager is here to manage the lifecycle and changes to the entities
> they
> > > manage.  However, it is not there to provide access.  Consumers of your
> > > code should know by convention to use the entity interface instead of
> the
> > > VO objects and leave modifications to that manager.  So here's some
> > general
> > > things to think about.
> > > >
> > > > If you are writing code for CloudStack's core orchestration work:
> > > > - Write your managers to manage entities, not provide access
> > > > - Entity interfaces are for consummation so it shouldn't have set
> > > methods on them.
> > > > - Entity interfaces should be in cloud-api which defines CloudStack's
> > > sdk.
> > > > - CloudStack's core  VO and DAO are in cloud-engine-schema.   This
> > forms
> > > the schema for CloudStack.  Note that this is for the core objects
> > > CloudStack manages and exposes.  If you're writing a plugin and the
> > plugin
> > > has its own DB, there's no need to put that into cloud-engine-schema.
> > > >
> > > > If you are writing code for plugins:
> > > > - If you need to modify certain entities in cloudstack, you can add
> > > dependency to cloud-engine-schema and have access to the vo and daos.
> >  Make
> > > sure you really need to do this though.
> > > > - Never assume an interface can be cast down to the VO.
> > > > - If you are consuming an entity, use the interface not the VO.  You
> > can
> > > use EntityManager to do this.  For example, any code can do the
> following
> > > after declaring dependency on the cloud-api package.
> > > >
> > > > @Inject
> > > > EntityManager _entityMgr;
> > > >
> > > > Vpc vpc = _entityMgr.findById(Vpc.class, vpcId);
> > > > PrivateGateway pg = _entityMgr.findById(PrivateGateway.class,
> > gatewayId);
> > > >
> > > > --Alex
> > >
> >
>



-- 
*Mike Tutkowski*
*Senior CloudStack Developer, SolidFire Inc.*
e: mike.tutkowski@solidfire.com
o: 303.746.7302
Advancing the way the world uses the
cloud<http://solidfire.com/solution/overview/?video=play>
*™*

Re: [DESIGN] Access entities directly...

Posted by Amit Das <am...@cloudbyte.com>.
Hi All,

I am in process of implementing a storage plugin and would require more
clarity on this front.

I need to update the 'Volume' as well as the 'StoragePool' in a new
implementing class of PrimaryDataStoreDriver (createAsync method to be
exact).

Should following method invocations be used to update above entities.
- VolumeManager's updateVolume(UpdateVolumeCmd updateVolumeCmd)
- StorageManager's updateStoragePool(UpdateStoragePoolCmd cmd)

In other words, do I need to construct UpdateVolumeCmd &
UpdateStoragePoolCmd objects in the storage plugin code ?

Regards,
Amit
*CloudByte Inc.* <http://www.cloudbyte.com/>


On Fri, Aug 23, 2013 at 2:10 AM, Darren Shepherd <
darren.s.shepherd@gmail.com> wrote:

> Sorry clicked something that send that last email...
>
> To finish
>
> if ( obj instance VMInstanceVO )
>   return (VMInstanceVO)obj
> else
>   return findByIdIncludingRemoved(obj.getId())
>
>
> Just a thought
>
> Darren
>
>
>
> On Thu, Aug 22, 2013 at 1:38 PM, Darren Shepherd <
> darren.s.shepherd@gmail.com> wrote:
>
> > These are great points Alex is making here and I was just noticing the
> > same thing.
> >
> > One if the things I've noticed is that most managers inject a lot of
> > DAOs.  When you look at the code usually about half those DAOs are
> > needed only for findById.  So if you are consuming an object use the
> > interface and use entity manager.  Only if you need a specialized
> > findBy[SomeSpecialCriteria] should you actually inject the VO's DAO.
> >
> > The problem with injecting a lot of DAO is the injected members given
> > you a quick look at what are the logical dependencies of the
> > class/manager.  So if you see 15 DAOs you then have to assume that
> > that class may modify up to 15 types of entities.
> >
> > In other projects that have a similar DAO usage style as CloudStack,
> > it's been helpful to actually define the DAOs as returning jnterfaces
> > only.  We may consider such a pattern, or some variation of it.  The
> > reason I say this, is that if you need to get a VM by instance name
> > your going to call the vmDao.findVMByInstanceName() which returns a
> > VO.  So two problems.  1) The temptation is to use a VO in the code
> > and not the interface 2) The class, which is just a consumer now has
> > access to a DAO that can write modify.
> >
> > So one possible pattern could be to seperate the interfaces into
> > VMInstanceAccessDAO and VMInstanceDAO where the access interface has
> > all find and list methods that return "? extend VMInstance" and the
> > other interface has the modify methods.  To make this fully work, you
> > would probably have to add a getVO() method to the GenericDao that
> > will translate an inteface to a VO.  So basically
> >
> > if ( obj instance VMInstanceVO )
> >
> >
> > Darren
> >
> > On Aug 22, 2013, at 10:58 AM, Alex Huang <Al...@citrix.com> wrote:
> >
> > > Hi Everyone,
> > >
> > > I've been doing a lot of refactoring and I noticed the following type
> of
> > code in our code base.
> > >
> > > Interface VpcService {
> > >  Vpc getVpc(long id);
> > >  PrivateGateway getPrviateGateway(long id);
> > > }
> > >
> > > Interface VpcManager extends VpcService {
> > > ...
> > > }
> > >
> > > Class VpcManager implements VpcManager {
> > >  Public Vpc getVpc(long id) {
> > >       Return _vpcDao.findById(id);
> > >  }
> > >  Public PrivateGateway getPrivateGateway(long id) {
> > >       Return _privateGateway.findById(id);
> > >  }
> > > }
> > >
> > > CloudStack was specifically written so people don't have to do this.
> >  It's just useless lines that makes following code harder.
> > >
> > > I know most schools teach these abstraction concepts and there are
> valid
> > uses but they don't apply in cloudstack.  There's certainly a school of
> > thought that you need to guard your entities from outside developers.  In
> > cloudstack, that fence is at the process boundary of cloudstack or, iow,
> > that fence is cloudstack's over the wire api.  Once code got past that
> > layer, code is collaborating and there's no need for that fence.  Now,
> this
> > doesn't mean we are advocating all code can modify all entities at will.
> >  Manager is here to manage the lifecycle and changes to the entities they
> > manage.  However, it is not there to provide access.  Consumers of your
> > code should know by convention to use the entity interface instead of the
> > VO objects and leave modifications to that manager.  So here's some
> general
> > things to think about.
> > >
> > > If you are writing code for CloudStack's core orchestration work:
> > > - Write your managers to manage entities, not provide access
> > > - Entity interfaces are for consummation so it shouldn't have set
> > methods on them.
> > > - Entity interfaces should be in cloud-api which defines CloudStack's
> > sdk.
> > > - CloudStack's core  VO and DAO are in cloud-engine-schema.   This
> forms
> > the schema for CloudStack.  Note that this is for the core objects
> > CloudStack manages and exposes.  If you're writing a plugin and the
> plugin
> > has its own DB, there's no need to put that into cloud-engine-schema.
> > >
> > > If you are writing code for plugins:
> > > - If you need to modify certain entities in cloudstack, you can add
> > dependency to cloud-engine-schema and have access to the vo and daos.
>  Make
> > sure you really need to do this though.
> > > - Never assume an interface can be cast down to the VO.
> > > - If you are consuming an entity, use the interface not the VO.  You
> can
> > use EntityManager to do this.  For example, any code can do the following
> > after declaring dependency on the cloud-api package.
> > >
> > > @Inject
> > > EntityManager _entityMgr;
> > >
> > > Vpc vpc = _entityMgr.findById(Vpc.class, vpcId);
> > > PrivateGateway pg = _entityMgr.findById(PrivateGateway.class,
> gatewayId);
> > >
> > > --Alex
> >
>

Re: [DESIGN] Access entities directly...

Posted by Darren Shepherd <da...@gmail.com>.
Sorry clicked something that send that last email...

To finish

if ( obj instance VMInstanceVO )
  return (VMInstanceVO)obj
else
  return findByIdIncludingRemoved(obj.getId())


Just a thought

Darren



On Thu, Aug 22, 2013 at 1:38 PM, Darren Shepherd <
darren.s.shepherd@gmail.com> wrote:

> These are great points Alex is making here and I was just noticing the
> same thing.
>
> One if the things I've noticed is that most managers inject a lot of
> DAOs.  When you look at the code usually about half those DAOs are
> needed only for findById.  So if you are consuming an object use the
> interface and use entity manager.  Only if you need a specialized
> findBy[SomeSpecialCriteria] should you actually inject the VO's DAO.
>
> The problem with injecting a lot of DAO is the injected members given
> you a quick look at what are the logical dependencies of the
> class/manager.  So if you see 15 DAOs you then have to assume that
> that class may modify up to 15 types of entities.
>
> In other projects that have a similar DAO usage style as CloudStack,
> it's been helpful to actually define the DAOs as returning jnterfaces
> only.  We may consider such a pattern, or some variation of it.  The
> reason I say this, is that if you need to get a VM by instance name
> your going to call the vmDao.findVMByInstanceName() which returns a
> VO.  So two problems.  1) The temptation is to use a VO in the code
> and not the interface 2) The class, which is just a consumer now has
> access to a DAO that can write modify.
>
> So one possible pattern could be to seperate the interfaces into
> VMInstanceAccessDAO and VMInstanceDAO where the access interface has
> all find and list methods that return "? extend VMInstance" and the
> other interface has the modify methods.  To make this fully work, you
> would probably have to add a getVO() method to the GenericDao that
> will translate an inteface to a VO.  So basically
>
> if ( obj instance VMInstanceVO )
>
>
> Darren
>
> On Aug 22, 2013, at 10:58 AM, Alex Huang <Al...@citrix.com> wrote:
>
> > Hi Everyone,
> >
> > I've been doing a lot of refactoring and I noticed the following type of
> code in our code base.
> >
> > Interface VpcService {
> >  Vpc getVpc(long id);
> >  PrivateGateway getPrviateGateway(long id);
> > }
> >
> > Interface VpcManager extends VpcService {
> > ...
> > }
> >
> > Class VpcManager implements VpcManager {
> >  Public Vpc getVpc(long id) {
> >       Return _vpcDao.findById(id);
> >  }
> >  Public PrivateGateway getPrivateGateway(long id) {
> >       Return _privateGateway.findById(id);
> >  }
> > }
> >
> > CloudStack was specifically written so people don't have to do this.
>  It's just useless lines that makes following code harder.
> >
> > I know most schools teach these abstraction concepts and there are valid
> uses but they don't apply in cloudstack.  There's certainly a school of
> thought that you need to guard your entities from outside developers.  In
> cloudstack, that fence is at the process boundary of cloudstack or, iow,
> that fence is cloudstack's over the wire api.  Once code got past that
> layer, code is collaborating and there's no need for that fence.  Now, this
> doesn't mean we are advocating all code can modify all entities at will.
>  Manager is here to manage the lifecycle and changes to the entities they
> manage.  However, it is not there to provide access.  Consumers of your
> code should know by convention to use the entity interface instead of the
> VO objects and leave modifications to that manager.  So here's some general
> things to think about.
> >
> > If you are writing code for CloudStack's core orchestration work:
> > - Write your managers to manage entities, not provide access
> > - Entity interfaces are for consummation so it shouldn't have set
> methods on them.
> > - Entity interfaces should be in cloud-api which defines CloudStack's
> sdk.
> > - CloudStack's core  VO and DAO are in cloud-engine-schema.   This forms
> the schema for CloudStack.  Note that this is for the core objects
> CloudStack manages and exposes.  If you're writing a plugin and the plugin
> has its own DB, there's no need to put that into cloud-engine-schema.
> >
> > If you are writing code for plugins:
> > - If you need to modify certain entities in cloudstack, you can add
> dependency to cloud-engine-schema and have access to the vo and daos.  Make
> sure you really need to do this though.
> > - Never assume an interface can be cast down to the VO.
> > - If you are consuming an entity, use the interface not the VO.  You can
> use EntityManager to do this.  For example, any code can do the following
> after declaring dependency on the cloud-api package.
> >
> > @Inject
> > EntityManager _entityMgr;
> >
> > Vpc vpc = _entityMgr.findById(Vpc.class, vpcId);
> > PrivateGateway pg = _entityMgr.findById(PrivateGateway.class, gatewayId);
> >
> > --Alex
>

Re: [DESIGN] Access entities directly...

Posted by Darren Shepherd <da...@gmail.com>.
These are great points Alex is making here and I was just noticing the
same thing.

One if the things I've noticed is that most managers inject a lot of
DAOs.  When you look at the code usually about half those DAOs are
needed only for findById.  So if you are consuming an object use the
interface and use entity manager.  Only if you need a specialized
findBy[SomeSpecialCriteria] should you actually inject the VO's DAO.

The problem with injecting a lot of DAO is the injected members given
you a quick look at what are the logical dependencies of the
class/manager.  So if you see 15 DAOs you then have to assume that
that class may modify up to 15 types of entities.

In other projects that have a similar DAO usage style as CloudStack,
it's been helpful to actually define the DAOs as returning jnterfaces
only.  We may consider such a pattern, or some variation of it.  The
reason I say this, is that if you need to get a VM by instance name
your going to call the vmDao.findVMByInstanceName() which returns a
VO.  So two problems.  1) The temptation is to use a VO in the code
and not the interface 2) The class, which is just a consumer now has
access to a DAO that can write modify.

So one possible pattern could be to seperate the interfaces into
VMInstanceAccessDAO and VMInstanceDAO where the access interface has
all find and list methods that return "? extend VMInstance" and the
other interface has the modify methods.  To make this fully work, you
would probably have to add a getVO() method to the GenericDao that
will translate an inteface to a VO.  So basically

if ( obj instance VMInstanceVO )


Darren

On Aug 22, 2013, at 10:58 AM, Alex Huang <Al...@citrix.com> wrote:

> Hi Everyone,
>
> I've been doing a lot of refactoring and I noticed the following type of code in our code base.
>
> Interface VpcService {
>  Vpc getVpc(long id);
>  PrivateGateway getPrviateGateway(long id);
> }
>
> Interface VpcManager extends VpcService {
> ...
> }
>
> Class VpcManager implements VpcManager {
>  Public Vpc getVpc(long id) {
>       Return _vpcDao.findById(id);
>  }
>  Public PrivateGateway getPrivateGateway(long id) {
>       Return _privateGateway.findById(id);
>  }
> }
>
> CloudStack was specifically written so people don't have to do this.  It's just useless lines that makes following code harder.
>
> I know most schools teach these abstraction concepts and there are valid uses but they don't apply in cloudstack.  There's certainly a school of thought that you need to guard your entities from outside developers.  In cloudstack, that fence is at the process boundary of cloudstack or, iow, that fence is cloudstack's over the wire api.  Once code got past that layer, code is collaborating and there's no need for that fence.  Now, this doesn't mean we are advocating all code can modify all entities at will.  Manager is here to manage the lifecycle and changes to the entities they manage.  However, it is not there to provide access.  Consumers of your code should know by convention to use the entity interface instead of the VO objects and leave modifications to that manager.  So here's some general things to think about.
>
> If you are writing code for CloudStack's core orchestration work:
> - Write your managers to manage entities, not provide access
> - Entity interfaces are for consummation so it shouldn't have set methods on them.
> - Entity interfaces should be in cloud-api which defines CloudStack's sdk.
> - CloudStack's core  VO and DAO are in cloud-engine-schema.   This forms the schema for CloudStack.  Note that this is for the core objects CloudStack manages and exposes.  If you're writing a plugin and the plugin has its own DB, there's no need to put that into cloud-engine-schema.
>
> If you are writing code for plugins:
> - If you need to modify certain entities in cloudstack, you can add dependency to cloud-engine-schema and have access to the vo and daos.  Make sure you really need to do this though.
> - Never assume an interface can be cast down to the VO.
> - If you are consuming an entity, use the interface not the VO.  You can use EntityManager to do this.  For example, any code can do the following after declaring dependency on the cloud-api package.
>
> @Inject
> EntityManager _entityMgr;
>
> Vpc vpc = _entityMgr.findById(Vpc.class, vpcId);
> PrivateGateway pg = _entityMgr.findById(PrivateGateway.class, gatewayId);
>
> --Alex