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

File downloads (streaming responses) #2230

Closed
bajtos opened this issue Jan 10, 2019 · 30 comments
Closed

File downloads (streaming responses) #2230

bajtos opened this issue Jan 10, 2019 · 30 comments
Labels
feature parity feature REST Issues related to @loopback/rest package and REST transport in general user adoption

Comments

@bajtos
Copy link
Member

bajtos commented Jan 10, 2019

To allow LB4 applications to implement file downloads, we need to make improvements in several areas:

  1. A route (a controller method) must be able to specify not only the response body, but also additional response headers (e.g. Content-Disposition: attachment; filename="filename.jpg"). This is already covered by the issue Configurable response types/formats #436.
  2. It should be possible for routes to provide the response body as a ReadableStream and Buffer too.
  3. Nice to have: a helper function similar to res.download provided by Express that will accept a file name and create an LB4 response object with all necessary fields correctly filled (content type, content disposition, body stream, etc.)
  4. Documentation: explain how a route can specify the response headers and provide body as a readable stream.

Related issues:

@bajtos bajtos added feature REST Issues related to @loopback/rest package and REST transport in general feature parity labels Jan 10, 2019
@dhmlau dhmlau added the 2019Q2 label Jan 22, 2019
@jannyHou
Copy link
Contributor

jannyHou commented Jan 22, 2019

Estimation meeting's discussion:

  • We already allow the controller function to return buffer.
  • Add some acceptance test case to demo an endpoint for file downloading.

@bajtos
Copy link
Member Author

bajtos commented Feb 26, 2019

We already allow the controller function to return buffer.

We can start by supporting only buffers in the first iteration. This is not enough for practical use though. For example, when accessing large data stored in an external storage container (see loopback-component-storage), we don't want to read the entire content into memory, but want to forward the readable stream returned by the outgoing request and write it to the incoming response.

@Bnaya
Copy link

Bnaya commented Mar 18, 2019

Is there any workaround to make stream response atm?

@bajtos
Copy link
Member Author

bajtos commented Mar 19, 2019

Is there any workaround to make stream response atm?

You can inject the Response object into your Controller and then write the response directly. See #1760 and loopbackio/loopback4-example-shopping#16, especially the HomeController.

@gopalakrishnan-subramani

Is Response a wrapper for express? I see a similar functions exposed. I want to see if I could compress and send json content using the same approach.

@dhmlau dhmlau added 2019Q4 and removed 2019Q3 labels Aug 20, 2019
@dhmlau dhmlau added 2020Q1 and removed 2019Q4 labels Nov 2, 2019
@dhmlau
Copy link
Member

dhmlau commented Dec 10, 2019

Discussing with @bajtos on the above comment, the code in the HomePageController that makes download possible are:

constructor(
    @inject(PackageKey) private pkg: PackageInfo,
    @inject(RestBindings.Http.RESPONSE) private response: Response,  //<--- HERE
  ) 

and

