manyminds / api2go

JSONAPI.org Implementation for Go
MIT License
696 stars 94 forks source link

Program panics with heterogeneous relationship types #249

Open mk6i opened 8 years ago

mk6i commented 8 years ago

This problem is similar to the one described in issue #125. In this case, the program panics when a relationship is defined with multiple resource types. I've forked @tanmayambre's test repository and fixed it up to reproduce the issue. It's available at https://github.com/mkaminski1988/testapi2go.

Let's say I want to define an Author with a relationship named works that contains types books and blogs. The JSON API output would resemble the following:

{
  "data": {
    "type": "author",
    "id": "100",
    "attributes": {
      "name": "John Doe"
    },
    "relationships": {
      "works": {
        "links": {
          "self": "http://192.168.99.100:31415/author/100000004343888/relationships/works",
          "related": "http://192.168.99.100:31415/author/100000004343888/works"
        },
        "data": [
          {
            "type": "books",
            "id": "200"
          },
          {
            "type": "blogs",
            "id": "300"
          }
        ]
      }
    }
  }
}

Here's how I tried to set up the GetReferences() function for the author model type:

func (author Author) GetReferences() []jsonapi.Reference {
    return []jsonapi.Reference{
        {
            Type: "books",
            Name: "works",
        },
        {
            Type: "blogs",
            Name: "works",
        },
    }
}

The following error is produced:

panic: a handle is already registered for path '/v1/authors/:id/relationships/works'

goroutine 1 [running]:
panic(0x30f440, 0xc8201485d0)
    /usr/local/go/src/runtime/panic.go:481 +0x3e6
github.com/julienschmidt/httprouter.(*node).addRoute(0xc820011cc0, 0xc82013d030, 0x13, 0xc8201485c0)
    /Users/207189/dev/go/src/github.com/julienschmidt/httprouter/tree.go:193 +0xcda
github.com/julienschmidt/httprouter.(*Router).Handle(0xc820150240, 0x422190, 0x3, 0xc82013d020, 0x23, 0xc8201485c0)
    /Users/207189/dev/go/src/github.com/julienschmidt/httprouter/router.go:236 +0x22b
github.com/manyminds/api2go/routing.HTTPRouter.Handle(0xc820150240, 0x422190, 0x3, 0xc82013d020, 0x23, 0xc82022c050)
    /Users/207189/dev/go/src/github.com/manyminds/api2go/routing/httprouter.go:25 +0x88
github.com/manyminds/api2go/routing.(*HTTPRouter).Handle(0xc820026218, 0x422190, 0x3, 0xc82013d020, 0x23, 0xc82022c050)
    <autogenerated>:1 +0xc1
github.com/manyminds/api2go.(*API).addResource(0xc82007c360, 0x1244700, 0xc82007c3f0, 0x12446c0, 0xc820026220, 0x1244700)
    /Users/207189/dev/go/src/github.com/manyminds/api2go/api.go:304 +0xbf1
github.com/manyminds/api2go.(*API).AddResource(0xc82007c360, 0x1244700, 0xc82007c3f0, 0x12446c0, 0xc820026220)
    /Users/207189/dev/go/src/github.com/manyminds/api2go/api_public.go:46 +0x49
main.main()
    /Users/207189/dev/go/src/github.com/tanmayambre/testapi2go/main.go:14 +0x129
exit status 2

It seems that this is caused by the fact that api2go generates all the relationship routes by looping over the references returned by GetReferences(), which may contain duplicate Name fields. The routine that generates routes should work off of a de-duped list of relationship names in order to avoid duplicate routes. I have not found a restriction in the JSON API spec that disallows multiple types in a relationship, so this functionality should be supported.

sharpner commented 8 years ago

you are correct. Two identical names in get reference ids currently crashes api2go. I haven't looked further into this issue yet, but it is definitely an interesting point.

mk6i commented 8 years ago

I have a fix, would you be interested in a pull request?

sharpner commented 8 years ago

of course! That would be amazing. Preferably with a corresponding test :-)

olivierperes commented 5 years ago

Was this fixed? I came upon a similar problem, but with to-one heterogeneous relationships. For example, let us say the author has a mostPraisedWork that can be either a book or a blog: this confuses api2go, which fails to call methods of the right structure. It may be a different instance of the same bug, so I am not sure whether I should open a new issue.

sharpner commented 5 years ago

unfortunately it has not been fixed yet... probably it has the same cause