Proposed change to HTTP Methods

[matryer]

Since POST should create resources, I propose we change the way Goweb maps the RESTful interfaces in the following way.

These will stay the same:

RestfulReader Read = GET /path/{id}
RestfulManyReader ReadMany = GET /path
RestfulCreator Create = POST /path
RestfulDeletor Delete = DELETE /path/{id}
RestfulManyDeleter DeleteMany = DELETE /path
(OPTIONS and HEAD will remain the same)    

These will change from:

RestfulUpdater Update = PUT /path/{id}
RestfulReplacer Replace = POST /path/{id}
RestfulManyUpdater UpdateMany = PUT /path

to

RestfulUpdater Update = PATCH /path/{id}
RestfulReplacer Replace = PUT /path/{id}
RestfulManyUpdater UpdateMany = PATCH /path

• Since this will break existing interfaces, it's an important thing to carefully consider the consequences.

[vayam]

Since this will break existing interfaces, it's an important thing to carefully consider the consequences.

Agreed. PATCH is used for partial updates. Most of goweb users must be using PUT for complete replacement. So it should not impact I am guessing.

[lukescott]

I would be fine with the changes if you added:

RestfulManyReplacer ReplaceMany = PUT /path

It would also be nice if you could configure which methods are used. XDomainRequest (CORS IE 8 & 9 support) only supports GET and POST. IE 8 also doesn't support PATCH. It would also help with backwards compatibility.

[matryer]

Do you think it makes more sense to add these interfaces for those who want to be explicit:

PatchOneController
GetOneController
PutOneController
DeleteOneController
PostOneController
PatchManyController
GetManyController
PutManyController
DeleteManyController
PostManyController

On 16 Oct 2013, at 10:52, Luke Scott notifications@github.com wrote:

I would be fine with the changes if you added:

RestfulManyReplacer ReplaceMany = PUT /path

It would also be nice if you could configure which methods are used. XDomainRequest (CORS IE 8 & 9 support) only supports GET and POST. IE 8 also doesn't support PATCH. It would also help with backwards compatibility.

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

[lukescott]

That would be overkill, wouldn't it? Just need a way to change http.Method* in the MapController function.

[matryer]

Technically, if you map the functions yourself, you can do this using the goweb.Map method. That might be easier.

[matryer]

Maybe this would make it cleaner: https://github.com/stretchr/goweb/issues/39

So we'd add something like this:

type Mapper interface {
  Map(context.Context) error
}

Then if a controller implements that interface, it'll call the Map method, and you can have the mapping done inside the controllers.

[lukescott]

I could be misunderstanding you, but that seems like a lot of work. I would have to write mapping code for every controller? Why wouldn't I use Map and avoid MapController then? It doesn't seem to add any benefit.

If I want every Update method to be POST, I'm not going to want to write mapping code for every single controller. I'd want to configure it in one place that Update should use POST for all controllers.

The Replace method currently uses POST, which other people currently depend on. If you change this to PUT you are going to break everything. A one line configuration change is easier to fix though.

[matryer]

OK that makes sense,

How about something like this:

goweb.HttpMethodDecider = func(controller interface{}) string {
  if _, ok := controller.(RestfulUpdator); ok {
    return "POST"
  }
}

We can provide a default one that gets used if the goweb.HttpMethodDecider returns an empty string.

Mat

On 16 Oct 2013, at 11:14, Luke Scott notifications@github.com wrote:

I could be misunderstanding you, but that seems like a lot of work. I would have to write mapping code for every controller? Why wouldn't I use Map and avoid MapController then? It doesn't seem to add any benefit.

If I want every Update method to be POST, I'm not going to want to write mapping code for every single controller. I'd want to configure it in one place that Update should use POST for all controllers.

The Replace method currently uses POST, which other people currently depend on. If you change this to PUT you are going to break everything. A one line configuration change is easier to fix though.

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

[lukescott]

While the intent in flexibility is nice, something like this would be far simpler:

controller.RestfulUpdaterMethod = "POST"

A bit off topic, but I'd really like to do something like this:

POST /invoices/[id]/pay :

func (ic *InvoicesController) Pay(id string, c context.Context) { // ... }

[matryer]

You can do almost that with goweb.Map:

goweb.Map("POST", "invoices/{id}/pay", InvoicesController.Pay)

^ this works today, but you'd have to do ctx.PathParams("id") instead of having it as a parameter.

P.s. thanks for engaging on this issue, it's proving to be very helpful indeed.

[lukescott]

I do this now, but this doesn't invoke my Controller's Before method.

[matryer]

https://github.com/stretchr/goweb/issues/59

[matryer]

please see https://github.com/stretchr/goweb/pull/60