From 8e2976ac1956bd616fc9185b19d5ab043de7b2cd Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Tue, 1 Oct 2024 16:00:07 +0200 Subject: [PATCH] [kbss-cvut/record-manager-ui#202] FromIri and FromName dont' throw an exception if the provided role does not exist --- .../java/cz/cvut/kbss/study/model/Role.java | 69 +++++++++++-------- .../study/service/security/SecurityUtils.java | 7 +- .../cz/cvut/kbss/study/model/RoleTest.java | 50 +++----------- 3 files changed, 55 insertions(+), 71 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/model/Role.java b/src/main/java/cz/cvut/kbss/study/model/Role.java index 988cbe26..700b7734 100644 --- a/src/main/java/cz/cvut/kbss/study/model/Role.java +++ b/src/main/java/cz/cvut/kbss/study/model/Role.java @@ -3,11 +3,12 @@ import com.fasterxml.jackson.annotation.JsonValue; import cz.cvut.kbss.jopa.model.annotations.Individual; import cz.cvut.kbss.study.security.SecurityConstants; +import java.util.Optional; public enum Role { // TODO deprecated -- should be removed. - @Individual(iri=Vocabulary.s_i_RM_ADMIN) + @Individual(iri = Vocabulary.s_i_RM_ADMIN) administrator(SecurityConstants.ROLE_ADMIN, Vocabulary.s_i_RM_ADMIN), // TODO deprecated -- should be removed. @Individual(iri = Vocabulary.s_i_RM_USER) @@ -44,7 +45,7 @@ public enum Role { rejectRecords(SecurityConstants.rejectRecords, Vocabulary.s_i_reject_records_role), @Individual(iri = Vocabulary.s_i_publish_records_role) - publishRecords(SecurityConstants.publishRecords ,Vocabulary.s_i_publish_records_role), + publishRecords(SecurityConstants.publishRecords, Vocabulary.s_i_publish_records_role), @Individual(iri = Vocabulary.s_i_import_codelists_role) importCodelists(SecurityConstants.importCodelists, Vocabulary.s_i_import_codelists_role); @@ -59,7 +60,7 @@ public enum Role { } @JsonValue - public String getRoleName(){ + public String getRoleName() { return roleName; } @@ -68,52 +69,60 @@ public String getIri() { } /** - * Returns {@link Role} with the specified IRI. + * Retrieves a role based on its IRI. * - * @param iri role identifier - * @return matching {@code Role} - * @throws IllegalArgumentException When no matching role is found + *

This method iterates over all available roles and checks if any role's IRI + * matches the provided IRI string. If a match is found, the corresponding role + * is returned as an Optional. If no match is found, an empty Optional is returned.

+ * + * @param iri the IRI of the role to retrieve + * @return an Optional containing the corresponding Role if found, + * or an empty Optional if no matching role exists */ - public static Role fromIri(String iri) { + public static Optional fromIri(String iri) { for (Role r : values()) { if (r.getIri().equals(iri)) { - return r; + return Optional.of(r); } } - throw new IllegalArgumentException("Unknown role identifier '" + iri + "'."); + return Optional.empty(); } /** - * Returns {@link Role} with the specified constant name. + * Retrieves a role based on its role name. + * + *

This method iterates over all available roles and checks if any role's + * name matches the provided name string (case-insensitive). If a match is found, + * the corresponding role is returned as an Optional. If no match is found, + * an empty Optional is returned.

* - * @param name role name - * @return matching {@code Role} - * @throws IllegalArgumentException When no matching role is found + * @param name the name of the role to retrieve + * @return an Optional containing the corresponding Role if found, + * or an empty Optional if no matching role exists */ - public static Role fromName(String name) { + public static Optional fromName(String name) { for (Role r : values()) { if (r.roleName.equalsIgnoreCase(name)) { - return r; + return Optional.of(r); } } - throw new IllegalArgumentException("Unknown role '" + name + "'."); + return Optional.empty(); } /** - * Returns a {@link Role} with the specified IRI or constant name. - *

- * This function first tries to find the enum constant by IRI. If it is not found, constant name matching is - * attempted. + * Retrieves a role based on either its IRI or role name. * - * @param identification Constant IRI or name to find match by - * @return matching {@code Role} - * @throws IllegalArgumentException When no matching role is found + *

This method first attempts to find a role using the provided identification string + * as an IRI. If no role is found, it then attempts to find a role using the + * identification string as a role name. The first successful match will be returned + * as an Optional. If no match is found, an empty Optional is returned.

+ * + * @param identification the IRI or role name of the role to retrieve + * @return an Optional containing the corresponding Role if found, + * or an empty Optional if no matching role exists */ - public static Role fromIriOrName(String identification) { - try { - return fromIri(identification); - } catch (IllegalArgumentException e) { - return fromName(identification); - } + public static Optional fromIriOrName(String identification) { + Optional role = fromIri(identification); + return role.isPresent() ? role : fromName(identification); } } diff --git a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java index 587e144b..05288653 100644 --- a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java +++ b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java @@ -99,10 +99,13 @@ private User resolveAccountFromOAuthPrincipal(Jwt principal) { throw new NotFoundException( "User with username '" + userInfo.getPreferredUsername() + "' not found in repository."); } - RoleGroup roleGroup = new RoleGroup(); user.setRoleGroup(roleGroup); - roles.forEach(r -> roleGroup.addRole(Role.fromIriOrName(r))); + roles.stream() + .map(Role::fromIriOrName) + .filter(Optional::isPresent) + .map(Optional::get) + .forEach(roleGroup::addRole); return user; } diff --git a/src/test/java/cz/cvut/kbss/study/model/RoleTest.java b/src/test/java/cz/cvut/kbss/study/model/RoleTest.java index 2346223a..46e4c901 100644 --- a/src/test/java/cz/cvut/kbss/study/model/RoleTest.java +++ b/src/test/java/cz/cvut/kbss/study/model/RoleTest.java @@ -8,65 +8,37 @@ class RoleTest { @Test void fromIriReturnsCorrectRole() { - assertEquals(Role.administrator, Role.fromIri(Vocabulary.s_i_RM_ADMIN)); - assertEquals(Role.viewAllRecords, Role.fromIri(Vocabulary.s_i_view_all_records_role)); + assertEquals(Role.administrator, Role.fromIri(Vocabulary.s_i_RM_ADMIN).get()); + assertEquals(Role.viewAllRecords, Role.fromIri(Vocabulary.s_i_view_all_records_role).get()); } - @Test - void fromIriThrowsExceptionForUnknownIri() { - String unknownIri = "unknown_iri"; - Exception exception = assertThrows(IllegalArgumentException.class, () -> { - Role.fromIri(unknownIri); - }); - assertEquals("Unknown role identifier '" + unknownIri + "'.", exception.getMessage()); - } - - @Test void fromNameReturnsCorrectRole() { - assertEquals(Role.administrator, Role.fromName(SecurityConstants.ROLE_ADMIN)); - assertEquals(Role.viewAllRecords, Role.fromName(SecurityConstants.viewAllRecords)); + assertEquals(Role.administrator, Role.fromName(SecurityConstants.ROLE_ADMIN).get()); + assertEquals(Role.viewAllRecords, Role.fromName(SecurityConstants.viewAllRecords).get()); } @Test void fromNameIsCaseInsensitive() { - assertEquals(Role.administrator, Role.fromName(SecurityConstants.ROLE_ADMIN.toLowerCase())); - assertEquals(Role.viewAllRecords, Role.fromName(SecurityConstants.viewAllRecords.toUpperCase())); - } - - @Test - void fromNameThrowsExceptionForUnknownName() { - String unknownName = "unknown_role"; - Exception exception = assertThrows(IllegalArgumentException.class, () -> { - Role.fromName(unknownName); - }); - assertEquals("Unknown role '" + unknownName + "'.", exception.getMessage()); + assertEquals(Role.administrator, Role.fromName(SecurityConstants.ROLE_ADMIN.toLowerCase()).get()); + assertEquals(Role.viewAllRecords, Role.fromName(SecurityConstants.viewAllRecords.toUpperCase()).get()); } - @Test void fromIriOrNameReturnsRoleByIri() { - assertEquals(Role.administrator, Role.fromIriOrName(Vocabulary.s_i_RM_ADMIN)); - assertEquals(Role.viewAllRecords, Role.fromIriOrName(Vocabulary.s_i_view_all_records_role)); + assertEquals(Role.administrator, Role.fromIriOrName(Vocabulary.s_i_RM_ADMIN).get()); + assertEquals(Role.viewAllRecords, Role.fromIriOrName(Vocabulary.s_i_view_all_records_role).get()); } @Test void fromIriOrNameReturnsRoleByName() { - assertEquals(Role.administrator, Role.fromIriOrName(SecurityConstants.ROLE_ADMIN)); - assertEquals(Role.viewAllRecords, Role.fromIriOrName(SecurityConstants.viewAllRecords)); + assertEquals(Role.administrator, Role.fromIriOrName(SecurityConstants.ROLE_ADMIN).get()); + assertEquals(Role.viewAllRecords, Role.fromIriOrName(SecurityConstants.viewAllRecords).get()); } @Test void fromIriOrNameIsCaseInsensitiveForName() { - assertEquals(Role.administrator, Role.fromIriOrName(SecurityConstants.ROLE_ADMIN.toLowerCase())); + assertEquals(Role.administrator, Role.fromIriOrName(SecurityConstants.ROLE_ADMIN.toLowerCase()).get()); } - @Test - void fromIriOrNameThrowsExceptionForUnknownIdentifier() { - String unknownIdentifier = "unknown_identifier"; - Exception exception = assertThrows(IllegalArgumentException.class, () -> { - Role.fromIriOrName(unknownIdentifier); - }); - assertEquals("Unknown role '" + unknownIdentifier + "'.", exception.getMessage()); - } }