----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17790/#review37097 -----------------------------------------------------------
Patch B. * You shouldn’t make your service a part of cloud-mom-rabbitmq plugin. Your subscribers/implementation are specific to your feature, and you need to introduce a special plugin just for your service. * AccountInterface and BaseInterface are still regular classes. You should split them into Service interface /ManagerImpl or Manager interface /ManagerImpl as Chiradeep suggested. * Once you extract services interfaces, make sure you don’t use VO objects in methods signatures. * You should really get a use of @Manager interface and @Inject annotations for autowiring your managers instead of setting them up using ComponentContext.getComponent() in each of your manager classes. Patch A. * AccountResponse. Why did you add domain path to the account response? You can always retrieve it by a) get domain info from account response b) execute listDomains&domainId to get the domain path. Try not to overload the response with attributes that don’t belong to response’s first class object. Generic comments. * I can see that you do a lot of string concatenation this way: paramStr += "&email=" + email + "&firstname=" + firstName + "&lastname=" + lastName + "&accounttype=" + accountType; I would suggest to use StringBuilder instead. - Alena Prokharchyk On March 12, 2014, 3:57 p.m., Alex Ough wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17790/ > ----------------------------------------------------------- > > (Updated March 12, 2014, 3:57 p.m.) > > > Review request for cloudstack. > > > Repository: cloudstack-git > > > Description > ------- > > Currently, under the environment of cloudstack with multiple regions, each > region has its own management server running with a separate database, which > will cause data discrepancies when users create/update/delete > domain/account/user data independently in each management server. So to > support multiple regions and provide one point of entry for each customer, > this implementation duplicates domain/account/user information of customers > in one region to all of the regions independently whenever there is any > change. > > https://issues.apache.org/jira/browse/CLOUDSTACK-4992 > https://cwiki.apache.org/confluence/display/CLOUDSTACK/Domain-Account-User+Sync+Up+Among+Multiple+Regions > > > Diffs > ----- > > > plugins/event-bus/rabbitmq/src/org/apache/cloudstack/mom/rabbitmq/AccountSubscriber.java > PRE-CREATION > > plugins/event-bus/rabbitmq/src/org/apache/cloudstack/mom/rabbitmq/DomainSubscriber.java > PRE-CREATION > > plugins/event-bus/rabbitmq/src/org/apache/cloudstack/mom/rabbitmq/MultiRegionEventBus.java > PRE-CREATION > > plugins/event-bus/rabbitmq/src/org/apache/cloudstack/mom/rabbitmq/MultiRegionSubscriber.java > PRE-CREATION > > plugins/event-bus/rabbitmq/src/org/apache/cloudstack/mom/rabbitmq/UserSubscriber.java > PRE-CREATION > > plugins/event-bus/rabbitmq/test/org/apache/cloudstack/mom/rabbitmq/AccountSubscriberTest.java > PRE-CREATION > > plugins/event-bus/rabbitmq/test/org/apache/cloudstack/mom/rabbitmq/DomainSubscriberTest.java > PRE-CREATION > > plugins/event-bus/rabbitmq/test/org/apache/cloudstack/mom/rabbitmq/MultiRegionSubscriberTest.java > PRE-CREATION > > plugins/event-bus/rabbitmq/test/org/apache/cloudstack/mom/rabbitmq/UserSubscriberTest.java > PRE-CREATION > server/src/org/apache/cloudstack/region/multi/api/AccountInterface.java > PRE-CREATION > server/src/org/apache/cloudstack/region/multi/api/BaseInterface.java > PRE-CREATION > server/src/org/apache/cloudstack/region/multi/api/DomainInterface.java > PRE-CREATION > server/src/org/apache/cloudstack/region/multi/api/UserInterface.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/service/AccountFullSyncProcessor.java > PRE-CREATION > server/src/org/apache/cloudstack/region/multi/service/AccountService.java > PRE-CREATION > server/src/org/apache/cloudstack/region/multi/service/BaseService.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/service/DomainFullSyncProcessor.java > PRE-CREATION > server/src/org/apache/cloudstack/region/multi/service/DomainService.java > PRE-CREATION > server/src/org/apache/cloudstack/region/multi/service/FullScanner.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/service/FullSyncProcessor.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/service/LocalAccountManager.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/service/LocalDomainManager.java > PRE-CREATION > server/src/org/apache/cloudstack/region/multi/service/LocalUserManager.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/service/RemoteAccountEventProcessor.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/service/RemoteDomainEventProcessor.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/service/RemoteEventProcessor.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/service/RemoteUserEventProcessor.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/service/UserFullSyncProcessor.java > PRE-CREATION > server/src/org/apache/cloudstack/region/multi/service/UserService.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/simulator/AccountLocalGenerator.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/simulator/AccountLocalGeneratorEvent.java > PRE-CREATION > server/src/org/apache/cloudstack/region/multi/simulator/AutoGenerator.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/simulator/DomainLocalGenerator.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/simulator/DomainLocalGeneratorEvent.java > PRE-CREATION > server/src/org/apache/cloudstack/region/multi/simulator/LocalGenerator.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/simulator/UserLocalGenerator.java > PRE-CREATION > > server/src/org/apache/cloudstack/region/multi/simulator/UserLocalGeneratorEvent.java > PRE-CREATION > > server/test/org/apache/cloudstack/region/multi/api/AccountInterfaceTest.java > PRE-CREATION > server/test/org/apache/cloudstack/region/multi/api/BaseInterfaceTest.java > PRE-CREATION > server/test/org/apache/cloudstack/region/multi/api/DomainInterfaceTest.java > PRE-CREATION > server/test/org/apache/cloudstack/region/multi/api/UserInterfaceTest.java > PRE-CREATION > > server/test/org/apache/cloudstack/region/multi/service/AccountFullSyncProcessorTest.java > PRE-CREATION > server/test/org/apache/cloudstack/region/multi/service/BaseServiceTest.java > PRE-CREATION > > server/test/org/apache/cloudstack/region/multi/service/DomainFullSyncProcessorTest.java > PRE-CREATION > > server/test/org/apache/cloudstack/region/multi/service/RemoteAccountEventProcessorTest.java > PRE-CREATION > > server/test/org/apache/cloudstack/region/multi/service/RemoteDomainEventProcessorTest.java > PRE-CREATION > > server/test/org/apache/cloudstack/region/multi/service/RemoteUserEventProcessorTest.java > PRE-CREATION > > server/test/org/apache/cloudstack/region/multi/service/UserFullSyncProcessorTest.java > PRE-CREATION > > server/test/org/apache/cloudstack/region/multi/simulator/AccountLocalGeneratorEventTest.java > PRE-CREATION > > server/test/org/apache/cloudstack/region/multi/simulator/AccountLocalGeneratorTest.java > PRE-CREATION > > server/test/org/apache/cloudstack/region/multi/simulator/DomainLocalGeneratorEventTest.java > PRE-CREATION > > server/test/org/apache/cloudstack/region/multi/simulator/DomainLocalGeneratorTest.java > PRE-CREATION > > server/test/org/apache/cloudstack/region/multi/simulator/LocalGeneratorTest.java > PRE-CREATION > > server/test/org/apache/cloudstack/region/multi/simulator/UserLocalGeneratorEventTest.java > PRE-CREATION > > server/test/org/apache/cloudstack/region/multi/simulator/UserLocalGeneratorTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/17790/diff/ > > > Testing > ------- > > 1. Successfully tested real time synchronization as soon as resources are > created/deleted/modified in one region. > 2. Successfully tested full scans to synchronize resources that were missed > during real time synchronization because of any reasons like network > connection issues. > 3. The tests were done manually and also automatically by randomly generating > changes each region. > > > Thanks, > > Alex Ough > >
