Skip to content

Commit

Permalink
Replace try/catch with AssertJ
Browse files Browse the repository at this point in the history 
Replace manual try/catch/fail blocks with AssertJ calls.
  • Loading branch information
@philwebb @jzheaux
philwebb authored and jzheaux committed Sep 22, 2020
1 parent d9276ed commit 910b819
Show file tree
Hide file tree
Showing 98 changed files with 717 additions and 2,122 deletions.
76 changes: 16 additions & 60 deletions acl/src/test/java/org/springframework/security/acls/AclFormattingUtilsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
import org.springframework.security.acls.model.Permission;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatNoException;

/**
* Tests for {@link AclFormattingUtils}.
Expand All @@ -33,30 +34,11 @@ public class AclFormattingUtilsTests {

@Test
public final void testDemergePatternsParametersConstraints() {
try {
AclFormattingUtils.demergePatterns(null, "SOME STRING");
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
try {
AclFormattingUtils.demergePatterns("SOME STRING", null);
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
try {
AclFormattingUtils.demergePatterns("SOME STRING", "LONGER SOME STRING");
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
try {
AclFormattingUtils.demergePatterns("SOME STRING", "SAME LENGTH");
}
catch (IllegalArgumentException notExpected) {
fail("It shouldn't have thrown IllegalArgumentException");
}
assertThatIllegalArgumentException().isThrownBy(() -> AclFormattingUtils.demergePatterns(null, "SOME STRING"));
assertThatIllegalArgumentException().isThrownBy(() -> AclFormattingUtils.demergePatterns("SOME STRING", null));
assertThatIllegalArgumentException()
.isThrownBy(() -> AclFormattingUtils.demergePatterns("SOME STRING", "LONGER SOME STRING"));
assertThatNoException().isThrownBy(() -> AclFormattingUtils.demergePatterns("SOME STRING", "SAME LENGTH"));
}

@Test
Expand All @@ -71,29 +53,11 @@ public final void testDemergePatterns() {

@Test
public final void testMergePatternsParametersConstraints() {
try {
AclFormattingUtils.mergePatterns(null, "SOME STRING");
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
try {
AclFormattingUtils.mergePatterns("SOME STRING", null);
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
try {
AclFormattingUtils.mergePatterns("SOME STRING", "LONGER SOME STRING");
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
try {
AclFormattingUtils.mergePatterns("SOME STRING", "SAME LENGTH");
}
catch (IllegalArgumentException notExpected) {
}
assertThatIllegalArgumentException().isThrownBy(() -> AclFormattingUtils.mergePatterns(null, "SOME STRING"));
assertThatIllegalArgumentException().isThrownBy(() -> AclFormattingUtils.mergePatterns("SOME STRING", null));
assertThatIllegalArgumentException()
.isThrownBy(() -> AclFormattingUtils.mergePatterns("SOME STRING", "LONGER SOME STRING"));
assertThatNoException().isThrownBy(() -> AclFormattingUtils.mergePatterns("SOME STRING", "SAME LENGTH"));
}

@Test
Expand All @@ -108,18 +72,10 @@ public final void testMergePatterns() {
@Test
public final void testBinaryPrints() {
assertThat(AclFormattingUtils.printBinary(15)).isEqualTo("............................****");
try {
AclFormattingUtils.printBinary(15, Permission.RESERVED_ON);
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException notExpected) {
}
try {
AclFormattingUtils.printBinary(15, Permission.RESERVED_OFF);
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException notExpected) {
}
assertThatIllegalArgumentException()
.isThrownBy(() -> AclFormattingUtils.printBinary(15, Permission.RESERVED_ON));
assertThatIllegalArgumentException()
.isThrownBy(() -> AclFormattingUtils.printBinary(15, Permission.RESERVED_OFF));
assertThat(AclFormattingUtils.printBinary(15, 'x')).isEqualTo("............................xxxx");
}

Expand Down
33 changes: 14 additions & 19 deletions ...g/springframework/security/acls/afterinvocation/AclEntryAfterInvocationProviderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@
import org.springframework.security.core.SpringSecurityMessageSource;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.BDDMockito.given;
Expand All @@ -50,15 +51,12 @@
@SuppressWarnings({ "unchecked" })
public class AclEntryAfterInvocationProviderTests {

@Test(expected = IllegalArgumentException.class)
@Test
public void rejectsMissingPermissions() {
try {
new AclEntryAfterInvocationProvider(mock(AclService.class), null);
fail("Exception expected");
}
catch (IllegalArgumentException expected) {
}
new AclEntryAfterInvocationProvider(mock(AclService.class), Collections.<Permission>emptyList());
assertThatIllegalArgumentException()
.isThrownBy(() -> new AclEntryAfterInvocationProvider(mock(AclService.class), null));
assertThatIllegalArgumentException().isThrownBy(
() -> new AclEntryAfterInvocationProvider(mock(AclService.class), Collections.<Permission>emptyList()));
}

@Test
Expand Down Expand Up @@ -98,7 +96,7 @@ public void accessIsGrantedIfObjectTypeNotSupported() {
SecurityConfig.createList("AFTER_ACL_READ"), returned));
}

@Test(expected = AccessDeniedException.class)
@Test
public void accessIsDeniedIfPermissionIsNotGranted() {
AclService service = mock(AclService.class);
Acl acl = mock(Acl.class);
Expand All @@ -113,16 +111,13 @@ public void accessIsDeniedIfPermissionIsNotGranted() {
provider.setObjectIdentityRetrievalStrategy(mock(ObjectIdentityRetrievalStrategy.class));
provider.setProcessDomainObjectClass(Object.class);
provider.setSidRetrievalStrategy(mock(SidRetrievalStrategy.class));
try {
provider.decide(mock(Authentication.class), new Object(),
SecurityConfig.createList("UNSUPPORTED", "MY_ATTRIBUTE"), new Object());
fail("Expected Exception");
}
catch (AccessDeniedException expected) {
}
assertThatExceptionOfType(AccessDeniedException.class)
.isThrownBy(() -> provider.decide(mock(Authentication.class), new Object(),
SecurityConfig.createList("UNSUPPORTED", "MY_ATTRIBUTE"), new Object()));
// Second scenario with no acls found
provider.decide(mock(Authentication.class), new Object(),
SecurityConfig.createList("UNSUPPORTED", "MY_ATTRIBUTE"), new Object());
assertThatExceptionOfType(AccessDeniedException.class)
.isThrownBy(() -> provider.decide(mock(Authentication.class), new Object(),
SecurityConfig.createList("UNSUPPORTED", "MY_ATTRIBUTE"), new Object()));
}

@Test
Expand Down
27 changes: 7 additions & 20 deletions acl/src/test/java/org/springframework/security/acls/domain/AccessControlImplEntryTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import org.springframework.security.acls.model.Sid;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;

Expand All @@ -39,27 +39,14 @@ public class AccessControlImplEntryTests {
@Test
public void testConstructorRequiredFields() {
// Check Acl field is present
try {
new AccessControlEntryImpl(null, null, new PrincipalSid("johndoe"), BasePermission.ADMINISTRATION, true,
true, true);
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
assertThatIllegalArgumentException().isThrownBy(() -> new AccessControlEntryImpl(null, null,
new PrincipalSid("johndoe"), BasePermission.ADMINISTRATION, true, true, true));
// Check Sid field is present
try {
new AccessControlEntryImpl(null, mock(Acl.class), null, BasePermission.ADMINISTRATION, true, true, true);
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
assertThatIllegalArgumentException().isThrownBy(() -> new AccessControlEntryImpl(null, mock(Acl.class), null,
BasePermission.ADMINISTRATION, true, true, true));
// Check Permission field is present
try {
new AccessControlEntryImpl(null, mock(Acl.class), new PrincipalSid("johndoe"), null, true, true, true);
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
assertThatIllegalArgumentException().isThrownBy(() -> new AccessControlEntryImpl(null, mock(Acl.class),
new PrincipalSid("johndoe"), null, true, true, true));
}

@Test
Expand Down
117 changes: 33 additions & 84 deletions acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
import org.springframework.security.util.FieldUtils;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.mockito.Mockito.mock;

/**
Expand Down Expand Up @@ -97,58 +98,38 @@ public void tearDown() {
SecurityContextHolder.clearContext();
}

@Test(expected = IllegalArgumentException.class)
@Test
public void constructorsRejectNullObjectIdentity() {
try {
new AclImpl(null, 1, this.authzStrategy, this.pgs, null, null, true, new PrincipalSid("joe"));
fail("Should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
new AclImpl(null, 1, this.authzStrategy, this.mockAuditLogger);
assertThatIllegalArgumentException().isThrownBy(
() -> new AclImpl(null, 1, this.authzStrategy, this.pgs, null, null, true, new PrincipalSid("joe")));
assertThatIllegalArgumentException()
.isThrownBy(() -> new AclImpl(null, 1, this.authzStrategy, this.mockAuditLogger));
}

@Test(expected = IllegalArgumentException.class)
@Test
public void constructorsRejectNullId() {
try {
new AclImpl(this.objectIdentity, null, this.authzStrategy, this.pgs, null, null, true,
new PrincipalSid("joe"));
fail("Should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
new AclImpl(this.objectIdentity, null, this.authzStrategy, this.mockAuditLogger);
assertThatIllegalArgumentException().isThrownBy(() -> new AclImpl(this.objectIdentity, null, this.authzStrategy,
this.pgs, null, null, true, new PrincipalSid("joe")));
assertThatIllegalArgumentException()
.isThrownBy(() -> new AclImpl(this.objectIdentity, null, this.authzStrategy, this.mockAuditLogger));
}

@SuppressWarnings("deprecation")
@Test(expected = IllegalArgumentException.class)
@Test
public void constructorsRejectNullAclAuthzStrategy() {
try {
new AclImpl(this.objectIdentity, 1, null, new DefaultPermissionGrantingStrategy(this.mockAuditLogger), null,
null, true, new PrincipalSid("joe"));
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
new AclImpl(this.objectIdentity, 1, null, this.mockAuditLogger);
assertThatIllegalArgumentException().isThrownBy(() -> new AclImpl(this.objectIdentity, 1, null,
new DefaultPermissionGrantingStrategy(this.mockAuditLogger), null, null, true,
new PrincipalSid("joe")));
assertThatIllegalArgumentException()
.isThrownBy(() -> new AclImpl(this.objectIdentity, 1, null, this.mockAuditLogger));
}

@Test
public void insertAceRejectsNullParameters() {
MutableAcl acl = new AclImpl(this.objectIdentity, 1, this.authzStrategy, this.pgs, null, null, true,
new PrincipalSid("joe"));
try {
acl.insertAce(0, null, new GrantedAuthoritySid("ROLE_IGNORED"), true);
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
try {
acl.insertAce(0, BasePermission.READ, null, true);
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
assertThatIllegalArgumentException()
.isThrownBy(() -> acl.insertAce(0, null, new GrantedAuthoritySid("ROLE_IGNORED"), true));
assertThatIllegalArgumentException().isThrownBy(() -> acl.insertAce(0, BasePermission.READ, null, true));
}

@Test
Expand Down Expand Up @@ -232,31 +213,17 @@ public void deleteAceFailsForNonExistentElement() {
new SimpleGrantedAuthority("ROLE_GENERAL"));
MutableAcl acl = new AclImpl(this.objectIdentity, (1), strategy, this.pgs, null, null, true,
new PrincipalSid("joe"));
try {
acl.deleteAce(99);
fail("It should have thrown NotFoundException");
}
catch (NotFoundException expected) {
}
assertThatExceptionOfType(NotFoundException.class).isThrownBy(() -> acl.deleteAce(99));
}

@Test
public void isGrantingRejectsEmptyParameters() {
MutableAcl acl = new AclImpl(this.objectIdentity, 1, this.authzStrategy, this.pgs, null, null, true,
new PrincipalSid("joe"));
Sid ben = new PrincipalSid("ben");
try {
acl.isGranted(new ArrayList<>(0), Arrays.asList(ben), false);
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
try {
acl.isGranted(READ, new ArrayList<>(0), false);
fail("It should have thrown IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
}
assertThatIllegalArgumentException()
.isThrownBy(() -> acl.isGranted(new ArrayList<>(0), Arrays.asList(ben), false));
assertThatIllegalArgumentException().isThrownBy(() -> acl.isGranted(READ, new ArrayList<>(0), false));
}

@Test
Expand All @@ -277,25 +244,16 @@ public void isGrantingGrantsAccessForAclWithNoParent() {
List<Permission> permissions = Arrays.asList(BasePermission.READ, BasePermission.CREATE);
List<Sid> sids = Arrays.asList(new PrincipalSid("ben"), new GrantedAuthoritySid("ROLE_GUEST"));
assertThat(rootAcl.isGranted(permissions, sids, false)).isFalse();
try {
rootAcl.isGranted(permissions, SCOTT, false);
fail("It should have thrown NotFoundException");
}
catch (NotFoundException expected) {
}
assertThatExceptionOfType(NotFoundException.class)
.isThrownBy(() -> rootAcl.isGranted(permissions, SCOTT, false));
assertThat(rootAcl.isGranted(WRITE, SCOTT, false)).isTrue();
assertThat(rootAcl.isGranted(WRITE,
Arrays.asList(new PrincipalSid("rod"), new GrantedAuthoritySid("WRITE_ACCESS_ROLE")), false)).isFalse();
assertThat(rootAcl.isGranted(WRITE,
Arrays.asList(new GrantedAuthoritySid("WRITE_ACCESS_ROLE"), new PrincipalSid("rod")), false)).isTrue();
try {
// Change the type of the Sid and check the granting process
rootAcl.isGranted(WRITE,
Arrays.asList(new GrantedAuthoritySid("rod"), new PrincipalSid("WRITE_ACCESS_ROLE")), false);
fail("It should have thrown NotFoundException");
}
catch (NotFoundException expected) {
}
// Change the type of the Sid and check the granting process
assertThatExceptionOfType(NotFoundException.class).isThrownBy(() -> rootAcl.isGranted(WRITE,
Arrays.asList(new GrantedAuthoritySid("rod"), new PrincipalSid("WRITE_ACCESS_ROLE")), false));
}

@Test
Expand Down Expand Up @@ -348,18 +306,9 @@ public void isGrantingGrantsAccessForInheritableAcls() {
assertThat(childAcl1.isGranted(DELETE, BEN, false)).isFalse();
// Check granting process for child2 (doesn't inherit the permissions from its
// parent)
try {
assertThat(childAcl2.isGranted(CREATE, SCOTT, false)).isTrue();
fail("It should have thrown NotFoundException");
}
catch (NotFoundException expected) {
}
try {
childAcl2.isGranted(CREATE, Arrays.asList((Sid) new PrincipalSid("joe")), false);
fail("It should have thrown NotFoundException");
}
catch (NotFoundException expected) {
}
assertThatExceptionOfType(NotFoundException.class).isThrownBy(() -> childAcl2.isGranted(CREATE, SCOTT, false));
assertThatExceptionOfType(NotFoundException.class)
.isThrownBy(() -> childAcl2.isGranted(CREATE, Arrays.asList((Sid) new PrincipalSid("joe")), false));
}

@Test
Expand Down
Loading

0 comments on commit 910b819

Please sign in to comment.