🧠 Logic: 🐛 Fix consult/1 base64 decoding

[bdeneux]

Fixes #384.

[codecov]

Codecov Report

Merging #387 (5ab1113) into main (bd6c780) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #387      +/-   ##
==========================================
+ Coverage   63.84%   63.93%   +0.09%     
==========================================
  Files          60       60              
  Lines        2633     2640       +7     
==========================================
+ Hits         1681     1688       +7     
  Misses        873      873              
  Partials       79       79              

Impacted Files	Coverage Δ
x/logic/fs/wasm.go	89.13% <100.00%> (+1.95%)	⬆️

[amimart]

That's really nice, thanks! Always appreciate reflect incantations 🙄

I've some remarks regarding error messages and also an open question regarding eventual iterator error, let's discuss it :)

[amimart]

Oh and as the changes on the virtual file system and the consult are breaking changes, I think the related commits shall contains the breaking change marker (i.e. !) after the commit's types :)

[amimart]

That's great thanks! 💪

[ccamel]

Apologies for the delay on this matter. I have some concerns about the proposed approach.

I would have incorporated instructions for Base64 decoding in the URI itself, as it carries the protocol elements. IMHO, the responsibility for decoding should lie with the VFS rather than the consuming function (as initially proposed by #384). And we could consider to have Base64 decoding set as the default option for the considered URI, to avoid any breaking changes, I presume. as it stands, the current approach appears to be quite limiting. What are your thoughts on this?

[bdeneux]

@ccamel Yes I'm agree, I prefer to make the logical on URI that will hold this decoding information but we have choose this approach to avoid breaking change. Otherwise, make the decoding by default on this URI and add an option to remove the decoding should be a good compromise.

Before continue, which URI parameter could be used to deactivate encoding ? &options=no_decoding, &decoding=none, or something else ?

[amimart]

I would have incorporated instructions for Base64 decoding in the URI itself, as it carries the protocol elements. IMHO, the responsibility for decoding should lie with the VFS rather than the consuming function (as initially proposed by #384). And we could consider to have Base64 decoding set as the default option for the considered URI, to avoid any breaking changes, I presume. as it stands, the current approach appears to be quite limiting. What are your thoughts on this?

Well, I'm divided..

Regarding the Base64 at the VFS level, the need appeared using a consult loading sources from the objectarium smart contract which return the data in Base64, this seems to me far from the nominal case and it hardly justify this behavior on the VFS, in my opinion.

On the other side this is weird to have the Base64 decoding in the consult, but to prevent any limitations the implementation here supports both Base64 and raw data.

However, I'm not against going on the URI param way I you both prefer :)

[ccamel]

Before continue, which URI parameter could be used to deactivate encoding ? &options=no_decoding, &decoding=none, or something else ?

One way to declare the intention of base64 decoding right in the URI would be: ?base64Decode=true. Or the opposite if the default behavior is systematic decoding: ?base64Decode=false.

[ccamel]

Great! Thanks.

[bot-anik]

🎉 This PR is included in version 5.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