----------------------------------------------------------- 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 > >
