tus / tusd

Reference server implementation in Go of tus: the open protocol for resumable file uploads
https://tus.github.io/tusd
MIT License
2.95k stars 467 forks source link

Why is config.validate() called inside UnroutedHandler? Should we let user decide for themselves? #1004

Closed jimydavis closed 9 months ago

jimydavis commented 9 months ago
  1. config.validate() should not be called in UnroutedHandler as it is hydrating the config with defaults, it is not just validating it. It is expected that NewHandler comes with all the bells and whistles via the validate call but UnroutedHandler should be slightly more primitive. User can always copy paste the code.
    1. Alternatively, we can make Validate public and let user call it themselves.
  2. config.isAbs should not be private if we wish for user to construct their own validate() function.
Acconut commented 9 months ago

I don't understand your point, sorry. Yes, validate might be ill-named as it also sets the defaults, but why would you want to disable this behavior? The handler depends on all config fields to be set, so it is important that default values are set when no value is provided by the caller. In the end, this also benefits the user as it reduces the possibility of errors.

  1. UnroutedHandler should be slightly more primitive. User can always copy paste the code.

I disagree with both. UnroutedHandler should be the same as the routed Handler, except for the routing. Everything else should be the same. Also, requiring the user to duplicate internal code is not the preferred approach.

jimydavis commented 9 months ago

Thank you for the quick reply! I think it was mostly my confusion with the verb validate as well as code that follows after the validate function inside NewUnroutedHandler. I was trying to figure out what was the separation. It is not an issue after I read your answer and I will close the question.