tomdesair / tus-java-server

Library to receive tus v1.0.0 file uploads in a Java server environment
MIT License
131 stars 62 forks source link

added new constructor to provide custom UploadIdFactory #17

Closed przemekc closed 5 years ago

przemekc commented 5 years ago

I added this to provide custom UploadIdFactory because there is no other way to provide custom instance.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.002%) to 96.636% when pulling be0b772a35eec61d827176767e15005f1966d4be on przemekc:master into c3a89345956538a72f0c46de634c8c2c5ccd1614 on tomdesair:master.

tomdesair commented 5 years ago

Hi @przemekc,

Thank you for the contribution! Could you explain to me in which way you customized your UploadIdFactory?

Depending on the use cases, we might need to adjust more code in order to support more different implementations of a ID factories.

przemekc commented 5 years ago

Hi

Thank you for the contribution! Could you explain to me in which way you customized your UploadIdFactory?

I need to provide other way to create UUID. In my use case I need to get timestamp from UUID but when I use UUID.randomUUID timestamp() method throw Exception (Not a time-based UUID). So I overide method createId in UploadIdfactor and I'm using Generators.timeBasedGenerator().generate() from com.fastxml.uuid.

That's all I need for now. But without that change there is no way to achive my use case.

I think extract interface for UploadIdFactory is a quite good solution.

tomdesair commented 5 years ago

I took some time to experiment with refactoring the code to also support other upload ID formats that are not UUID-based. The result of this can be found in PR #18.

The main changes are:

@przemekc, would this also cover your requirements?

The thing I like about this approach is that people can choose other ID formats than UUID.

tomdesair commented 5 years ago

Note that in that PR UploadInfo only holds a UploadId instance and no longer a UUID. And that UploadId just contains a (URL safe) String.

przemekc commented 5 years ago

@przemekc, would this also cover your requirements?

The thing I like about this approach is that people can choose other ID formats than UUID.

Yes, It look nice and yes it meets my requirements. Thanks a lot.

przemekc commented 5 years ago

Beacuse #18 has been merged this isn't usefull anymore

tomdesair commented 5 years ago

Hi @przemekc,

I still wanted to thank you again for all your input. I've released the support for custom UploadIdFactory instances of #18 to version 1.0.0-2.0 in Maven Central.

Have a nice weekend!