@get('/', {
    responses: {
      '200': {
        description: 'Home Page',
        content: {'text/html': {schema: {type: 'string'}}}, //<----- AND HERE
      },
    },
  })
  homePage() {

See full file in https://github.com/strongloop/loopback4-example-shopping/pull/16/files.

@Riplar
Copy link

Riplar commented Dec 17, 2019

@dhmlau @bajtos
I am trying to get this working for a pdf which is stored as an utf-8 in my database. For testing, I also tried to load a local file as utf-8 and send this data, but it also does not get downloaded.

@get('/dateien/download/{id}', {
    responses: {
      '200': {
        description: 'Dateien download success',
        content: {'application/pdf': {schema: {type: 'string'}}}
      }
    }
  })
  async download (
    @param.path.number('id') id: number
  ): Promise<any> {
    let testData = fs.readFileSync("C:\\Users\\xxxxx\\Downloads\\test.pdf", {encoding: 'utf-8'});

    this.response
      .status(200)
      .contentType('application/pdf')
      .send(testData);
  }

When I try to execute this with the API explorer, I get the following message:

Unrecognized response type; displaying content as text.

The following response headers are set:

 access-control-allow-credentials: true 
 access-control-allow-origin: * 
 content-length: 34238 
 content-type: application/pdf; charset=utf-8 
 date: Tue, 17 Dec 2019 12:35:36 GMT 
 etag: W/"85be-/rwRXmc4aMvo1UfSHV18YWA4dJc" 
 x-powered-by: Express 

Do you have any idea why this is not working?

@dhmlau
Copy link
Member

dhmlau commented Dec 19, 2019

@RipkensLar, I found from swagger-api/swagger-ui#4250 that we should probably set the content-disposition in the header as well. The attachment function would just do that. Therefore, I have this working code below:

@get('/testfile', {
    responses: {
      '200': {
        description: 'get dummy file',
        content: {'application/pdf': {schema: {type: 'string'}}},
      },
    },
  })
  getFile() {
    let data = fs.readFileSync('some-file-path');
    this.response
      .status(200)
      .contentType('application/pdf')
      .attachment('some-file-name.pdf')
      .send(data);
  }

Hope it helps.

@Riplar
Copy link

Riplar commented Dec 19, 2019

Thanks @dhmlau, that helped. I am able to download the pdf, but all pages are just blank. This could be a problem with loopback because I am able to download the exact same file in another project which is written in php but uses the same database.

Edit:
When I receive the file in loopback, it throws the following error:

Malformed UTF-8 characters, possibly incorrectly encoded.

This happens when I call my service which delivers me the file from the database. I double checked it in the php project and there I am able to use the data as it is, so I don't know why I get the error message that it is malformed.

Edit2:
Ok, so when I simply echo the data instead of returning it from my php project, I don't get the malformed error message and receive the data as it is in loopback, but the pages are still blank.
Calling the php service directly via browser allows me to download the file correctly.

@dhmlau
Copy link
Member

dhmlau commented Dec 19, 2019

@RipkensLar, i got the problem with empty file before, but after I switched to use fs.readFileSync instead of fs.createFileStream, the problem no longer exists. When looking up the issue, I found someone uses some libraries to create a PDFDocument and load the file.

@hacksparrow
Copy link
Contributor

fs.readFileSync is blocking, use fs.readFile with a callback function instead.

@Riplar
Copy link

Riplar commented Dec 19, 2019

@dhmlau But I am loading the pdf from a database, see explanation below:
I have two projects. In project A (written in PHP) the users are able to upload and download pdfs. This is working and used in production quite some time. Currently I am setting up another project where I need to access the database from project A because I need the same pdfs and don't want to store them twice. So I wrote a service which delivers me the pdf from the database from project A, and in my controller I send this data via

this.response
      .status(200)
      .contentType('application/pdf')
      .attachment('some-file-name.pdf')
      .send(data);

But as stated above the pages remain blank. Calling the service in project A directly via browser give me the correct pdf. I also compared the raw data of the pdf and it is the same in the service in project A as well as in my controller in the loopback project.

I suspect that OpenApi destroys the encoding when it sends the response, but I am running out of ideas.

@gopalakrishnan-subramani

@dhmlau

response.send(data) worked for me, thanks for the support.

@Riplar
Copy link

Riplar commented Dec 20, 2019

@dhmlau I just created a repo for you so you are able to see what is happening:
https://github.com/RipkensLar/loopback-next

Steps to reproduce:
Check out my repo.
Switch into branch pdf-bug.
Go to examples/todo.
Npm install.
Npm start.
Open the api-explorer.
Try out todos/download and then download the file.
As you can see the file is empty.

The pdf data which I use in the controller method is the data I receive from my php project api.

@Riplar
Copy link

Riplar commented Jan 2, 2020

@dhmlau do you have any updates on this?

@hacksparrow
Copy link
Contributor

@RipkensLar the file is not empty as seen in this screenshot.
image

If it is not displaying any content in PDF viewers, something must be wrong in the format.

@Riplar
Copy link

Riplar commented Jan 8, 2020

@hacksparrow sorry, with empty I meant that it is not displaying any content. Do you have any ideas how I could troubleshoot this?

@hacksparrow
Copy link
Contributor

That's a PDF file format problem, so no idea.

@bajtos bajtos added the p3 label Jan 13, 2020
@bajtos
Copy link
Member Author

bajtos commented Jan 13, 2020

This looks like a character encoding (charset) problem to me.

  1. What database are you using?
  2. What column type is used to store PDF documents? Is it binary, varchar, something else? What character encoding is your database configured with?
  3. What is your model definition? In particular, how is defined the model property holding PDF data? Is it a string? A Buffer? (BTW, I am not sure if we support binary data in LoopBack, we will have to check.)
  4. In your sandbox, you are setting the content type to application/pdf. You may need to include charset, e.g. application/pdf; charset=utf-8. The charset value (utf-8 in my example), must match the encoding used by the PDF content.

@bajtos
Copy link
Member Author

bajtos commented Jan 13, 2020

@RipkensLar the PDF problem you have described is off topic here, can you please open a new issue please?

@joeysino
Copy link

joeysino commented Feb 4, 2020

Suggestion: Allow us to call out to the static file delivery

Use case: I have large files stored on the drive, but they are sensitive, so I don't want to use this.static() in the application. I need to check access rights first.

But I do want to take advantage of static's streaming and caching features. (For example it can also support HEAD requests for skipping to the middle of the content.)

Example of how I might like to use it:

  • I add a controller route which does access checking, and if access is granted, then I pass the request off to the static handler.
@authenticate('BearerStrategy')
@get('/documents/{filename}', {
async serveDocument(@param.path.string('filename') filename: string): Promise<void> {
  if (await callerHasAccessTo(filename)) {
    // Please give me a function serveStaticFile so that I can do this
    return await serveStaticFile(path.join(storageFolder, filename), this.req);
  } else {
    throw new HttpErrors.NotFound('Nope');
  }
}

Loopback could help by providing this serveStaticFile() function. The function would handle any caching and skipping concerns, just like the current static routing does.


Update: I notice that this.static() does provide seeking with HEAD but it does not appear to support caching (or at least it's not working with my HTTP requests).

@joeysino
Copy link

joeysino commented Feb 4, 2020

Suggestion: Support caching when delivering from DB

Use case: I have a large file in a repository, and I have also stored a checksum for it there.

The client requests the file from our API, also passing an etag header.

If the header value matches the stored checksum, that means the browser already has the file, and I can reply with 304 Not Modified. If not, then I must stream the file as normal (and I will also pass the checksum in the response etag header).

Loopback can help either by:

  1. Provide example code for how to do this, OR

  2. Provide a function that will do this logic for us when called.

Option 1 offers the most flexibility. But an implementation of number 2 might suit a majority of use cases. (If etag works, does anyone care about last-modified?)


For anyone who wants this now, I had some success with:

// Instead of a generated checksum, in this example I just use the document ID
const eTag = 'my-hash-' + fileDoc.id;
// Causes Chrome to cache for 1 month, during which it does not even make requests.
// After 1 month it makes If-None-Match requests, resulting in an efficient 304.
this.req.res!.setHeader('Cache-Control', `private, max-age=${31 * 24 * 60 * 60}`);
this.req.res!.setHeader('ETag', eTag);

@asaurav025
Copy link

asaurav025 commented Feb 11, 2020

@bajtos Can you help me out with stubing Response parameter of the controller.(Any documentation or example)

@Arathy-sivan
Copy link

Arathy-sivan commented Mar 11, 2020

hi,
I am trying to download files using loopback4, showing error message,

Server is running at http://[::1]:3000
Try http://[::1]:3000/ping
Unhandled error in GET /testfile: 500 TypeError: Cannot read property 'status' of undefined
at TodoController.getFile (D:\todo\src\controllers\todo.controller.ts:280:8)
at invokeTargetMethod (D:\todo\node_modules@loopback\context\src\invocation.ts:255:47)
at D:\todo\node_modules@loopback\context\src\invocation.ts:232:12
at Object.transformValueOrPromise (D:\todo\node_modules@loopback\context\src\value-promise.ts:270:12)
at invokeTargetMethodWithInjection (D:\todo\node_modules@loopback\context\src\invocation.ts:227:10)
at InterceptedInvocationContext.invokeTargetMethod (D:\todo\node_modules@loopback\context\src\invocation.ts:118:14) at targetMethodInvoker (D:\todo\node_modules@loopback\context\src\interceptor.ts:341:23)
at D:\todo\node_modules@loopback\context\src\interceptor-chain.ts:175:14
at Object.transformValueOrPromise (D:\todo\node_modules@loopback\context\src\value-promise.ts:270:12)
at GenericInterceptorChain.invokeNextInterceptor (D:\todo\node_modules@loopback\context\src\interceptor-chain.ts:170:12)
at GenericInterceptorChain.next (D:\todo\node_modules@loopback\context\src\interceptor-chain.ts:158:17)
at GenericInterceptorChain.invokeInterceptors (D:\todo\node_modules@loopback\context\src\interceptor-chain.ts:144:17)
at Object.invokeInterceptors (D:\todo\node_modules@loopback\context\src\interceptor-chain.ts:207:16)
at D:\todo\node_modules@loopback\context\src\interceptor.ts:343:14
at Object.tryWithFinally (D:\todo\node_modules@loopback\context\src\value-promise.ts:196:14)
at Object.invokeMethodWithInterceptors (D:\todo\node_modules@loopback\context\src\interceptor.ts:337:10)

** controller is,

@get('/testfile', {
responses: {
'200': {
description: 'get dummy file',
content: { 'application/txt': { schema: { type: 'string' } } },
},
},
})
getFile() {
let data = fs.readFileSync('D:/todo/public/files/file1');
this.response
.status(200)
.contentType('application/txt')
.attachment('file1.txt')
.send(data);
}

Could you please help me to solve this.

@emonddr
Copy link
Contributor

emonddr commented Mar 11, 2020

@Arathy-sivan , please provide a minimal application that exhibits the problem. This will help us investigate. Thank you.

@emonddr
Copy link
Contributor

emonddr commented Mar 11, 2020

@Arathy-sivan , here is a recent example for uploading a sample : https://github.com/strongloop/loopback-next/tree/master/examples/file-upload .

@Arathy-sivan
Copy link

@Arathy-sivan , here is a recent example for uploading a sample : https://github.com/strongloop/loopback-next/tree/master/examples/file-upload .

When I used this code I got the error,

Unhandled error in POST /file-upload: 500 Error: unknown format "binary" is used in schema at path "#/properties/file"
at Object.generate_format [as code] (D:\todo\node_modules@loopback\rest\node_modules\ajv\lib\dotjs\format.js:69:15)
at Object.generate_validate [as validate] (D:\todo\node_modules@loopback\rest\node_modules\ajv\lib\dotjs\validate.js:382:35)
at Object.generate_properties [as code] (D:\todo\node_modules@loopback\rest\node_modules\ajv\lib\dotjs\properties.js:195:26)
at generate_validate (D:\todo\node_modules@loopback\rest\node_modules\ajv\lib\dotjs\validate.js:382:35)
at localCompile (D:\todo\node_modules@loopback\rest\node_modules\ajv\lib\compile\index.js:88:22)
at Ajv.compile (D:\todo\node_modules@loopback\rest\node_modules\ajv\lib\compile\index.js:55:13)
at Ajv._compile (D:\todo\node_modules@loopback\rest\node_modules\ajv\lib\ajv.js:348:27)
at Ajv.compile (D:\todo\node_modules@loopback\rest\node_modules\ajv\lib\ajv.js:114:37)
at createValidator (D:\todo\node_modules@loopback\rest\src\validation\request-body.validator.ts:223:14)
at validateValueAgainstSchema (D:\todo\node_modules@loopback\rest\src\validation\request-body.validator.ts:139:16)
at Object.validateRequestBody (D:\todo\node_modules@loopback\rest\src\validation\request-body.validator.ts:71:9)
at buildOperationArguments (D:\todo\node_modules@loopback\rest\src\parser.ts:91:9)
at Object.parseOperationArgs (D:\todo\node_modules@loopback\rest\src\parser.ts:47:10)
at MySequence.handle (D:\todo\src\sequence.ts:36:20)
at HttpHandler._handleRequest (D:\todo\node_modules@loopback\rest\src\http-handler.ts:78:5)

@raymondfeng
Copy link
Contributor

See #4834 (comment)

@Arathy-sivan
Copy link

See #4834 (comment)

Thank You soo much. It's working.

@lygstate
Copy link

lygstate commented Apr 5, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature parity feature REST Issues related to @loopback/rest package and REST transport in general user adoption
Projects
None yet
Development

No branches or pull requests