state: Make Relation an Entity

state: Make Relation an Entity

This adds relation tags to the names package. The
format is "relation-<id>", where <id> is the relation's
numeric id.
It also adds a Tag() method on the relation, and a few
other necessary modifications to make state.Relation
objects behave like other entities. This is needed for
the uniter API facade, so relations can be used as
params and results.

https://code.launchpad.net/~dimitern/juju-core/103-relation-entity-tags/+merge/180157

Requires: https://code.launchpad.net/~dimitern/juju-core/102-state-endpoint-refactoring/+merge/180141

(do not edit description out of merge proposal)

[dimitern, 2013-08-14 14:40:51]

Please take a look.

[rog, 2013-08-14 15:02:48]

LGTM with some extra tests.

https://codereview.appspot.com/12748044/diff/1/names/relation.go
File names/relation.go (right):

https://codereview.appspot.com/12748044/diff/1/names/relation.go#newcode18
names/relation.go:18: func RelationTag(relationId string) string {
if !IsRelation(relationId) {
   panic(fmt.Sprintf("%q is not a valid relation id", relationId))
}

https://codereview.appspot.com/12748044/diff/1/state/relation.go
File state/relation.go (right):

https://codereview.appspot.com/12748044/diff/1/state/relation.go#newcode66
state/relation.go:66: func (r *Relation) Tag() string {
This should have a test.

https://codereview.appspot.com/12748044/diff/1/state/state_test.go
File state/state_test.go (right):

https://codereview.appspot.com/12748044/diff/1/state/state_test.go#newcode1438
state/state_test.go:1438: tag: "service-ser-vice2",
We need a relation tag test here.

[dimitern, 2013-08-14 15:16:16]

Please take a look.

https://codereview.appspot.com/12748044/diff/1/names/relation.go
File names/relation.go (right):

https://codereview.appspot.com/12748044/diff/1/names/relation.go#newcode18
names/relation.go:18: func RelationTag(relationId string) string {
On 2013/08/14 15:02:49, rog wrote:
> if !IsRelation(relationId) {
>    panic(fmt.Sprintf("%q is not a valid relation id", relationId))
> }

Done.

https://codereview.appspot.com/12748044/diff/1/state/relation.go
File state/relation.go (right):

https://codereview.appspot.com/12748044/diff/1/state/relation.go#newcode66
state/relation.go:66: func (r *Relation) Tag() string {
On 2013/08/14 15:02:49, rog wrote:
> This should have a test.

Done.

https://codereview.appspot.com/12748044/diff/1/state/state_test.go
File state/state_test.go (right):

https://codereview.appspot.com/12748044/diff/1/state/state_test.go#newcode1438
state/state_test.go:1438: tag: "service-ser-vice2",
On 2013/08/14 15:02:49, rog wrote:
> We need a relation tag test here.

Done.

[gz, 2013-08-14 15:54:56]

LGTM.

https://codereview.appspot.com/12748044/diff/5002/names/relation_test.go
File names/relation_test.go (right):

https://codereview.appspot.com/12748044/diff/5002/names/relation_test.go#newc...
names/relation_test.go:16: var relationIdTests = []struct {
Okay, so this is a case where I like the table test style.

https://codereview.appspot.com/12748044/diff/5002/state/state.go
File state/state.go (right):

https://codereview.appspot.com/12748044/diff/5002/state/state.go#newcode498
state/state.go:498: return nil, errors.NotFoundf("relation %s", id)
If the conversion fails, is that really a not found rather than an invalid
situation?

Is there a test exercising this branch?

[dimitern, 2013-08-14 16:03:30]

Please take a look.

https://codereview.appspot.com/12748044/diff/5002/names/relation_test.go
File names/relation_test.go (right):

https://codereview.appspot.com/12748044/diff/5002/names/relation_test.go#newc...
names/relation_test.go:16: var relationIdTests = []struct {
On 2013/08/14 15:54:56, gz wrote:
> Okay, so this is a case where I like the table test style.

Yeah!

https://codereview.appspot.com/12748044/diff/5002/state/state.go
File state/state.go (right):

https://codereview.appspot.com/12748044/diff/5002/state/state.go#newcode498
state/state.go:498: return nil, errors.NotFoundf("relation %s", id)
On 2013/08/14 15:54:56, gz wrote:
> If the conversion fails, is that really a not found rather than an invalid
> situation?
> 
> Is there a test exercising this branch?

I'll add a test, thanks. And the point is: there won't be a relation with an
invalid id anyway, so it's NotFoundError. It will also help later in the API
when we convert NotFoundErrors to ErrPerm.