helapkg / hela

:icecream: Powerful software development experience and management. Enhancing @tc39 JS, @denoland and @nodejs, because we need a bit of magic. :sparkles: You can think of it as Cargo for the JavaScript ecosystem.
Mozilla Public License 2.0
331 stars 41 forks source link

How can I rename the upload file? #88

Closed imkimchi closed 7 years ago

imkimchi commented 7 years ago

I'm trying to rename upload file(especially image) so I can make them have own unique name using shortid module. I just read all readme and couldn't figure out how to do it.

tunnckoCore commented 7 years ago

It's a problem of Formidable. So you can just get the filepath and move it. Probably this issue #260 will work for you, since you can pass instance of formidable to the options.

var path = require("path");

form.on("fileBegin", function(name, file) {
  file.path = path.join("<upload directory here>", file.name);
});

But "note that it disables form.uploadDir and form.keepExtensions parameters", which is normal, logical and expected when you think it.

imkimchi commented 7 years ago

I never used formidable so I just want to ask quick question about it. I have multiple seperated router and I just want to apply 'formidable' to a single router.

In this case, should I code like this? Would it work?

let form = new IncomingForm();
form.encoding = "utf-8";

form.on("fileBegin", function(name, value) {
  file.path = path.join("../uploads", shortid.generate());
});

form.on("end", function(name, file) {
  console.log("finish parse");
});

router.post("/upload", upload, async (ctx, next) => {
  console.log(ctx.request.body.files);
  ctx.body = { status: "success" };
});
tunnckoCore commented 7 years ago

I believe, yes. I had some idea for module called koa-upload before a few years, but don't have time. It won't be something hard, but need to handle such simple things like this, that's why i registered it.

imkimchi commented 7 years ago

I don't understand how I can get full filepath. As I see this example It uses this.body.files.foo.path to get path, but since this is written in koa1, I don't know how to do the same thing with version 2. any idea?

tunnckoCore commented 7 years ago

There's no so much diffs between Koa1 and Koa2, just replace this with ctx. Or what's the problem?

imkimchi commented 7 years ago

yeah I get that part, but It's really weird that It doesn't parse anything besides multipart/formdata since I use koa-better-body,. so everytime I get POST request to my server, ctx.request.body is always undefined. I'd be really appreciated if you answer this!

app.js

import form from './util/formidable'
import betterBody from 'koa-better-body'

