[GitHub] [james-project] chibenwa commented on a change in pull request #557: [REFACTORING] UsersRepositoryContract

[GitBox <gi...@apache.org>]

chibenwa commented on a change in pull request #557:
URL: https://github.com/apache/james-project/pull/557#discussion_r678085389



##########
File path: server/data/data-library/src/test/java/org/apache/james/user/lib/UsersRepositoryContract.java
##########
@@ -514,29 +505,107 @@ default void removeUserShouldThrowWhenUserNotInRepository(TestSystem testSystem)
                 .isInstanceOf(UsersRepositoryException.class);
         }
 
-        @Test
-        default void isAdministratorShouldReturnFalseWhenNotConfigured(TestSystem testSystem) throws Exception {
-            testee().setAdministratorId(Optional.empty());
+    }
+
+    interface UsersRepositoryImplContract {
+        interface ReadOnlyContract {
+            UsersRepositoryImpl<?> testee();
 
-            assertThat(testee().isAdministrator(testSystem.admin)).isFalse();
+            @Test
+            default void isAdministratorShouldBeCaseInsensitive(TestSystem testSystem) throws Exception {
+                testee().setAdministratorId(Optional.of(testSystem.admin));
+                assertThat(testee().isAdministrator(testSystem.adminCaseVariation))
+                    .isTrue();
+            }
         }
 
-        @Test
-        default void isAdministratorShouldReturnTrueWhenConfiguredAndUserIsAdmin(TestSystem testSystem) throws Exception {
-            testee().setAdministratorId(Optional.of(testSystem.admin));
+        interface ReadWriteContract {
+            UsersRepositoryImpl<?> testee();
+
+            @Test
+            default void isAdministratorShouldReturnFalseWhenNotConfigured(TestSystem testSystem) throws Exception {
+                testee().setAdministratorId(Optional.empty());
+
+                assertThat(testee().isAdministrator(testSystem.admin)).isFalse();
+            }
+
+            @Test
+            default void isAdministratorShouldReturnTrueWhenConfiguredAndUserIsAdmin(TestSystem testSystem) throws Exception {
+                testee().setAdministratorId(Optional.of(testSystem.admin));
+
+                assertThat(testee().isAdministrator(testSystem.admin)).isTrue();
+            }
 
-            assertThat(testee().isAdministrator(testSystem.admin)).isTrue();
+            @Test
+            default void isAdministratorShouldReturnFalseWhenConfiguredAndUserIsNotAdmin(TestSystem testSystem) throws Exception {
+                testee().setAdministratorId(Optional.of(testSystem.admin));

Review comment:
       Let me suggest have two methods
   
   ```
   UsersRepository testee();
   
   UsersRepository testee(String administratorId);
   ```
   
   As part of the contract definition so that you never need contracts specific to `UsersRepositoryImpl`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org