koltyakov / sppull

📎 Download files from SharePoint document libraries using Node.js without hassles
MIT License
46 stars 16 forks source link

Unreachable logic #46

Closed IgorP120 closed 4 years ago

IgorP120 commented 4 years ago

downloadAsStream is never called since metadata never has Length property assigned, and there's no way to assign it from options or context:

const filesize: number = parseInt(metadata.Length + ''); if (filesize > 20000000) { this.downloadAsStream(spFilePath, saveFilePath)

To fix we can:

1) Pass length if known, along with the filename in the strictObjects array. I have a use case, for example, when the lenght is always known.

or

2) Retrieve file metadata internally from SP before calling download.

Please let me know if I can submit a PR.

Thanks, Igor

koltyakov commented 4 years ago

Thanks, Igor! Will take a look.

koltyakov commented 4 years ago

This was relevant for configurations with CAML - a very rare case nowadays. When a CAML filter is configured payload actually misses some metadata as file size (Length prop). Please check a fix published with v2.6.4. Please reopen if the issue still remains for you.

IgorP120 commented 4 years ago

Thanks Andrew, Just a quick question: why don't always use downloadAsStream method, regardless of the file size?

koltyakov commented 4 years ago

Initially, sp-request library was used for every call to the API. But it used request-promise internally, which doesn't support streaming. Download as a stream method was added later on without using sp-request. Recalling this, I can say that this part was not refactored completely to what I considered, as the default download method is also stream-based now. I'd say that there is nothing that should make it impossible to keep one method for any filesize.