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]: BlobClient.PutBlockList() unusable #615

Closed
1 task done
Drakonian opened this issue Feb 22, 2024 · 3 comments · Fixed by #664
Closed
1 task done

[Bug]: BlobClient.PutBlockList() unusable #615

Drakonian opened this issue Feb 22, 2024 · 3 comments · Fixed by #664
Assignees
Labels
Approved The issue is approved Bug Something isn't working

Comments

@Drakonian
Copy link
Contributor

Describe the issue

While working with Azure Blob Services API module I found some inconsistencies and after analyzing it I want to share it.

The point is that PutBlockList cannot be used now.

procedure PutBlockList(CommitedBlocks: Dictionary of [Text, Integer]; UncommitedBlocks: Dictionary of [Text, Integer]): Codeunit "ABS Operation Response"

According to the documentation:
image

But at the same time PutBlock operation is not represented in the ABSBlobClient codeunit. Which makes it impossible to use the PutBlockList() method.

What is interesting is that in the implementation this method already exists and all we have to do is add it to the ABSBlobClient interface.

procedure PutBlock(SourceContentVariant: Variant; BlockId: Text): Codeunit "ABS Operation Response"

In addition, I think that the PutBlockList() method lacks the BlobName parameter because it is not clear at what point this name is implicitly received by ABSOperationPayload.

ABSOperationResponse := ABSWebRequestHelper.PutOperation(ABSOperationPayload, HttpContent, StrSubstNo(PutBlockOperationNotSuccessfulErr, ABSOperationPayload.GetBlobName()));

Expected behavior

This should work so that we can make multiple PutBlocks with a unique identifier in Base64 and then modify/add the blocks we need to get one initial Blob consisting of these blocks.

As example:

Put first uncommitted block:

curl --location --request PUT 'https://beedynamics.blob.core.windows.net/cronususainc/1/test1.txt?{SAS}&comp=block&blockid=MQ%3D%3D' \
--header 'x-ms-blob-type: BlockBlob' \
--header 'Content-Type: text/plain' \
--data 'part1'

Put second uncommitted block:

curl --location --request PUT 'https://beedynamics.blob.core.windows.net/cronususainc/1/test1.txt?{SAS}&comp=block&blockid=Mg%3D%3D' \
--header 'x-ms-blob-type: BlockBlob' \
--header 'Content-Type: text/plain' \
--data 'part2'

Commit two blocks as one Blob:

curl --location --request PUT 'https://beedynamics.blob.core.windows.net/cronususainc/1/test1.txt?{SAS}&comp=blocklist' \
--header 'Content-Type: application/xml' \
--data '<BlockList>  
  <Uncommitted>MQ==</Uncommitted>
  <Uncommitted>Mg==</Uncommitted>
</BlockList>  '

Steps to reproduce

Try to use PutBlockList

Additional context

I originally wanted to send a PR with a fix, but judging by the rules it has to be negotiated in Issue first. And yes I know AppendBlock works in a similar way and it works fine.

I will provide a fix for a bug

  • I will provide a fix for a bug
@Drakonian Drakonian added the Bug Something isn't working label Feb 22, 2024
@PaulFurlet
Copy link

PaulFurlet commented Feb 22, 2024

AppendBlock works in a similar way and it works fine.

It produces different type of blob. It would help if we could convert easily afterwards. But once the blob has been created, its type cannot be changed, and it can be updated only by using operations appropriate for that blob type, so it is not easiy convertable, only downloading and uploading with different type, which is negating all efforts we are trying to use here.

Thank you for opening this ticket, I was looking for solution and you managed to get there first :)

@PaulFurlet
Copy link

for reference, my Yammer post

@JesperSchulz
Copy link
Contributor

That sounds like a great candidate for a code contribution! Issue approved. Please go ahead and create a PR 😊

@JesperSchulz JesperSchulz added the Approved The issue is approved label Feb 26, 2024
Drakonian added a commit to Drakonian/BCApps that referenced this issue Feb 29, 2024
JesperSchulz pushed a commit that referenced this issue Mar 1, 2024
<!-- Thank you for submitting a Pull Request. If you're new to
contributing to BCApps please read our pull request guideline below
* https://github.com/microsoft/BCApps/Contributing.md
-->
#### Summary <!-- Provide a general summary of your changes -->
Added PutBlockList overload method with required BlobName param to
BlobClient interface
Added PutBlock methods with required BlobName param to BlobClient
interface
BlobClient implementation updated to set required BlobName param
New unit test for PutBlock and PutBlockList methods.

#### Work Item(s) <!-- Add the issue number here after the #. The issue
needs to be open and approved. Submitting PRs with no linked issues or
unapproved issues is highly discouraged. -->
Fixes #615 

Fixes
[AB#502953](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/502953)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved The issue is approved Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants