fix issues from code-review

This commit is contained in:
Michael Hoennig 2024-08-19 15:48:09 +02:00
parent 0ab09f7610
commit 048a976a1a
7 changed files with 80 additions and 13 deletions

View File

@ -45,7 +45,7 @@ public class HsHostingAssetController implements HsHostingAssetsApi {
private HsHostingAssetRealRepository realAssetRepo;
@Autowired
private HsBookingItemRealRepository relBookingItemRepo;
private HsBookingItemRealRepository realBookingItemRepo;
@Override
@Transactional(readOnly = true)
@ -151,7 +151,7 @@ public class HsHostingAssetController implements HsHostingAssetsApi {
final BiConsumer<HsHostingAssetInsertResource, HsHostingAssetRbacEntity> RESOURCE_TO_ENTITY_POSTMAPPER = (resource, entity) -> {
entity.putConfig(KeyValueMap.from(resource.getConfig()));
if (resource.getBookingItemUuid() != null) {
entity.setBookingItem(relBookingItemRepo.findByUuid(resource.getBookingItemUuid())
entity.setBookingItem(realBookingItemRepo.findByUuid(resource.getBookingItemUuid())
.orElseThrow(() -> new EntityNotFoundException("ERROR: [400] bookingItemUuid %s not found".formatted(
resource.getBookingItemUuid()))));
}

View File

@ -8,7 +8,7 @@ import java.util.Optional;
import java.util.UUID;
public interface HsHostingAssetRbacRepository extends Repository<HsHostingAssetRbacEntity, UUID> {
public interface HsHostingAssetRbacRepository extends HsHostingAssetRepository<HsHostingAssetRbacEntity>, Repository<HsHostingAssetRbacEntity, UUID> {
Optional<HsHostingAssetRbacEntity> findByUuid(final UUID serverUuid);

View File

@ -7,7 +7,7 @@ import java.util.List;
import java.util.Optional;
import java.util.UUID;
public interface HsHostingAssetRealRepository extends Repository<HsHostingAssetRealEntity, UUID> {
public interface HsHostingAssetRealRepository extends HsHostingAssetRepository<HsHostingAssetRealEntity>, Repository<HsHostingAssetRealEntity, UUID> {
Optional<HsHostingAssetRealEntity> findByUuid(final UUID serverUuid);
@ -38,7 +38,7 @@ public interface HsHostingAssetRealRepository extends Repository<HsHostingAssetR
return findAllByCriteriaImpl(projectUuid, parentAssetUuid, HsHostingAssetType.asString(type));
}
HsHostingAssetRealEntity save(HsHostingAsset current);
HsHostingAssetRealEntity save(HsHostingAssetRealEntity current);
int deleteByUuid(final UUID uuid);

View File

@ -0,0 +1,24 @@
package net.hostsharing.hsadminng.hs.hosting.asset;
import java.util.List;
import java.util.Optional;
import java.util.UUID;
public interface HsHostingAssetRepository<E extends HsHostingAsset> {
Optional<E> findByUuid(final UUID serverUuid);
List<E> findByIdentifier(String assetIdentifier);
List<E> findAllByCriteriaImpl(UUID projectUuid, UUID parentAssetUuid, String type);
default List<E> findAllByCriteria(final UUID projectUuid, final UUID parentAssetUuid, final HsHostingAssetType type) {
return findAllByCriteriaImpl(projectUuid, parentAssetUuid, HsHostingAssetType.asString(type));
}
E save(HsHostingAsset current);
int deleteByUuid(final UUID uuid);
long count();
}

View File

@ -45,9 +45,8 @@ public abstract class HsEntityValidator<E extends PropertiesProvider> {
localEntityManager.set(em);
try {
return code.get();
} catch (final RuntimeException e) {
} finally {
localEntityManager.remove();
throw e;
}
}

View File

@ -12,6 +12,8 @@ import net.hostsharing.hsadminng.rbac.test.ContextBasedTestWithCleanup;
import net.hostsharing.hsadminng.rbac.test.JpaAttempt;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EnumSource;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
import org.springframework.boot.test.mock.mockito.MockBean;
@ -39,7 +41,7 @@ import static org.assertj.core.api.Assertions.assertThat;
@DataJpaTest
@Import({ Context.class, JpaAttempt.class })
class HsHostingAssetRbacRepositoryIntegrationTest extends ContextBasedTestWithCleanup {
class HsHostingAssetRepositoryIntegrationTest extends ContextBasedTestWithCleanup {
@Autowired
HsHostingAssetRealRepository realAssetRepo;
@ -203,13 +205,14 @@ class HsHostingAssetRbacRepositoryIntegrationTest extends ContextBasedTestWithCl
@Nested
class FindAssets {
@Test
public void globalAdmin_withoutAssumedRole_canViewArbitraryAssetsOfAllDebitors() {
@ParameterizedTest
@EnumSource(TestCase.class)
public void globalAdmin_withoutAssumedRole_canViewArbitraryAssetsOfAllDebitors(final TestCase testCase) {
// given
context("superuser-alex@hostsharing.net");
// when
final var result = rbacAssetRepo.findAllByCriteria(null, null, MANAGED_WEBSPACE);
final var result = repoUnderTest(testCase).findAllByCriteria(null, null, MANAGED_WEBSPACE);
// then
exactlyTheseAssetsAreReturned(
@ -451,13 +454,45 @@ class HsHostingAssetRbacRepositoryIntegrationTest extends ContextBasedTestWithCl
}
void exactlyTheseAssetsAreReturned(
final List<HsHostingAssetRbacEntity> actualResult,
final List<? extends HsHostingAsset> actualResult,
final String... serverNames) {
assertThat(actualResult)
.extracting(HsHostingAssetRbacEntity::toString)
.extracting(HsHostingAsset::toString)
.extracting(input -> input.replaceAll("\\s+", " "))
.extracting(input -> input.replaceAll("\"", ""))
.extracting(input -> input.replaceAll("\" : ", "\": "))
.containsExactlyInAnyOrder(serverNames);
}
void allTheseBookingProjectsAreReturned(
final List<? extends HsHostingAsset> actualResult,
final String... serverNames) {
assertThat(actualResult)
.extracting(HsHostingAsset::toString)
.extracting(input -> input.replaceAll("\\s+", " "))
.extracting(input -> input.replaceAll("\"", ""))
.extracting(input -> input.replaceAll("\" : ", "\": "))
.contains(serverNames);
}
private HsHostingAssetRepository<? extends HsHostingAsset> repoUnderTest(final HsHostingAssetRepositoryIntegrationTest.TestCase testCase) {
return testCase.repo(HsHostingAssetRepositoryIntegrationTest.this);
}
enum TestCase {
REAL {
@Override
HsHostingAssetRepository<HsHostingAssetRealEntity> repo(final HsHostingAssetRepositoryIntegrationTest test) {
return test.realAssetRepo;
}
},
RBAC {
@Override
HsHostingAssetRepository<HsHostingAssetRbacEntity> repo(final HsHostingAssetRepositoryIntegrationTest test) {
return test.rbacAssetRepo;
}
};
abstract HsHostingAssetRepository<? extends HsHostingAsset> repo(final HsHostingAssetRepositoryIntegrationTest test);
}
}

View File

@ -293,7 +293,10 @@ public abstract class ContextBasedTestWithCleanup extends ContextBasedTest {
/**
* @return an array of all RBAC roles matching the given pattern
*
* Usually unused, but for temporary debugging purposes of findind role names for new tests.
*/
@SuppressWarnings("unused")
protected String[] roleNames(final String sqlLikeExpression) {
final var pattern = Pattern.compile(sqlLikeExpression);
//noinspection unchecked
@ -307,7 +310,10 @@ public abstract class ContextBasedTestWithCleanup extends ContextBasedTest {
/**
* Generates a diagram of the RBAC-Grants to the current subjects (user or assumed roles).
*
* Usually unused, but for temporary use for debugging and other analysis.
*/
@SuppressWarnings("unused")
protected void generateRbacDiagramForCurrentSubjects(final EnumSet<RbacGrantsDiagramService.Include> include, final String name) {
RbacGrantsDiagramService.writeToFile(
name,
@ -318,7 +324,10 @@ public abstract class ContextBasedTestWithCleanup extends ContextBasedTest {
/**
* Generates a diagram of the RBAC-Grants for the given object and permission.
*
* Usually unused, but for temporary use for debugging and other analysis.
*/
@SuppressWarnings("unused")
protected void generateRbacDiagramForObjectPermission(final UUID targetObject, final String rbacOp, final String name) {
RbacGrantsDiagramService.writeToFile(
name,