Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Received-SPF: pass (athena.apache.org: domain of darren.s.shepherd@gmail.com
 designates 74.125.82.50 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <20CF38CB4385CE4D9D1558D52A0FC0580E7395@SJCPEX01CL03.citrite.net>
References: <20CF38CB4385CE4D9D1558D52A0FC0580E7395@SJCPEX01CL03.citrite.net>
Date: Thu, 22 Aug 2013 13:38:53 -0700
Message-ID: 
 <CAMCEDC3tCEh3VcZMMxPy9CQcB32PofJTb=h-B9uj7ap16RH-Rw@mail.gmail.com>
Subject: Re: [DESIGN] Access entities directly...
From: Darren Shepherd <darren.s.shepherd@gmail.com>
To: "dev@cloudstack.apache.org" <dev@cloudstack.apache.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

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 <Alex.Huang@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 tho=
ught that you need to guard your entities from outside developers.  In clou=
dstack, that fence is at the process boundary of cloudstack or, iow, that f=
ence is cloudstack's over the wire api.  Once code got past that layer, cod=
e is collaborating and there's no need for that fence.  Now, this doesn't m=
ean 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.  How=
ever, it is not there to provide access.  Consumers of your code should kno=
w by convention to use the entity interface instead of the VO objects and l=
eave 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 CloudSta=
ck 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 depen=
dency 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 a=
fter declaring dependency on the cloud-api package.
>
> @Inject
> EntityManager _entityMgr;
>
> Vpc vpc =3D _entityMgr.findById(Vpc.class, vpcId);
> PrivateGateway pg =3D _entityMgr.findById(PrivateGateway.class, gatewayId=
);
>
> --Alex