This is a very good topic.  Here's my $.02 on this.

The reason why AccountMgr has so many dependencies is because of the following:

"Oh deleting an account?  I have to delete all the vms, I have to delete the 
volumes, I have to remove all firewall rules, I have to release ip address, 
etc"  So the developers naturally add it to AccountMgr since they're all part 
of an account deletion.  Imagine that for all of accountmanager's functions.  
Then that tightly couples the internals of CloudStack.  

What it really should do is follow the pattern set in AgentManager.  
AgentManager says I take care of agent connections but if you're interested in 
doing processing due to agent connection loss/connect, then register a listener 
with me and I will publish the agent connection events.  That way all other 
managers are decoupled from AgentManager.

Now, this is different from a pure publisher/subscriber model because in this 
we don't want the processing to be completely independent of each other since 
the account manager does want to know if some resources have not been cleaned 
up.  We just want the code to be decoupled.

In this model, it would work like this.

On delete:
- AccountManager sets the state of the account to 'Deleted'.  This causes the 
account to not be able to login.
- AccountManager fires a DELETE event.
- The thread actually will go to all of the listeners and processes delete so 
code to handle vms, volumes, rules, ip addresses etc all reside in their own 
managers.
- This event may cause ripple effects throughout the Managers as each Manager 
should have their own listener structure. 
- Once all of that is done, then AccountManager finally removes the row from 
the table.

On error:
- AccountManager appropriately notifies the caller.
- The caller then fixes the problem and calls delete again.
- Account Manager then again goes through the procedure above.

All of CloudStack's managers should behave in this pattern.  I would encourage 
all of the developers to study this and contact me with any questions.

--Alex


> -----Original Message-----
> From: Joe San [mailto:[email protected]]
> Sent: Saturday, February 02, 2013 5:57 AM
> To: [email protected]
> Subject: [Refactor AccountManagerImpl][DISCUSS]
> 
> Cloudstack dev team,
> 
> I would like to propose to refactor the AccountManagerImpl.java. The idea
> would be to make the implementation without any instance fields. At
> present
> there are a lot of Dao's being injected. This brings a lot of cross
> references to other dao's as well. Ideally AccountManagerImpl only needs
> the AccountDAO and should only contain methods pertaining to Account
> related activities. Any other DAO calls like NetworkDAO should be handled
> through the NetworkManagerImpl (needs to be created if one does not
> exist).
> 
> I would love to see AccountManagerImpl free of any injections.
> 
> Regards,
> Joe

Reply via email to