function startApp() {
    const app = new Koa()
    const port = process.env.PORT || 3000
    const dist = __dirname + '/views/'
    const bpOption = { IncomingForm: form }

    app
      .use(logger())
      .use(serve(dist))
      .use(session({}, app))
      .use(betterBody(bpOption))
      .use(passport.initialize())
      .use(passport.session())
      .use(views(__dirname+'/views', { extension: 'pug'}))
      .use(routes())

one of the routers that doesn't use multipart/form-data

router.post("/signup", async (ctx, next) => {
  let data = ctx.request.body; // -> It's undefined since I use koa-better-body
  const param = await makeParam(data);

  try {
    const user = new User(param);
    await user.save();
    ctx.redirect(`/u/${data.username}`);
  } catch (e) {
    console.error("Failed to save post request", e, param);
  }
});

edited by Charlike: please add js to the code blocks

tunnckoCore commented 7 years ago

Don't know. It may be a problem from other middleware. It's tested against Koa1 and Koa2, so i can't believe it is not working.

Can you post dependencies from your package.json?

imkimchi commented 7 years ago

I do have koa-bodyparser but as you can see in my code above I'm not using it.

"dependencies": {
    "bcrypt": "^1.0.2",
    "bcrypt-nodejs": "0.0.3",
    "bcryptjs": "^2.4.3",
    "bluebird": "^3.5.0",
    "fine-uploader": "^5.14.5",
    "koa": "^2.3.0",
    "koa-better-body": "^3.0.2",
    "koa-body": "^2.3.0",
    "koa-bodyparser": "^4.2.0",
    "koa-logger": "^3.0.0",
    "koa-multer": "^1.0.1",
    "koa-passport": "^3.0.0",
    "koa-router": "^7.2.1",
    "koa-session": "^5.4.0",
    "koa-static": "^3.0.0",
    "koa-views": "^6.0.2",
    "mongoose": "^4.10.8",
    "passport": "^0.3.2",
    "passport-local": "^1.0.0",
    "path": "^0.12.7",
    "pug": "^2.0.0-rc.2"
  }
imkimchi commented 7 years ago

I just fixed this problem using koa-body, but I'm wondering how I can get actual downloaded path. because when I print out ctx.request.body.files.pic.path, I get '/var/folders/ls/75t48c651sb1pt749vj3wz4m0000gn/T/upload_dc1a848b5aa6da5ff38bbb21d610199e' but actually It's supposed to be /uploads/filename. you can see this in my code below

import IncomingForm from "formidable";
import shortid from "shortid";
import path from "path";

let form = new IncomingForm();
form.encoding = "utf-8";

form.on("fileBegin", function(name, file) {
  let regex = /[^.]*/;
  let fileName = file.name.replace(regex, shortid());
  file.path = path.join(__dirname + "/../uploads/", fileName);
});

export default form;
tunnckoCore commented 7 years ago

Hm. Sorry, don't know. Probably this approach from linked issue in Formidable repo, seems to not work then. I should definitely add such tests. I'm setuping new dev machine, so i can try some variants and add some tests about that.

Probably...

a second later.. Oh, i see what may be the problem. You should pass form.uploadDir too.

Side note: I definitely don't like that Formidable API. Sadly, the whole code base behind it too and that's why i wanted to be one of the maintainers. When i and the others have more time we can modernize formidable a lot.

imkimchi commented 7 years ago

but didn't you that renaming file.path disables form.uploadDir?

imkimchi commented 7 years ago

I just figured that fileBegin event in formidable never gets triggered. It worked until yesterday and It randomly stopped working. I didn't do anything such upgrading node version or something. Node version is v7.9 now.

formidable.js


import IncomingForm from "formidable";
import shortid from "shortid";
import path from "path";

let form = new IncomingForm();
form.encoding = "utf-8";

form.on("fileBegin", function(name, file) {
  let regex = /[^.]*/;
  console.log("file name is", file.name);
  let fileName = file.name.replace(regex, shortid());
  file.path = path.join(__dirname + "/../uploads/", fileName);
});

export default form;

router

import Router from "koa-router";

const router = new Router({ prefix: "/image" });

router.post("/", async (ctx, next) => {
  console.log(ctx.request.body.files.pic); // It works fine
  ctx.body = { status: "success" };
});

export default router;

app.js

import form from './util/formidable'

    const app = new Koa()
    const port = process.env.PORT || 3000
    const dist = __dirname + '/views/'
    const bpOption = {multipart: true, IncomingForm: form}

    app
      .use(logger())
      .use(serve(dist))
      .use(session({}, app))
      .use(bodyParser(bpOption))
      .use(passport.initialize())
      .use(passport.session())
      .use(views(__dirname+'/views', { extension: 'pug'}))
      .use(routes())

    app.listen(port, () => console.log(`[!] Server is Running on ${port}`))
}
imkimchi commented 7 years ago

this problem seems really messed up. I fixed the uploading problem changing bodyparser to koa-better-body from koa-body, now ctx.request.body.files returns undefined. https://github.com/tunnckoCore/koa-better-body/issues/89 this issue references this problem as well

tunnckoCore commented 7 years ago

this problem seems really messed up.

I'm agree. And it's totally unrelated to what was the title of the thread. Is it renaming okey now, so we can close this?

I'll will start work on new version soon in anyway.

imkimchi commented 7 years ago

sorry for getting out of topic! I just closed the issue and hope it'll get fixed asap as I use this for production :)

tunnckoCore commented 7 years ago

Cool! Sorry for the headaches, but mostly it seems it works, tests shows this too.

Can you please try to not pass Formidable instance to options, but just use options and uploadDir, then in the next middleware you can see what and where is uploaded.

Something like

app.use(
  betterBody({
    // encoding: 'utf-8', // this is the default
    // multipart: true, // this is default
    uploadDir: path.join(__dirname, "..", "uploads")
  })
);
app.use((ctx, next) => {
  console.log(ctx.request.files);
  return next();
});

If this shows the files, you can loop over that array and move them

imkimchi commented 7 years ago

it worked. but since I can't change the name of file before saving it, file name get messed up something like upload_41234719283742813 (even has no filename extension). How can I fix it ?

tunnckoCore commented 7 years ago

I believe we have file's mime or content type? Check out what the object of the file contain?

seconds later: yea, file.type is the mime type of the file, so based on that and some help libraries you can set correct extension.

imkimchi commented 7 years ago

I just fixed it by git rebasing to the day when It worked fine! now everything works fine. I think there was some issue in client codes. honestly I still think there was some issue in this module. I can show you my private repo if you want to figure out what was wrong.

anyways, I really appreciate all your helps :) have a great day!

tunnckoCore commented 7 years ago

Sweet to hear! :) Clearing caches, node_modules and reinstall everything may help some times too, instead of rebasing.

Yea, i can look if you give access, no problem.

honestly I still think there was some issue in this module.

Probably, we will see in next version.

anyways, I really appreciate all your helps :) have a great day!

Trying my best :100:

sendQueue commented 3 years ago

But "note that it disables form.uploadDir and form.keepExtensions parameters", which is normal, logical and expected when you think it.

You saved my day a few years later..! :) Thanks haha