> On June 14, 2017, 11:46 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserAuthenticationDAO.java
> > Lines 43 (patched)
> > <https://reviews.apache.org/r/59956/diff/2/?file=1748767#file1748767line43>
> >
> > Should this PK be a Long?
Probably... copy/paste issue from UserEntity...
```
@RequiresSession
public UserEntity findByPK(Integer userPK) {
return entityManagerProvider.get().find(UserEntity.class, userPK);
}
```
Fixing.
> On June 14, 2017, 11:46 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserAuthenticationDAO.java
> > Lines 49 (patched)
> > <https://reviews.apache.org/r/59956/diff/2/?file=1748767#file1748767line49>
> >
> > Can you make this a NamedQuery in the UserAuthenticationEntity?
Fixing.
> On June 14, 2017, 11:46 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserAuthenticationDAO.java
> > Lines 79 (patched)
> > <https://reviews.apache.org/r/59956/diff/2/?file=1748767#file1748767line79>
> >
> > remove(merge()) ? Is this necessary? Seems like the merge would only
> > keep entities consistent before the removal if you did something like
> > change a collection. I wouldn't think you'd need to do this.
Another copy/paste issue
```
@Transactional
public void remove(UserEntity user) {
entityManagerProvider.get().remove(merge(user));
entityManagerProvider.get().getEntityManagerFactory().getCache().evictAll();
}
```
Fixing.
> On June 14, 2017, 11:46 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserAuthenticationDAO.java
> > Lines 80 (patched)
> > <https://reviews.apache.org/r/59956/diff/2/?file=1748767#file1748767line80>
> >
> > Why do you need to evict from the cache here? Since you're going
> > through the entity for this, JPA should know to clean the shared caches
> > correctly. I only would think this is necessary if you use JPQL to remove
> > entities.
Seems like there may have been caching issues... but maybe only related to
views? See [AMBARI-7152](https://issues.apache.org/jira/browse/AMBARI-7152).
Fixing.
> On June 14, 2017, 11:46 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserDAO.java
> > Lines 118 (patched)
> > <https://reviews.apache.org/r/59956/diff/2/?file=1748768#file1748768line200>
> >
> > Can you do the lowercase() in the entity instead? This way you don't
> > need to remember to do it.
Fixing.
> On June 14, 2017, 11:46 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UserAuthenticationEntity.java
> > Lines 73-76 (patched)
> > <https://reviews.apache.org/r/59956/diff/2/?file=1748769#file1748769line73>
> >
> > Always initialized, but not enforced to be non-null.
Fixing... Seems like there is no real need to set update and create time. I
will remove those.
> On June 14, 2017, 11:46 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UserAuthenticationEntity.java
> > Lines 142-156 (patched)
> > <https://reviews.apache.org/r/59956/diff/2/?file=1748769#file1748769line142>
> >
> > Any reason you didn't use EqualsBuilder here instead?
`EqualsBuilder` is new to me. Fixing.
> On June 14, 2017, 11:46 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UserEntity.java
> > Lines 76-83 (patched)
> > <https://reviews.apache.org/r/59956/diff/2/?file=1748770#file1748770line98>
> >
> > Are these all nullable? The default when not specified is true.
`consecutive_failures` is not nullable, but the others are. Fixing.
- Robert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59956/#review177909
-----------------------------------------------------------
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
>
>
