lint & coverage

Author: fdaugan

src/main/java/org/ligoj/app/plugin/id/dao/IdCacheDaoImpl.java

@@ -191,6 +191,7 @@ public void delete(final GroupOrg group) {
 	}
 
 	private void removeAll(Query... queries) {
+		//noinspection unchecked
 		Stream.of(queries).map(Query::getResultList).forEach(l -> l.forEach(em::remove));
 		em.flush();
 	}
@@ -364,7 +365,7 @@ public void removeUserFromGroup(final UserOrg user, final GroupOrg group) {
 	}
 
 	private void deleteBatch(Class<?> cls, List<String> ids, Consumer<List<String>> batchConsumer) {
-		log.info("Deleting removed cache {} {} entries", cls.getSimpleName(), (Object) ids.size());
+		log.info("Deleting removed cache {} {} entries", cls.getSimpleName(), ids.size());
 		ListUtils.partition(ids, 1000).forEach(batchConsumer);
 	}
 
@@ -492,24 +493,21 @@ private long updateDelegateDn(final Map<String, ? extends CacheContainer> contai
 		final var updated = new AtomicInteger();
 		// Get all delegates of he related receiver type
 		delegateOrgRepository.findAllBy(typePath, type).stream().filter(d -> {
-			// Consider only the existing ones
-			final var dn = Optional.ofNullable(containers.get(id.apply(d))).map(CacheContainer::getDescription)
-					.orElse(null);
-
-			// Consider only the dirty one
 			final var delegateDn = getDn.apply(d);
-			if (delegateDn == null) {
-				return false;
-			}
+			// Consider only the existing ones
+			final var container = containers.get(id.apply(d));
+			final var dn = Optional.ofNullable(container).map(CacheContainer::getDescription).orElse(null);
 			if (dn == null) {
-				// Not resolved DN for this entity's id
+				// Not resolved DN for this entity's id, can be deleted
 				return true;
 			}
 			if (!delegateDn.equalsIgnoreCase(dn)) {
 				// The delegate DN needed this update
 				setDn.accept(d, dn);
 				updated.incrementAndGet();
 			}
+
+			// Keep this delegate
 			return false;
 		}).forEach(delegateOrgRepository::delete);
 		return updated.get();

src/main/java/org/ligoj/app/plugin/id/resource/UserOrgResource.java

@@ -356,19 +356,19 @@ public String create(final UserOrgEditionVo user) {
 	/**
 	 * Validate the user changes regarding the current user's right, replace group names with the exact CN, and replace
 	 * the company with a normalized one.<br>
-	 * Rules, order is important :
+	 * Rules where order is important:
 	 * <ul>
 	 * <li>At least one valid delegate must exist (valid or not against the involved user). If not, act as if the
 	 * company does not exist.</li>
 	 * <li>Involved company must exist</li>
-	 * <li>Involved company must be visible by the principal user. If not at if it does not exist, one</li>
-	 * <li>Involved company must be writable by the principal user when there is one updated attribute. Otherwise
+	 * <li>Involved company must be visible by the principal user. If not, act if it does not exist, one</li>
+	 * <li>Involved company must be writable by the principal user when there is one updated attribute. Otherwise,
 	 * indicate the read-only state.</li>
 	 * <li>Involved groups must exist</li>
 	 * <li>Involved groups must be visible by the current user, if not, act as if it does not exist. So the user can
 	 * only involve visible groups he/she. These groups are completed with the other invisible groups the user may
 	 * already have.</li>
-	 * <li>Involved changed groups must writable by the principal user. Otherwise indicate the read-only state.</li>
+	 * <li>Involved changed groups must writable by the principal user. Otherwise, indicate the read-only state.</li>
 	 * </ul>
 	 */
 	private boolean validateChanges(final String principal, final UserOrgEditionVo importEntry) {
@@ -552,7 +552,11 @@ private boolean hasAttributeChange(final SimpleUser importEntry, boolean hasChan
 	@SafeVarargs
 	private boolean hasAttributeChange(final SimpleUser user1, final SimpleUser user2, final Function<SimpleUser, String>... equals) {
 		final var predicateFalse = Arrays.stream(equals).filter(f -> !Strings.CI.equals(StringUtils.trimToNull(f.apply(user2)), StringUtils.trimToNull(f.apply(user1)))).findFirst().orElse(null);
-		return predicateFalse != null && hasAttributeChange(user1, true, String.format("'%s' != '%s'", predicateFalse.apply(user1), predicateFalse.apply(user2)));
+		if (predicateFalse == null) {
+			return false;
+		}
+		hasAttributeChange(user1, true, String.format("'%s' != '%s'", predicateFalse.apply(user1), predicateFalse.apply(user2)));
+		return true;
 	}
 
 	/**
@@ -714,7 +718,7 @@ public void lock(@PathParam("user") final String user) {
 	 * tools are watching.<br>
 	 * All memberships are updated, the user's DN is changed, all groups must be updated. Rules, order is important :
 	 * <ul>
-	 * <li>Only users managing the company of this user can perform the disable, if not, act as if the user did not
+	 * <li>Only users managing the company of this user can perform the disablement, if not, act as if the user did not
 	 * exist</li>
 	 * <li>User must exist</li>
 	 * </ul>
@@ -734,7 +738,7 @@ public void isolate(@PathParam("user") final String user) {
 	 * Unlock a user.<br>
 	 * Rules, order is important :
 	 * <ul>
-	 * <li>Only users managing the company of this user can perform the enable, if not, act as if the user did not
+	 * <li>Only users managing the company of this user can perform the enablement, if not, act as if the user did not
 	 * exist</li>
 	 * <li>User must exist</li>
 	 * </ul>
@@ -753,7 +757,7 @@ public void unlock(@PathParam("user") final String user) {
 	 * Restore a user from the isolate zone to the old company.<br>
 	 * Rules, order is important :
 	 * <ul>
-	 * <li>Only users managing the company of this user can perform the enable, if not, act as if the user did not
+	 * <li>Only users managing the company of this user can perform the enablement, if not, act as if the user did not
 	 * exist</li>
 	 * <li>User must exist</li>
 	 * </ul>
@@ -988,20 +992,14 @@ public void decorate(final SessionSettings settings) {
 
 	@Override
 	public Collection<GrantedAuthority> getGrantedAuthorities(final String username) {
-		try {
-			// Check if the user lock status without using cache
-			final var rawUserOrg = getUserRepository().toUser(Normalizer.normalize(username));
-			final var roles = new ArrayList<GrantedAuthority>();
-			for (String group : CollectionUtils.emptyIfNull(rawUserOrg.getGroups())) {
-				roles.add(new SimpleGrantedAuthority(group.toUpperCase()));
-				roles.add(new SimpleGrantedAuthority(group.toLowerCase()));
-				roles.add(new SimpleGrantedAuthority(group));
-			}
-			return roles;
-		} catch (ValidationJsonException ve) {
-			// Ignore this error
-			log.debug("User being authenticated '{}' is not defined in primary identity provider ", username);
-			return Collections.emptyList();
+		// Check if the user lock status without using cache
+		final var rawUserOrg = getUserRepository().toUser(Normalizer.normalize(username));
+		final var roles = new HashSet<GrantedAuthority>();
+		for (String group : CollectionUtils.emptyIfNull(rawUserOrg.getGroups())) {
+			roles.add(new SimpleGrantedAuthority(group.toUpperCase()));
+			roles.add(new SimpleGrantedAuthority(group.toLowerCase()));
+			roles.add(new SimpleGrantedAuthority(group));
 		}
+		return roles;
 	}
 }

src/test/java/org/ligoj/app/plugin/id/dao/IdCacheDaoTest.java

@@ -511,6 +511,7 @@ void reset() {
 		up2dateDelegate.setReceiverDn("dn=group1");
 		up2dateDelegate.setReceiverType(ReceiverType.GROUP);
 		delegateOrgRepository.saveAndFlush(up2dateDelegate);
+
 		em.flush();
 
 		// Pre state
@@ -604,4 +605,9 @@ void updateUser() {
 		Assertions.assertNull(memberships.getFirst().getSubGroup());
 		Assertions.assertEquals("u0", memberships.getFirst().getUser().getId());
 	}
+
+	@Test
+	void test() {
+		Assertions.assertEquals(0, new IdCacheDaoImpl().getCacheRefreshTime());
+	}
 }

src/test/java/org/ligoj/app/plugin/id/resource/ContainerScopeResourceTest.java

@@ -3,13 +3,9 @@
  */
 package org.ligoj.app.plugin.id.resource;
 
-import java.io.IOException;
-import java.nio.charset.StandardCharsets;
-import java.util.List;
-
+import jakarta.persistence.EntityNotFoundException;
 import jakarta.transaction.Transactional;
 import jakarta.ws.rs.core.UriInfo;
-
 import org.apache.cxf.jaxrs.impl.MetadataMap;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
@@ -29,6 +25,10 @@
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
 
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+
 /**
  * Test class of {@link ContainerScopeResource}
  */
@@ -46,7 +46,7 @@ class ContainerScopeResourceTest extends AbstractJpaTest {
 
 	@BeforeEach
 	void setUpEntities() throws IOException {
-		persistEntities("csv", new Class<?>[] { ContainerScope.class }, StandardCharsets.UTF_8);
+		persistEntities("csv", new Class<?>[]{ContainerScope.class}, StandardCharsets.UTF_8);
 	}
 
 	@Test
@@ -134,6 +134,11 @@ void findByName() {
 		checkType(resource.findByName("Project"));
 	}
 
+	@Test
+	void findByNameNotExist() {
+		Assertions.assertThrows(EntityNotFoundException.class, () -> resource.findByName(ContainerType.GROUP, "Other"));
+	}
+
 	private void checkType(final ContainerScope type) {
 		Assertions.assertEquals("Project", type.getName());
 		Assertions.assertTrue(type.isLocked());

src/test/java/org/ligoj/app/plugin/id/resource/UserOrgResourceTest.java

@@ -35,6 +35,7 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.ApplicationContext;
 import org.springframework.data.domain.PageImpl;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.test.annotation.Rollback;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
@@ -1235,4 +1236,16 @@ public UserOrg findById(@PathParam("user") final String user) {
 		});
 	}
 
+
+	@Test
+	void getGrantedAuthorities() {
+		final UserOrg userOrg = new UserOrg();
+		userOrg.setGroups(List.of("Group1", "Group2"));
+		Mockito.when(userRepository.toUser("junit")).thenReturn(userOrg);
+		var authorities = resource.getGrantedAuthorities("junit");
+		Assertions.assertTrue( authorities.contains(new SimpleGrantedAuthority("Group1")));
+		Assertions.assertTrue( authorities.contains(new SimpleGrantedAuthority("group1")));
+		Assertions.assertTrue( authorities.contains(new SimpleGrantedAuthority("GROUP1")));
+	}
+
 }