> On Feb. 13, 2017, 3:49 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java,
> >  lines 243-251
> > <https://reviews.apache.org/r/56599/diff/1/?file=1631928#file1631928line243>
> >
> >     Any reason you didn't use a Set here (especially since the contract 
> > only defines a Collection)?
> 
> Attila Magyar wrote:
>     The existing code uses ArrayList. I'm not sure if it will break anything 
> if I change it to Set.

@jhurley

If we change it to set then we should remove the setter, and use 
addAuthorization() all over the place.

public void setAuthorizations(Collection<RoleAuthorizationEntity> 
authorizations) {
  this.authorizations = authorizations;
}


The setAuthorizations() is used at one place in production code, and in 10 
tests. But it will behave differently because it won't preserver the ordering. 
Or we can use LinkedHashSet to preserve the ordering too. What do you think?


- Attila


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165341
-----------------------------------------------------------


On Feb. 13, 2017, 1:48 p.m., Attila Magyar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2017, 1:48 p.m.)
> 
> 
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert 
> Levas, and Sebastian Toader.
> 
> 
> Bugs: AMBARI-19984
>     https://issues.apache.org/jira/browse/AMBARI-19984
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Running ambari-upgrade multiple times cause an exception due to violation of 
> unique database constraint (the same RoleAuthorization can be added multiple 
> times to admin permissions).
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java
>  fb01cca 
>   
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java
>  4b33bcd 
>   
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java
>  0cd4f12f 
>   
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java
>  1c742ef 
> 
> Diff: https://reviews.apache.org/r/56599/diff/
> 
> 
> Testing
> -------
> 
> Added new unittest that checks if adding RoleAuthorization is idempotent.
> 
> Tested manually:
>  - installed ambari 2.4
>  - upgraded to ambari 2.5
>  - set version back to 2.4 by running: update metainfo set metainfo_value = 
> '2.4.0' where metainfo_key = 'version';
>  - upgraded to ambari 2.5 again
>  - started ambari-server checked the UI
> 
> 
> Existing tests ran clealy
> 
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
> 
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
> 
> 
> Thanks,
> 
> Attila Magyar
> 
>

Reply via email to