fix HsHostingAssetRepository.findAllByCriteriaImpl query #69

Merged
hsh-michaelhoennig merged 1 commits from fix-find-hosting-assets-query into master 2024-07-03 10:36:30 +02:00
4 changed files with 28 additions and 32 deletions

View File

@ -14,12 +14,26 @@ public interface HsHostingAssetRepository extends Repository<HsHostingAssetEntit
List<HsHostingAssetEntity> findByIdentifier(String assetIdentifier); List<HsHostingAssetEntity> findByIdentifier(String assetIdentifier);
@Query(""" @Query(value = """
SELECT asset FROM HsHostingAssetEntity asset select ha.uuid,
WHERE (:projectUuid IS NULL OR asset.bookingItem.project.uuid = :projectUuid) ha.alarmcontactuuid,

This JPQL query did only work for hosting-assets which have a directly-assigned booking-item because "left" was missing for the joins.

When I tried to run a test with EMAIL_ALIAS, it returned no records at all.

This JPQL query did only work for hosting-assets which have a directly-assigned booking-item because "left" was missing for the joins. When I tried to run a test with EMAIL_ALIAS, it returned no records at all.
AND (:parentAssetUuid IS NULL OR asset.parentAsset.uuid = :parentAssetUuid) ha.assignedtoassetuuid,
AND (:type IS NULL OR :type = CAST(asset.type AS String)) ha.bookingitemuuid,
""") ha.caption,
ha.config,
ha.identifier,
ha.parentassetuuid,
ha.type,
ha.version
from hs_hosting_asset_rv ha
left join hs_booking_item bi on bi.uuid = ha.bookingitemuuid
left join hs_hosting_asset pha on pha.uuid = ha.parentassetuuid
where (:projectUuid is null or bi.projectuuid=:projectUuid)
and (:parentAssetUuid is null or pha.uuid=:parentAssetUuid)
and (:type is null or :type=cast(ha.type as text))
""", nativeQuery = true)
// The JPQL query did not generate "left join" but just "join".
// I also optimized the query by not using the _rv for hs_booking_item and hs_hosting_asset, only for hs_hosting_asset_rv.
List<HsHostingAssetEntity> findAllByCriteriaImpl(UUID projectUuid, UUID parentAssetUuid, String type); List<HsHostingAssetEntity> findAllByCriteriaImpl(UUID projectUuid, UUID parentAssetUuid, String type);
default List<HsHostingAssetEntity> findAllByCriteria(final UUID projectUuid, final UUID parentAssetUuid, final HsHostingAssetType type) { default List<HsHostingAssetEntity> findAllByCriteria(final UUID projectUuid, final UUID parentAssetUuid, final HsHostingAssetType type) {
return findAllByCriteriaImpl(projectUuid, parentAssetUuid, HsHostingAssetType.asString(type)); return findAllByCriteriaImpl(projectUuid, parentAssetUuid, HsHostingAssetType.asString(type));

View File

@ -7,13 +7,13 @@ get:
parameters: parameters:
- $ref: 'auth.yaml#/components/parameters/currentUser' - $ref: 'auth.yaml#/components/parameters/currentUser'
- $ref: 'auth.yaml#/components/parameters/assumedRoles' - $ref: 'auth.yaml#/components/parameters/assumedRoles'
- name: debitorUuid - name: projectUuid

This was a remnant from times before we had a project between debitor and asset. project makes more sense and the actual query was only implemented for project anyway.

This was a remnant from times before we had a project between debitor and asset. project makes more sense and the actual query was only implemented for project anyway.
in: query in: query
required: false required: false
schema: schema:
type: string type: string
format: uuid format: uuid
description: The UUID of the debitor, whose hosting assets are to be listed. description: The UUID of the project, whose hosting assets are to be listed.
- name: parentAssetUuid - name: parentAssetUuid
in: query in: query
required: false required: false

View File

@ -89,22 +89,10 @@ class HsHostingAssetControllerAcceptanceTest extends ContextBasedTestWithCleanup
.contentType("application/json") .contentType("application/json")
.body("", lenientlyEquals(""" .body("", lenientlyEquals("""
[ [
{

because of the mismatch of debitor+project, ?projectUuid=" + givenProject.getUuid() was ignored and too many rows were returned, now, where this is fixed, only the rows for "D-1000111 default project" get returned

because of the mismatch of debitor+project, `?projectUuid=" + givenProject.getUuid() ` was ignored and too many rows were returned, now, where this is fixed, only the rows for "D-1000111 default project" get returned
"type": "MANAGED_WEBSPACE",
"identifier": "sec01",
"caption": "some Webspace",
"config": {}
},
{ {
"type": "MANAGED_WEBSPACE", "type": "MANAGED_WEBSPACE",
"identifier": "fir01", "identifier": "fir01",
"caption": "some Webspace", "caption": "some Webspace",
"config": {}
},
{
"type": "MANAGED_WEBSPACE",
"identifier": "thi01",
"caption": "some Webspace",
"config": {} "config": {}
} }
] ]

View File

@ -174,7 +174,7 @@ class HsHostingAssetRepositoryIntegrationTest extends ContextBasedTestWithCleanu
final var result = assetRepo.findAllByCriteria(null, null, MANAGED_WEBSPACE); final var result = assetRepo.findAllByCriteria(null, null, MANAGED_WEBSPACE);
// then // then
allTheseServersAreReturned( exactlyTheseAssetsAreReturned(
result, result,
"HsHostingAssetEntity(MANAGED_WEBSPACE, sec01, some Webspace, MANAGED_SERVER:vm1012, D-1000212:D-1000212 default project:separate ManagedWebspace)", "HsHostingAssetEntity(MANAGED_WEBSPACE, sec01, some Webspace, MANAGED_SERVER:vm1012, D-1000212:D-1000212 default project:separate ManagedWebspace)",
"HsHostingAssetEntity(MANAGED_WEBSPACE, fir01, some Webspace, MANAGED_SERVER:vm1011, D-1000111:D-1000111 default project:separate ManagedWebspace)", "HsHostingAssetEntity(MANAGED_WEBSPACE, fir01, some Webspace, MANAGED_SERVER:vm1011, D-1000111:D-1000111 default project:separate ManagedWebspace)",
@ -202,18 +202,19 @@ class HsHostingAssetRepositoryIntegrationTest extends ContextBasedTestWithCleanu
public void normalUser_canFilterAssetsRelatedToParentAsset() { public void normalUser_canFilterAssetsRelatedToParentAsset() {
// given // given
context("superuser-alex@hostsharing.net"); context("superuser-alex@hostsharing.net");
final var parentAssetUuid = assetRepo.findAllByCriteria(null, null, MANAGED_SERVER).stream() final var parentAssetUuid = assetRepo.findByIdentifier("vm1012").stream()
.filter(ha -> ha.getType() == MANAGED_SERVER)
.findAny().orElseThrow().getUuid(); .findAny().orElseThrow().getUuid();
// when // when
context("superuser-alex@hostsharing.net", "hs_hosting_asset#vm1012:AGENT");

the test name says "normalUser ..." but the superuser context without assumed role was still open, now assuming a normal users role

the test name says "normalUser ..." but the superuser context without assumed role was still open, now assuming a normal users role
final var result = assetRepo.findAllByCriteria(null, parentAssetUuid, null); final var result = assetRepo.findAllByCriteria(null, parentAssetUuid, null);
// then // then
allTheseServersAreReturned( exactlyTheseAssetsAreReturned(
result, result,
"HsHostingAssetEntity(MANAGED_WEBSPACE, sec01, some Webspace, MANAGED_SERVER:vm1012, D-1000212:D-1000212 default project:separate ManagedWebspace)"); "HsHostingAssetEntity(MANAGED_WEBSPACE, sec01, some Webspace, MANAGED_SERVER:vm1012, D-1000212:D-1000212 default project:separate ManagedWebspace)");
} }
} }
@Nested @Nested
@ -411,11 +412,4 @@ class HsHostingAssetRepositoryIntegrationTest extends ContextBasedTestWithCleanu
.extracting(input -> input.replaceAll("\"", "")) .extracting(input -> input.replaceAll("\"", ""))
.containsExactlyInAnyOrder(serverNames); .containsExactlyInAnyOrder(serverNames);
} }
void allTheseServersAreReturned(final List<HsHostingAssetEntity> actualResult, final String... serverNames) {

it's always better to test for exact returns then just for "contains", amended all calls, thus this methods is not needed anymore

it's always better to test for exact returns then just for "contains", amended all calls, thus this methods is not needed anymore
assertThat(actualResult)
.extracting(HsHostingAssetEntity::toString)
.contains(serverNames);
actualResult.forEach(loadedEntity -> assertThat(loadedEntity.isLoaded()).isTrue());
}
} }