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

[Fix] 14 Add Catalog Metadata Returns No Error #152

Conversation

Conundraah
Copy link
Contributor

  • changed return type of SetMetadataAsync from Task to Task<ActionResult>

@Conundraah
Copy link
Contributor Author

Interestingly the method appears to not ProtectCatalogAsync even enter the lambda-function when SetMetadataAsync's return type is Task.

@Apollo3zehn
Copy link
Member

Apollo3zehn commented Oct 2, 2024

I think the following code block has multiple issues (lines 494 - 505):

var response = ProtectCatalogAsync<object>(catalogId, ensureReadable: true, ensureWritable: false, async catalogContainer =>
{
    var canEdit = AuthUtilities.IsCatalogWritable(catalogId, catalogContainer.Metadata, User);

    if (!canEdit)
        return StatusCode(StatusCodes.Status403Forbidden, $"The current user is not permitted to modify the catalog {catalogId}.");

    await catalogContainer.UpdateMetadataAsync(metadata);

    return new object();

}, cancellationToken);
  1. ensureReadable should be false
  2. ensureWritable should be true
  3. The subsequent lines containing canEdit can be removed because ensureWritable is now true

I think I caused this mess when I introduced the ensureReadable and ensureWritable parameters without properly updating this method

@Apollo3zehn
Copy link
Member

The rest looks fine :-)

@Apollo3zehn Apollo3zehn force-pushed the 14-add-catalog-metadata-returns-no-error-because-of-wrong-return-type-in-api branch from fe4bfba to 34d4b5c Compare November 14, 2024 18:51
@Apollo3zehn
Copy link
Member

I fixed the mentioned issues above (because they are unrelated to this issue) and rebased your changes to new newest commit. I am now running on the new .NET 9 release (its much faster now to compile again :-)) and I cannot reproduce the original issue. So if you cannot reproduce it as well, we might just close this P/R.

@Apollo3zehn Apollo3zehn force-pushed the 14-add-catalog-metadata-returns-no-error-because-of-wrong-return-type-in-api branch from 34d4b5c to 833334b Compare November 14, 2024 19:03
@Apollo3zehn Apollo3zehn force-pushed the 14-add-catalog-metadata-returns-no-error-because-of-wrong-return-type-in-api branch from 833334b to 65a9f5d Compare November 14, 2024 19:26
@Apollo3zehn
Copy link
Member

Apollo3zehn commented Nov 14, 2024

Sorry for the chaos, I changed the method implementation to ensure the catalog ID is correct. During that change I had the need to change the return type to Task<ActionResult<object>> which is exactly what you tried to to in this P/R. Because no changes were left now, GitHub automatically closed the P/R.

@Apollo3zehn Apollo3zehn deleted the 14-add-catalog-metadata-returns-no-error-because-of-wrong-return-type-in-api branch November 14, 2024 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add catalog metadata returns no error (because of wrong return type in API)
2 participants