Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Empty criteria list in AssetSelectorExpression results in valid data offers #2009

Closed
juliapampus opened this issue Sep 23, 2022 · 2 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@juliapampus
Copy link
Contributor

juliapampus commented Sep 23, 2022

Bug Report

Describe the Bug

Potential misbehavior: When a ContractDefinition is created with an accessPolicyId, contractPolicyId and an empty Criterion list, this ContractDefinition is applied to all Assets and results in a Catalog with ContractOffers that each link the Policy with the contractPolicyId and every single Asset.

Example:

  • Create 1 Policy
  • Create 1 ContractDefinition (accessPolicyId, contractPolicyId, empty Criterion list)
  • 50 Assets

Expected Behavior

Catalog with 0 ContractOffers as the ContractDefinition is invalid and does not point to a single Asset.

Observed Behavior

Catalog with 50 ContractOffers, each Asset linking to the same Policy from the ContractDefinition.

As a result: 50 ContractOffers (empty Criterion list) and 50 registered Assets result in 50x50 ContractOffers in the Catalog.

Possible Implementation

The method queryAssets(AssetSelectorExpression expression) in AssetIndex implementations currently only checks for null objects and the AssetSelectorExpression.SELECT_ALL expression. Filtering should be aborted when the criteria list is empty:

@Override
public Stream<Asset> queryAssets(AssetSelectorExpression expression) {
    Objects.requireNonNull(expression, "AssetSelectorExpression can not be null!");

    // select everything ONLY if the special constant is used
    if (expression == AssetSelectorExpression.SELECT_ALL) {
        return queryAssets(QuerySpec.none());
    }

    // don't continue if no criteria exist
    var criteria = expression.getCriteria();
    if (criteria == null || criteria.isEmpty()) {
        return Stream.empty();
    }

    return queryAssets(QuerySpec.Builder.newInstance().filter(expression.getCriteria()).build());
}

InMemoryAssetIndexTest:

@Test
void queryAssets_missingCriteriaList_returnsEmpty() {
    var testAsset = createAsset("foobar");
    index.accept(testAsset, createDataAddress(testAsset));

    var assets = index.queryAssets(AssetSelectorExpression.Builder.newInstance().build());

    assertThat(assets).isEmpty();
}

@Test
void queryAssets_emptyCriteriaList_returnsEmpty() {
    var testAsset = createAsset("foobar");
    index.accept(testAsset, createDataAddress(testAsset));

    var assets = index.queryAssets(AssetSelectorExpression.Builder.newInstance().criteria(List.of()).build());

    assertThat(assets).isEmpty();
}
@juliapampus juliapampus added the bug Something isn't working label Sep 23, 2022
@juliapampus juliapampus changed the title Potential Bug: Empty criteria list in ContractDefinition results in valid data offers Bug: Empty criteria list in AssetSelectorExpression results in valid data offers Sep 26, 2022
@juliapampus juliapampus self-assigned this Sep 27, 2022
@juliapampus juliapampus added this to the Milestone 7 milestone Sep 27, 2022
@juliapampus juliapampus modified the milestones: Milestone 7, Milestone 8 Nov 1, 2022
@juliapampus juliapampus modified the milestones: Milestone 8, Milestone 9 Jan 17, 2023
@juliapampus juliapampus modified the milestones: Milestone 9, Backlog May 4, 2023
@github-actions
Copy link

This issue is stale because it has been open for 28 days with no activity.

@github-actions github-actions bot added the stale Open for x days with no activity label Jun 24, 2023
@juliapampus juliapampus removed the stale Open for x days with no activity label Jun 26, 2023
@ndr-brt
Copy link
Member

ndr-brt commented Jun 28, 2023

Is this still valid? empty criteria list stands for "select all", if we want to avoid this behavior for any reason a validation could be added, but I'd say that this can be closed

@juliapampus juliapampus closed this as not planned Won't fix, can't repro, duplicate, stale Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment