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.

Sign up for GitHub

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

Jump to bottom

Work in progress: Improve filestream #173

Open
wants to merge 20 commits into
base: filestream
Choose a base branch
from
Open

Work in progress: Improve filestream #173

wants to merge 20 commits into from

Conversation

pbricout
Copy link
Contributor

@pbricout pbricout commented May 7, 2023 •
edited
Loading

New implementation makes completely isomorphic the read/write sync and async implementation, the only difference in the async implementation are the file calls and the await.
The new implementation handles multiple ID3 tags.

Still to do:

  • Improve write implementation
  • Make buffer size an option for write, write test to cover different code path for writeId3TagToFileSync/Async
  • Improve read implementation
  • Refactor common parts of the steam process class
  • Make buffer size an option for read, write test to cover different code path for read
  • Support update
  • Add padding support
  • Fix temp file generation, actually Date.now() may generate several times the same file especially when tests are ran in parallel and even more if the precision is reduced
  • Something is broken with Node 10.x I must use a feature not available in node 10.x

pbricout added 15 commits May 8, 2023 01:29
@pbricout
Improve file-write
d671721 
- simplify the code and remove some functions
- remove unnecessary unlink before the rename
  this is acually better as rename is atomic so we don't
  lose the original file if the rename fails
- delete the temp file in the case of error
- factor common code for tmp filename:
  this allows to replace two functions with one
- factor common code in file copy (findId3TagPosition):
  this simplifies the function
- factor common code to remove id3 tag from buffer:
  this allows to replace two functions with one
- simplify function names
- split long lines
- implementation is much shorter
- the sync/async implementations almost only differs with
  this calls to sync/async APIs
@pbricout
Document further simplification to be done
de1d85d
@pbricout
Remove the use of "tmp" library
12fa3e5
@pbricout
Cleanup the API and more
08a4827 
- remove support for function which do not make sense for:
  create, read, write, update, promises
- clean up return types and throws in case of error
  (create with callback for example)
@pbricout
Add write option file buffer size
82ca7a3 
- introduce WriteOptions type
- implements a new write API proposal
- update tests to use file buffer size options
  (more tests to do to improve coverage)
@pbricout
Add some failing write test cases
5b0d804
@pbricout
Add a getId3Tag() util function
317b314 
Returning this information is cheap and will simplify code
in other places.
@pbricout
Fix write for ID3 id spanning across two read access
538bb1e 
- also simplify some names
@pbricout
Simplify removeId3Tag()
85150bc
@pbricout
Simplify getId3TagBody()
4e7695e
@pbricout
Rename getId3TagDataFromFileAync()
1557d86
@pbricout
Experimental: Use generator in Id3TagRemover
aa8ce89 
Not really simpler, the only benefit is more scoped `let tag`.
Not really worth the extra complexity.
@pbricout
Simplify file write
c5549e0
@pbricout
wip: Refactor file-read
15b7166
@pbricout
wip: Factor file stream processors
3563334
@pbricout pbricout changed the base branch from master to filestream May 7, 2023 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@pbricout