> On Jan. 24, 2014, 6:52 a.m., Prachi Damle wrote: > > I see many files unrelated to this fix have been updated and present in the > > patch. > > Please can you update the patch with changes needed for the fix only? > > > > Following are some such files. > > api/src/com/cloud/user/AccountService.java > > engine/schema/src/com/cloud/user/UserVO.java > > plugins/dedicated-resources/test/org/apache/cloudstack/dedicated/manager/DedicatedApiUnitTest.java: > > 8 changes [ 1 2 3 4 5 6 7 8 ] > > plugins/deployment-planners/implicit-dedication/test/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java: > > 16 changes [ 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 ] > > plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java > > plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java > > > > Also the below file has all Mockito references replaced by Matchers - what > > is the reason for this change? > > plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java > > Rajani Karuturi wrote: > The major changes for this patch are in the following files: > api/src/com/cloud/user/AccountService.java: 1 change [ 1 ] - > added a > new overloaded method to take the source of the account for the create call > api/src/com/cloud/user/User.java: 2 changes [ 1 2 ] -> added the new > field's getter and its enum > engine/schema/src/com/cloud/user/UserVO.java: 4 changes [ 1 2 3 4 ] -> > mapped the new database cloumn and added getter/setter, also changed the > constructor to take the new field > > plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java: > 2 changes [ 1 2 ] -> this is the one which is called when creating an ldap > user and this calls create account with source as LDAP > > plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java: > 2 changes [ 1 2 ] -> this is ldap's import multiple users command. change is > same as above > server/src/com/cloud/user/AccountManagerImpl.java: 6 changes [ 1 2 3 4 5 > 6 ] -> implementation of the new method > setup/db/db/schema-421to430.sql: 2 changes [ 1 2 ] -> SQL to add the new > column > > The rest changes in tests/mock classes are due to change in constructor > of UserVo which cant be avoided > VmwareDatacenterApiUnitTest also has the same change as above. I dont see > the Mockito/Matchers change you mentioned. > https://reviews.apache.org/r/17120/diff/#5 > > Also note that patch1/diff1 is for 4.3 and and patch2/diff2 is for > master. As we are getting conflicts due to spaces changes I created two > patches. The changes are same in both the patches. > >
You can avoid changing all the tests/mock classes. Don't change the existing UserVo constuctor - in the existing constructor default to source NATIVE so that no other callers are changed. Add a new constructor to take in source. What about existing ldap users on some existing setup - is there a way to identify those? We should add upgrade steps to update the 'source' for those user records - Prachi ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17120/#review32702 ----------------------------------------------------------- On Jan. 22, 2014, 7:24 a.m., Rajani Karuturi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17120/ > ----------------------------------------------------------- > > (Updated Jan. 22, 2014, 7:24 a.m.) > > > Review request for cloudstack, Abhinandan Prateek, Ian Duffy, Min Chen, and > Prachi Damle. > > > Bugs: CLOUDSTACK-5910 > https://issues.apache.org/jira/browse/CLOUDSTACK-5910 > > > Repository: cloudstack-git > > > Description > ------- > > added an extra column to mark the imported ldap users as from ldap > > > Diffs > ----- > > api/src/com/cloud/user/AccountService.java a9be292 > api/src/com/cloud/user/User.java 36e9028 > engine/schema/src/com/cloud/user/UserVO.java 68879f6 > > plugins/dedicated-resources/test/org/apache/cloudstack/dedicated/manager/DedicatedApiUnitTest.java > 213174b > > plugins/deployment-planners/implicit-dedication/test/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java > 4182193 > > plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java > 4a00489 > > plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java > 2f81688 > > plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java > 100ffe6 > > plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java > 89cec65 > server/src/com/cloud/user/AccountManagerImpl.java 5204589 > server/test/com/cloud/configuration/ConfigurationManagerTest.java f1bb30a > server/test/com/cloud/network/DedicateGuestVlanRangesTest.java 1615b84 > server/test/com/cloud/user/MockAccountManagerImpl.java 62e7fc8 > server/test/com/cloud/vm/UserVmManagerTest.java 83f7520 > server/test/com/cloud/vpc/NetworkACLManagerTest.java 629afa3 > server/test/com/cloud/vpc/NetworkACLServiceTest.java 786789f > server/test/org/apache/cloudstack/affinity/AffinityApiUnitTest.java 061fd42 > server/test/org/apache/cloudstack/network/lb/CertServiceTest.java a67a9ab > > server/test/org/apache/cloudstack/region/gslb/GlobalLoadBalancingRulesServiceImplTest.java > d152c66 > setup/db/db/schema-421to430.sql ccff7c1 > > Diff: https://reviews.apache.org/r/17120/diff/ > > > Testing > ------- > > manually tested > > > Thanks, > > Rajani Karuturi > >
