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

feat: Multipart Form Data file uploads #1130

Merged
merged 6 commits into from Feb 4, 2024

Conversation

maxdestors
Copy link
Contributor

@maxdestors maxdestors commented Dec 3, 2023

This PR attempts to implement Multipart Form Data file uploads, as discussed in #195.

I am new to this framework and have referred to the work done in #258 and #558 for guidance.

In the .bru file, I used a | to separate different files, considering that it is a forbidden character on both Windows and Linux.

image

image

Contribution Checklist:

  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have read the contribution guidelines.
  • I have added screenshots or gifs to help explain the change if applicable.
  • Create an issue and link to the pull request.

@maxdestors maxdestors changed the title [Feature] Multipart Form Data file uploads feat: Multipart Form Data file uploads Dec 11, 2023
@Nahuel92 Nahuel92 mentioned this pull request Jan 8, 2024
@Nahuel92
Copy link

@helloanoop Hi (and thanks for this fantastic software :)).
Is there any chance for this to be merged and released any time soon? It would be awesome to have this feature available on Bruno.

@mateo-gallardo
Copy link
Contributor

@maxdestors I made a PR to use relative paths for files that are inside the collection's folder. Since when we share/push the collection to a repository we would upload those test files, and anybody that pulls the collection doesn't have to re-select the files to send the request (and if they save after re-selecting then they would have unnecessary changes in git).

For any files outside the collection's folder I kept the absolute paths. I think that makes the most sense.

@mateo-gallardo
Copy link
Contributor

One more thing, this won't work in the CLI runner as is, since it has another prepare-request script.

@helloanoop do you recommend just copy-pasting for now? Are there plans to share the prepare request code between the CLI runner and the app in the future?

@mateo-gallardo
Copy link
Contributor

Here's a NodeJS server for anyone that wants to try the feature.

server.js

const express = require('express');
const multer = require('multer');
const app = express();

// Configure multer for file uploads
const upload = multer({ dest: 'uploads/' }); // Files will be saved in 'uploads' directory

// Endpoint for multiple files
app.post('/multiple-files', upload.array('files'), (req, res) => {
  console.log(req.files);
  res.send('Multiple files uploaded successfully.');
});

// Endpoint for a single file
app.post('/single-file', upload.single('file'), (req, res) => {
  // req.file contains information about the uploaded file
  console.log(req.file);
  res.send('Single file uploaded successfully.');
});

// Start the server
const PORT = 8000;
app.listen(PORT, () => {
  console.log(`Server running on http://localhost:${PORT}`);
});

Just npm install express multer and node server.js

@binaryfire
Copy link

We didn't realise UI file uploads weren't possible with Bruno when we switched from Postman. Just discovered this limitation today while transferring a collection. Would also love to have this merged soon.

@ash-eng-spin
Copy link

Would also like this feature implemented and have the ability for user to edit content-type of each part.

@helloanoop
Copy link
Contributor

Thank you @maxdestors for your work on this!
I am reviewing this and changes look good.

I will have this merged and shipped in 2 days.

@helloanoop
Copy link
Contributor

Would also like this feature implemented and have the ability for user to edit content-type of each part.

@ash-eng-spin Are you talking about having an option to select the content type as text/file ?

@ash-eng-spin
Copy link

ash-eng-spin commented Jan 30, 2024

Would also like this feature implemented and have the ability for user to edit content-type of each part.

@ash-eng-spin Are you talking about having an option to select the content type as text/file ?

Not quite. In a multi-part request, there is a content type for each part. The user should be able to set the content type themselves so they can ensure it matches the API specification. The content-type can be different for each part and is often required to be sent over in the multi-part request, I've used software before that auto-generates the content-type and that auto-generation isn't always what the API specification wants.

Typically, the Text/File designation in a UI is just to enable the file selection/explorer.

image

Thank you for your hard work and engaging this topic! I was shocked how seamless migration was over to this product, but multi-part functionality is a show stopper for us.

@binaryfire
Copy link

@helloanoop Very happy this is being merged, thank you!

+1 for being able to set the content-type for each part as per @ash-eng-spin's comment. Here's how to do it in Postman (in case you want to check that out as a reference): https://www.youtube.com/watch?v=1yqNfqfZPB8

This is essential for endpoints you need to send a file + a JSON payload to.

…es-browser-fix

Fixed electron files browser for Multipart Form files
…ative-paths

Using relative paths for files inside the collection's folder
@maxdestors
Copy link
Contributor Author

@helloanoop Any idea on how to store the content type in the bru file ?

@helloanoop helloanoop merged commit 634f9ca into usebruno:main Feb 4, 2024
helloanoop added a commit that referenced this pull request Feb 4, 2024
@maxdestors maxdestors deleted the feature/multipart-file branch February 6, 2024 07:16
@ash-eng-spin
Copy link

Hello. I've downloaded version 1.9 and do not see the ability to edit the content-type for each part in a multipart request. Am I missing something do we need to open another issue for it?

@mateo-gallardo
Copy link
Contributor

@ash-eng-spin Changing the content-type wasn't added to this PR. I believe the idea is to wait until the new version of the Bru lang is implemented so we can add decorators for extra info about each param. Please read this issue and see if the proposal would satisfy your use case #1502.

Besides that issue of the Bru lang schema, I don't think there's an issue tracking the ability to edit the content-type, so I think you can go ahead and create one to track it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants