> On June 12, 2017, 2:06 p.m., Laszlo Puskas wrote: > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UserEntity.java > > Lines 97 (patched) > > <https://reviews.apache.org/r/59956/diff/1/?file=1747000#file1747000line119> > > > > I think cascading should be removed here (AFAIK there were some issues > > where cascading was interfering with some DAO logic) > > Robert Levas wrote: > Cascade seems to be used a few lines up in this file... > ``` > @OneToMany(mappedBy = "user", cascade = CascadeType.ALL) > private Set<MemberEntity> memberEntities = new HashSet<>(); > ``` > > Other than that, a quick search in the source code reveals 72 occurances > of this, 42 instances are of `CascadeType.ALL`. > > So far my tests seem to work ok. > > Sebastian Toader wrote: > There was already an attempt to remove the Cascadings. I guess these are > left overs. > > Robert Levas wrote: > What is the alternative to cascading? If I remove a user, I will need to > add code in the DAO to first remove the user authentication records? > > Jonathan Hurley wrote: > I don't have problems with cascading as long as the entity relationships > are setup correctly for it. In many places, they are not and we need to rely > on the DAO to do the work. Maintaining these methods seems to be a source of > bugs, so if the entities here can be cascaded correctly, then I don't see why > it would be a problem. > > Robert Levas wrote: > The entity relationship is set up properly: > > ``` > CONSTRAINT FK_user_authentication_users FOREIGN KEY (user_id) REFERENCES > users(user_id) > ``` > > So how do we want to proceed? Laszlo?
I am not against cascading; not using them may be a convention so that all operations are explicitly done in the code (its easier to follow what's going on, cascading induces some kind of "side effect"). But I am with Jonathan let's keep the cascading here. - Laszlo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59956/#review177610 ----------------------------------------------------------- On June 14, 2017, 9:22 p.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59956/ > ----------------------------------------------------------- > > (Updated June 14, 2017, 9:22 p.m.) > > > Review request for Ambari, Attila Magyar, Balázs Bence Sári, Eugene > Chekanskiy, Jonathan Hurley, Laszlo Puskas, and Sebastian Toader. > > > Bugs: AMBARI-21147 > https://issues.apache.org/jira/browse/AMBARI-21147 > > > Repository: ambari > > > Description > ------- > > Update Database Access Layer to Support New Database Schema for Improved User > Account Management. > > * Update `org.apache.ambari.server.orm.entities.UserEntity` > * Update `org.apache.ambari.server.orm.dao.UserDAO` > * Add `org.apache.ambari.server.orm.entities.UserAuthenticationEntity` > * Add `org.apache.ambari.server.orm.dao.UserAuthenticationDAO` > > Note: Some changes will be revisited when updating the different > authentication processes to work with the improved user account management > code. > > > Diffs > ----- > > ambari-server/docs/api/generated/index.html 7ea4297b99 > ambari-server/docs/api/generated/swagger.json d7d54a510f > > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java > fb06e6d8a5 > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java > 807bded873 > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java > 8d262e269b > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java > aeba739a6d > > ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java > f3c2ec871b > > ambari-server/src/main/java/org/apache/ambari/server/controller/ResourceProviderFactory.java > 391213858e > > ambari-server/src/main/java/org/apache/ambari/server/controller/UserRequest.java > 40818c8f48 > > ambari-server/src/main/java/org/apache/ambari/server/controller/UserResponse.java > 5afacb70ef > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java > b35b2a8612 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProvider.java > 389f0b2bf2 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserPrivilegeResourceProvider.java > 614f7abda1 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserResourceProvider.java > c5c36e9942 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserAuthenticationDAO.java > PRE-CREATION > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserDAO.java > ce47c4c38c > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UserAuthenticationEntity.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UserEntity.java > 9011eaecec > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariJWTAuthenticationFilter.java > 195c55afa5 > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AuthenticationMethodNotAllowedException.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/kerberos/AmbariAuthToLocalUserDetailsService.java > 1e4f6ead08 > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/kerberos/AmbariKerberosAuthenticationProperties.java > 09422e51e3 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java > ce9a79023d > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProvider.java > b7ff297ce5 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthoritiesPopulator.java > d38d44c16f > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariLocalUserProvider.java > 37d5d49c37 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariPamAuthenticationProvider.java > 373552e6e1 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariUserAuthorizationFilter.java > 95e90b3e49 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AuthorizationHelper.java > 64d5e6124f > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/User.java > bff1fd2a16 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/UserType.java > aabd368aeb > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java > 9cdde8fe4d > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/internal/AmbariInternalAuthenticationProvider.java > 383e8fac87 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/jwt/AuthenticationJwtUserNotFoundException.java > f18af101a2 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/jwt/JwtAuthenticationFilter.java > e27afdbade > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java > f413c697d6 > ambari-server/src/main/resources/META-INF/persistence.xml e4045ef536 > > ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java > 1b8de79737 > > ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java > 3215e7246d > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractPrivilegeResourceProviderTest.java > 547bba57ad > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProviderTest.java > 4dc06b92c8 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/GroupPrivilegeResourceProviderTest.java > 36f6a1e2e4 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UserPrivilegeResourceProviderTest.java > 9ccbc11529 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UserResourceProviderDBTest.java > c4f0f349fb > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UserResourceProviderTest.java > d298b7f135 > ambari-server/src/test/java/org/apache/ambari/server/orm/OrmTestHelper.java > 271d5368ad > > ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UserDAOTest.java > 05733fa57a > > ambari-server/src/test/java/org/apache/ambari/server/security/SecurityHelperImplTest.java > f15f2f5218 > > ambari-server/src/test/java/org/apache/ambari/server/security/authentication/AmbariJWTAuthenticationFilterTest.java > de5b768863 > > ambari-server/src/test/java/org/apache/ambari/server/security/authentication/kerberos/AmbariAuthToLocalUserDetailsServiceTest.java > 530bf651bb > > ambari-server/src/test/java/org/apache/ambari/server/security/authentication/kerberos/AmbariKerberosAuthenticationPropertiesTest.java > eb26cd839b > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java > 15e243e224 > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationProviderDisableUserTest.java > 891ab38638 > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProviderForDNWithSpaceTest.java > 442414f14d > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProviderTest.java > 4941bc7afb > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLocalUserProviderTest.java > 2362823b30 > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariPamAuthenticationProviderTest.java > 8faa6ce316 > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariUserAuthenticationFilterTest.java > 0483b04ec0 > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/TestAmbariLdapAuthoritiesPopulator.java > fff39d8bf3 > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/TestUsers.java > e29791f19b > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java > ac91c904ac > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/jwt/JwtAuthenticationFilterTest.java > 24f5f88490 > > ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java > 63b69277a4 > > ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog240Test.java > f10665825c > > > Diff: https://reviews.apache.org/r/59956/diff/3/ > > > Testing > ------- > > Manually tested in cluster > > > # Local test results: > ``` > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 34:03 min > [INFO] Finished at: 2017-06-09T17:05:32-04:00 > [INFO] Final Memory: 209M/1768M > [INFO] > ------------------------------------------------------------------------ > ``` > > # Jenkins test results: PENDING > > > Thanks, > > Robert Levas > >