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

Add onBlock processor to all requests #139

Open
9 tasks
edeckers opened this issue Mar 21, 2021 · 5 comments
Open
9 tasks

Add onBlock processor to all requests #139

edeckers opened this issue Mar 21, 2021 · 5 comments
Labels
Android enhancement New feature or request iOS
Projects

Comments

@edeckers
Copy link
Owner

edeckers commented Mar 21, 2021

Originally suggested by @pke in #136

The library is missing the support for processing data that is being transferred.

Proposed solution

  • Add .onBlock(fn: (block) => void, settings?: { blockSize: number }), similar to onProgress
  • Add .onFinishStream(fn: (block) => void) which will be called instead of .onBlock when the last block of the stream arrives

Progress

  • Add .onBlock method to TypeScript code base
  • Add .onFinishStream method to TypeScript code base
  • Intercept data and send it in chunks to .onBlock and .onFinishStream over the bridge on Android
  • Intercept data and send it in chunks to .onBlock and .onFinishStream over the bridge on iOS
  • Allow transformation of intercepted data in .onBlock and .onFinishStream-callbacks
  • Write transformed data to the target
  • Add TypeScript tests
  • Add Android tests
  • Add iOS tests
@edeckers edeckers added Android enhancement New feature or request iOS labels Mar 21, 2021
@edeckers edeckers added this to Backlog in Planned Mar 21, 2021
@edeckers edeckers changed the title Add onBlock processor to all requests Add onBlock processor to all requests Mar 21, 2021
@pke
Copy link

pke commented Mar 21, 2021

Way to go!

The callback could/should(?) return a promise and get handed in if the given block is the last one. To know its the last block is crucial for some transforms, like crypto or checksum calculation.

What do you mean by that?

Add target toggle that to fetch that sends data not to a file but to /dev/null

@edeckers
Copy link
Owner Author

The callback could/should(?) return a promise and get handed in if the given block is the last one. To know its the last block is crucial for some transforms, like crypto or checksum calculation.

Whoops, I looked at your use-case and I figured you could just call aes.decrypt on each received block, and then when the request finishes call aes.finish. That is too late though and it wouldn't let you transform the last block of data correctly.

How about I also add an .onFinishStream(fn:(block) => void)-method, which will be called instead of onBlock when the last block is received, like this?

@edeckers
Copy link
Owner Author

edeckers commented Mar 22, 2021

Add target toggle that to fetch that sends data not to a file but to /dev/null

Scratch that, I rushed jotting down this ticket a little. I removed this requirement.

@pke
Copy link

pke commented Mar 23, 2021

An additional argument to onBlock could indicate it's the last block. Why would it be too late to call aes.finish then?
I know node has a stream API that also uses onEnd alongside onData but I've never understood why it was not comb into one callback with argument indicating the stream is at its end.

Either way, conform to known node patterns or a single callback function is fine with me.

@edeckers
Copy link
Owner Author

edeckers commented Mar 23, 2021

An additional argument to onBlock could indicate it's the last block. Why would it be too late to call aes.finish then?

Your solution with the last parameter was absolutely fine. I was trying to explain why I thought before we could do without that parameter and why I was wrong. I hope my comment makes more sense now?

Either way, conform to known node patterns or a single callback function is fine with me.

Alright, thank you for your confirmation; the .onFinishStream-way will be my approach then!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android enhancement New feature or request iOS
Projects
Planned
  
Backlog
Development

No branches or pull requests

2 participants