CRUD API

[yuyabee]

I added CRUD API which can be used like:

type Books struct {}

func (books Books) CreateHandler(ctx *gin.Context) {
  // create logic here ...
}

func (books Books) ReadHandler(ctx *gin.Context) {
  // read logic here ...
}

func (books Books) UpdateHandler(ctx *gin.Context) {
  // update logic here ...
  // you can access to :id by ctx.Params.ByName("id")
}

func (books Books) DeleteHandler(ctx *gin.Context) {
  // delete logic here ...
  // you can access to :id by ctx.Params.ByName("id")
}

// The API is:
//   POST:       /api/books
//   GET:        /api/books
//   PUT:        /api/books/:id
//   DELETE:     /api/books/:id
booksAPI := r.Group("/api/books", auth.TokenAuth)
booksAPI.CRUD("", books)

What do you think about this?

[Thomasdezeeuw]

No GET /api/books/:id? Also are all handlers required? Make they should be optional.

[yuyabee]

Well, in my opinion, getting details of each book is client side's job.
/api/books should return all the information about all of the books and you should manipulate the data in the client side.

And I'll work on making handlers optional as soon as possible.

[yuyabee]

So I added new Interface to make each method to be optional.
It basically supports all the methods that are implemented on the resource, and ignore if a method is not implemented.

Example of not providing Delete method(just not implementing it on Books):

type Books struct {}

func (books Books) CreateHandler(ctx *gin.Context) {
  // create logic here ...
}

func (books Books) ReadHandler(ctx *gin.Context) {
  // read logic here ...
}

func (books Books) UpdateHandler(ctx *gin.Context) {
  // update logic here ...
  // you can access to :id by ctx.Params.ByName("id")
}

// The API is:
//   POST:       /api/books
//   GET:        /api/books
//   PUT:        /api/books/:id
booksAPI := r.Group("/api/books", auth.TokenAuth)
booksAPI.CRUD("", books)

[jmrobles]

Hi!

Just I wanna add the resource object similar to Django REST Framework does with the ViewSet class (list, create, update, delete,...)

IMHO, API resource should return an 501 or 405 status code if not handler is attached. Is it possible return this code only checking if the interface signal is nil?

Thanks and go go go! :)

Enviado desde mi iPhone

El 20/08/2014, a las 07:09, yuyabee notifications@github.com escribió:

So I added new Interface to make each method to be optional.
It basically supports all the methods that are implemented on the resource, and ignore if a method is not implemented.

Example of not providing Delete method(just not implementing it on Books):

type Books struct {
}

func (books Books) CreateHandler(ctx *gin.Context) {
// create logic here ...
}

func (books Books) ReadHandler(ctx *gin.Context) {
// read logic here ...
}

func (books Books) UpdateHandler(ctx *gin.Context) {
// update logic here ...
}

// The API is:
// POST: /api/books
// GET: /api/books
// PUT: /api/books/:id
booksAPI := r.Group("/api/books", auth.TokenAuth)
booksAPI.CRUD("", books)
—
Reply to this email directly or view it on GitHub.

[yuyabee]

Hmm...
I get your point and I think that is possible, but isn't it too much work of a framework to provide a default not supported handlers for each method that are not implemented?

To be honest, I am +1 on adding not supported handlers, but I believe that providing a way to add NotSupported handler for each method as optional is enough as a framework.

I'm thinking of something like this to support not implemented methods:

type Books struct {}

func (books Books) CreateNotSupportedHandler(ctx *gin.Context) {
  ctx.Fail(501, "Not Implemented")
}

Let me here your opinion @jmrobles

[Thomasdezeeuw]

I think a default handler which returns 501 or 405 is unnecessary, the developer should handle that. And it wil likely result in a 404, which is ok.

[jmrobles]

I think that solution generate a lot of dumb code, not? Which the rule nil == not allowed, less code, do more IMHO.

By the other hand, we need some mechanism to auth request. Django rest framework solution is good for me.
http://www.django-rest-framework.org/api-guide/permissions#custom-permissions

El 20/08/2014, a las 20:05, Thomas de Zeeuw notifications@github.com escribió:

I think a default handler which returns 501 or 405 is unnecessary, the developer should handle that. And it wil likely result in a 404, which is ok.

—
Reply to this email directly or view it on GitHub.

[yuyabee]

In #20 it was discussed whether to support 405 status code support by default or not, but it seems that it is something to be done with httprouter which is another library, to support not implemented default method. And I agree to @Thomasdezeeuw that returning 404 is ok.

Therefore, I'm not going to include 405 or 501 status code support in this PR.
I think you can create an issue about that.
@jmrobles @Thomasdezeeuw Thanks for your comments anyways.

