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

Allow for custom file names #25

Closed
bhstahl opened this issue May 19, 2016 · 5 comments
Closed

Allow for custom file names #25

bhstahl opened this issue May 19, 2016 · 5 comments

Comments

@bhstahl
Copy link
Contributor

bhstahl commented May 19, 2016

Implementations should be able to provide filename for POST requests, and only if it is not provided, it can default back to Uid.

Thanks for the suggestion @js3692!

@bhstahl bhstahl added this to the 1.0.0 milestone May 19, 2016
@js3692
Copy link
Contributor

js3692 commented May 19, 2016

Great! I would love to help out with this one, if it wasn't already picked up.

My thoughts right now is to pass an options parameter to the TusServer instance:

app.post('/api/videos/tus-upload', function(req, res) {
  server.handle(req, res, { filename: '50554d63-29bb-11e5-b345-feff819cdc9f' });
});

And either override the File id or instantiate with a file name in the POST handler:

send(req, res, options) {
[...]
  if (options && options.filename) {
    file.id = options.filename;
  }

  OR

  // filename would be '50554d63-29bb-11e5-b345-feff819cdc9f'
  // When undefined the constructor would default it to Uid
  const file = new File(upload_length, upload_defer_length, filename);
[...]
}

@bhstahl What are your thoughts?

@bhstahl
Copy link
Contributor Author

bhstahl commented May 19, 2016

Sure! Feel free to open a PR from your fork.

While your suggestion works well for express, it doesn't really make sense for a standalone server that isn't planning on intercepting every request.

I wonder if it could be even simpler. You're going to have some sort of hashing function to generate the file name anyhow, right? What if DataStore.create takes the request instead of a File, then the DataStore decides the file name and creates the File itself. That way you provide the hashing function once and you're good to go:

// Datastore.js
class DataStore {
    constructor(options) {
        this.generateFileName = options.namingFunction || Uid.rand;
    }

    create(req) {
        const file_name = this.generateFileName(req);
        //...
    }
}

// your_server_implementation.js
const fileNameFromUrl = (req) => {
    return req.url.replace(/\//g, '-');
}

server.datastore = new tus.FileStore({
    path: '/files',
    namingFunction: fileNameFromUrl
});

What do you think @js3692 ?

@js3692
Copy link
Contributor

js3692 commented May 20, 2016

Thanks for the comments! I'll open a PR soon.

Yes, I think your suggestion makes better sense for standalone servers to take the hashing function. It could also allow express to flexibly attach the file name to req.filename for example and in the namingFunction just return req.filename.

I've implemented the Datastore.create() to take in req, decide the file name, and create File. Please let me know what you think!

@bhstahl
Copy link
Contributor Author

bhstahl commented May 23, 2016

closed in #30

@bhstahl bhstahl closed this as completed May 23, 2016
@avillarubia
Copy link

Thanks for the comments! I'll open a PR soon.

Yes, I think your suggestion makes better sense for standalone servers to take the hashing function. It could also allow express to flexibly attach the file name to req.filename for example and in the namingFunction just return req.filename.

I've implemented the Datastore.create() to take in req, decide the file name, and create File. Please let me know what you think!

sorry but req.filename is undefined

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

No branches or pull requests

3 participants