vardius / go-api-boilerplate

Go Server/API boilerplate using best practices DDD CQRS ES gRPC
https://go-api-boilerplate.local
MIT License
929 stars 135 forks source link

"x-www-form-urlencoded" to "json" conversion middleware for gorouter #59

Closed mar1n3r0 closed 4 years ago

mar1n3r0 commented 4 years ago

I am currently working on porting a "x-www-form-urlencoded" to "json" conversion middleware for gorouter: https://github.com/mar1n3r0/go-json-rest-middleware-formjson.

I will open a separate topic about that if you don't mind and think that it will be of significant use to the boilerplate overall.

Originally posted by @mar1n3r0 in https://github.com/vardius/go-api-boilerplate/pull/58#issuecomment-597015340

mar1n3r0 commented 4 years ago

60 In the future it would be harder to contribute to the package since our directions for development diverged already with social oauth and jwt token generate differences. Probably we should switch to a different strategy from now on where we cross-check masters for desired features but still it would be a lot of hassle with dependencies to transfer features in between.

After all that's why it's a boilerplate so we can contribute to the point where we have common grounds and aims. My fork is becoming more tailored to my personal needs so becoming more of a niche base rather than a boilerplate. As such I would be mostly pulling critical bugs and security fixes.

The only reasonable way I see contributing back is with external packages like the middleware in this PR. I could not take advantage and reuse RespondJSONError and the custom internal error wrapper in this middleware due to the fact they are kept internal to the boilerplate and not being external packages like the gorouter.

If you don't feel like merging this PR let me know so I can delete the branch in the fork. It's already been merged in master on my side.

vardius commented 4 years ago

we should rename it from internal to pkg then also what do you think if we refactor RespondJSONError a little bit ?

maybe we should make handlers return errors and then handle that error ? I don't know how it would work with router yet or if its good solution ? What do you think ?

vardius commented 4 years ago

Shouldn't this be called somehow differently ? https://github.com/mar1n3r0/gorouter-middleware-formjson/blob/23077ef237c30909047406e789d35af985d8e718/formjson.go#L22

Its more like it does toJson or even multipartFormToJson

EDIT: OMG I am blind! its form... not from hahahaha

vardius commented 4 years ago

https://github.com/vardius/go-api-boilerplate/pull/60#discussion_r391585556

mar1n3r0 commented 4 years ago

we should rename it from internal to pkg then also what do you think if we refactor RespondJSONError a little bit ? maybe we should make handlers return errors and then handle that error ? I don't know how it would work with router yet or if its good solution ? What do you think ?

I think it serves its purpose well for internal CLI error reporting. No complaints about that. For command payload response though I am replacing it with native e.ErrorMessage() since this is the response to the client.

Shouldn't this be called somehow differently ? https://github.com/mar1n3r0/gorouter-middleware-formjson/blob/23077ef237c30909047406e789d35af985d8e718/formjson.go#L22

Its more like it does toJson or even multipartFormToJson

The middleware doesn't handdle multipart as of yet, only urlencoded. Multipart can not be converted to JSON as it is a blob and depends on the purpose of the whole application so it would be hard to boilerplate that.

multipart-requests-in-go

Multipart form data Normally, when a form is submitted through the browser, it will use application/x-www-form-urlencoded content-type, which is just a list of keys and values. This is not for uploading files and therefore, that’s where multipart/form-data content-type comes in.

vardius commented 4 years ago

We can create pkg directory and move stuff that can be reused there from https://github.com/vardius/go-api-boilerplate/tree/master/internal

in a new pull request

maybe all of them could be reused ? then we just rename dir ?

vardius commented 4 years ago

For command payload response though I am replacing it with native e.ErrorMessage() since this is the response to the client.

maybe should push that ?

mar1n3r0 commented 4 years ago

We can create pkg directory and move stuff that can be reused there from https://github.com/vardius/go-api-boilerplate/tree/master/internal

in a new pull request

maybe all of them could be reused ? then we just rename dir ?

That should be the way to go.

For command payload response though I am replacing it with native e.ErrorMessage() since this is the response to the client.

maybe should push that ?

I will try to cherry-pick that.

vardius commented 4 years ago

That should be the way to go.

do you want to create a pr for it ?

or if you want me to do it I can do it over the weekend

mar1n3r0 commented 4 years ago

I will create a PR for it no worries.

vardius commented 4 years ago

Merged