[Thomasdezeeuw]

Any change this could get merged? I could really use this :)

[Thomasdezeeuw]

I think their could be an improvement in the api, why not something like this:

r.CRUD("/api/books", auth.TokenAuth, books)

And maybe add an IndexHandler, getting multiple or all items.

// GET: /api/books       // Get all books, calls IndexHandler
// GET: /api/books/:id, // Gets a single book, calls ReadHandler

[yuyabee]

@Thomasdezeeuw Your suggested improvement looks great!! I definitely want it. However I'm not sure whether there should be another function called CRUDWithMiddleware or should the CRUD function force users to give middleware to it or maybe same CRUD function with another signatures.

Since the handlers are optional now, which wasn't at the time you asked about it few days ago, I'll add the ReadHandler in your context which I would call TakeHandler. I think that's more self descriptive. Isn't it? Also, in my opinion, IndexHandler is too ambiguous by its name, so I'll call it ListHandler for now :)

In short, I'll work on adding GET method to specified id item first and your suggested new API later when we come up with a good solution to its name.

Thanks anyways :)

BTW, is it gonna be merged?

[Thomasdezeeuw]

ListHandler sound good to me, as for accepting middleware maybe something like this would work:

func (group *RouterGroup) CRUD(path string, middleware... interface{}) {
  // Pop the the last "middleware" from the slice, which is actually the resource
  resource, middleware = middleware[len(middleware)-1], middleware[:len(middleware)-1]

  //...
}

As for merging this pr, I don't know, ask @manucorporat, he is the owner.

[manucorporat]

I am evaluating this new API, I wonder how to integrate it properly. I do not want to start converting Gin a dog with tentacles.

[jmrobles]

Hi Manu,

I think that one good approach would be a subpackage like "gin/rest", is
it possible in Go?

Salu2!

2014-08-29 13:58 GMT+02:00 Manu Mtz.-Almeida notifications@github.com:

I am evaluating this new API, I wonder how to integrate it properly. I do
not want to start converting Gin a dog with tentacles.

—
Reply to this email directly or view it on GitHub
#97 (comment).

Jose M. Robles Hermoso

digitalilusion.com
Google+: http://gplus.to/jmr
Twitter: twitter.com/jmrobles
Blog: http://www.robleshermoso.com
AboutMe: http://about.me/roblesjm

[manucorporat]

yes, it's possible. yes I like that much more, keeping the core separated

[yuyabee]

You mean providing another package called rest in the gin project directory?
Since the CRUD method is defined as a method to gin.RouterGroup, it is impossible to define it in another package. Go doesn't let you define a method to a type that is not defined locally.

Isn't separating a file enough?
Maybe add rest.go in the root directory?

[reedobrien]

I am of the opinion that you should be able to get a single resource with GET /.../encyclopedia/:id. It is not useful to transfer all the encyclopedia volumes to the client for it to find the one it wants.

Also, I would want to see tests, more complete documentation including Examples, and further community discussion about it's value.

As it stands I am -1 on seeing this merged.

[yuyabee]

You are able to get a single resource with GET /.../encyclopedia/:id in my current commit if you implement TakeHandler on your encyclopedia resource.

I would like to write tests and complete documentation once the details of integrating this new API with Gin is decided, which we are still not sure of whether to provide it as a sub package or put it in a different file. However, for now, in my opinion, the examples provided on the comments is enough.

@reedobrien Thanks for your comment :)

[johndeng]

Just we know the Gin is simple and clear Golang web framework, if add more feature like create a restful api etc that the Gin will been not pure.

Whatever, @yuyabee your jobs is great!! just like @jmrobles say, being a Gin package is better.

[yuyabee]

I agree to @johndeng opinion that adding a feature like CRUD API is too high level for Gin, because it is a simple framework.

However, I don't understand what you mean by sub package of Gin. You mean providing it in another directory so that it can be referenced by 'gin-gonic/gin/rest' or make another repository which is called 'gin-rest'?

Well, I'll just work on another commit that provides CRUD API in 'gin-gonic/gin/rest' with different arguments from the original version due to the fact that Go doesn't allow you to define methods of a type that isn't defined locally.

Thanks for your comment @johndeng :)

[johndeng]

@yuyabee As i mean the package just like the Martini Contrib

[rogeriomarques]

Why not add it to https://github.com/gin-gonic/contrib ?

[yuyabee]

@rogeriomarques Isn't gin-gonic/contrib for middleware?

[se77en]

@yuyabee yes gin-gonic/contrib for middleware, you can add it to contrib

[javierprovecho]

@yuyabee sorry for delay.

Merged in gin-gonic/contrib at gin-gonic/contrib@fabaf0e.