> On June 12, 2017, 10:06 a.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?
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. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59956/#review177610 ----------------------------------------------------------- On June 12, 2017, 1:15 p.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59956/ > ----------------------------------------------------------- > > (Updated June 12, 2017, 1:15 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 > eb64030e45 > > 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/2/ > > > 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 > >
