geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kirk Lund <kirk.l...@gmail.com>
Subject Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService
Date Tue, 13 Jun 2017 19:00:05 GMT

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



-1 I don't see why these changes are needed other than to wipe out what I did? For example,
you've undone my introduction of RealmInitializer and ConfigInitializer. I introduced those
so I can continue writing more mocking tests for SecurityService so that our unit tests don't
have to be integration tests that actually interact with Shiro. We should be able to separate
our code. Also, I really prefer using a "DisabledSecurityService" when security is disabled
rather than using a "LegacySecurityService" -- when security is disabled, it isn't "legacy".

- Kirk Lund


On June 13, 2017, 4:49 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60028/
> -----------------------------------------------------------
> 
> (Updated June 13, 2017, 4:49 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick
Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * combine EnabledSecurityService and CustomSecurityService into IntegratedSecurityService
> * combine DisabledSecurityService and LegacySecurityService
> * provide default impelementations of SecurityService
> * consolidate SecurityService creation.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
22edb6f06c7791929cc9a033ca1a1bfed5751a47 
>   geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java
3ff632d3857189513243959ee96da89da66d5a27 
>   geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java
0ba1cb62468f15fda60a94dac9c781fe1793b28f 
>   geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java
b50569036db1acac927f08ee5c1771c72ede3c1d 
>   geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java
f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
>   geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java
44562537bc426e47a42d680bb410dc59bf9bd854 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java a4041e198f1a9a4961915504c51256d0b03aa7c2

>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java
02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java
8ae76d22b628b3175db45116b80dfcfbe34aba1d 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java
60f014b9c33732a4ea134a654e666a9439b210bb 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java
51449fdd5570494f3cf91325985a5eb9fc9f6d57 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java
978c4dd0ab92afde53972f7feb9d8f018d0bf662 
>   geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java
a0c3cf3074051990cc50755131f8024db0b1faad 
>   geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java
cacbeed957c3b87d08c93db74e38e0134565f699 
>   geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java
fca7eae9413cee98d351db5349fd950d3aa56180 
>   geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java
70823443b5c4f776c86bb28ed49a73e690c5c872 
>   geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java
ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
>   geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java
PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java
PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java
89070123978c22c4cfa8684fbb5b033dc9d83ffa 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java
f027a4367b38681f83dad2d4c4add67759633644 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java
44893520962331bcd41e972afa661538c28d4fb2 
>   geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java
857c0be8940b4acde2aa4992fac0122b687391c2 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java
c551ca9104a85dcf54c0bebbc4178fce4114a416 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java
01d6bb6488e76fb3cf652ad211e9f7e2fac51389 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java
1caedbcede239d6a96640381cc6941948637b442 
>   geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java
cdb90f1b580edaf6a2762883d4159a45d69c4728 
>   geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
e90bc0a69222998322e02fcfad1b6bad3c97f4d1 
>   geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java
a9048b9219a494f61e3873ee3f2908da04bf6154 
>   geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 63f907cb007846626a9a66dc6b1ef28e0bb6db45

>   geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
767588d3717fccbb0b9c7dec7c5439e16d5381aa 
> 
> 
> Diff: https://reviews.apache.org/r/60028/diff/1/
> 
> 
> Testing
> -------
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message