Merge lp:~dimitern/juju-core/102-state-endpoint-refactoring into lp:~go-bot/juju-core/trunk

[Dimiter Naydenov]

state;charm: Refactor state/Endpoint

Two methods of state.Endpoint are not related to state,
more to charm metadata. Endpoint embeds a charm.Relation,
so I moved IsImplicit() and ImplementedBy() from Endpoint
to charm.Relation, along with their tests.
This is necessary, so that we can avoid implementing API
calls for them in the uniter API facade, becasue the
uniter needs them.

https://codereview.appspot.com/12906044/

R=jameinel, rogpeppe

state;charm: Refactor state/Endpoint

Two methods of state.Endpoint are not related to state,
more to charm metadata. Endpoint embeds a charm.Relation,
so I moved IsImplicit() and ImplementedBy() from Endpoint
to charm.Relation, along with their tests.
This is necessary, so that we can avoid implementing API
calls for them in the uniter API facade, becasue the
uniter needs them.

https://codereview.appspot.com/12906044/

[Dimiter Naydenov (dimitern)]

Reviewers: mp+180141_code.launchpad.net,

Message:
Please take a look.

Description:
state;charm: Refactor state/Endpoint

Two methods of state.Endpoint are not related to state,
more to charm metadata. Endpoint embeds a charm.Relation,
so I moved IsImplicit() and ImplementedBy() from Endpoint
to charm.Relation, along with their tests.
This is necessary, so that we can avoid implementing API
calls for them in the uniter API facade, becasue the
uniter needs them.

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

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/12906044/

Affected files:
   A [revision details]
   M charm/meta.go
   M charm/meta_test.go
   M state/endpoint_test.go

[Dimiter Naydenov (dimitern)]

Please take a look.

https://codereview.appspot.com/12906044/

[John A Meinel (jameinel)]

This just moves the functions to charm.Relation which has all the
information we were operating on.

LGTM

https://codereview.appspot.com/12906044/

[Roger Peppe (rogpeppe)]

LGTM

https://codereview.appspot.com/12906044/

=== modified file 'charm/meta.go'
--- charm/meta.go	2013-07-09 10:32:23 +0000
+++ charm/meta.go	2013-08-14 14:03:11 +0000
@@ -48,6 +48,47 @@
 	Scope     RelationScope
 }
 
+// ImplementedBy returns whether the relation is implemented by the supplied charm.
+func (r Relation) ImplementedBy(ch Charm) bool {
+	if r.IsImplicit() {
+		return true
+	}
+	var m map[string]Relation
+	switch r.Role {
+	case RoleProvider:
+		m = ch.Meta().Provides
+	case RoleRequirer:
+		m = ch.Meta().Requires
+	case RolePeer:
+		m = ch.Meta().Peers
+	default:
+		panic(fmt.Errorf("unknown relation role %q", r.Role))
+	}
+	rel, found := m[r.Name]
+	if !found {
+		return false
+	}
+	if rel.Interface == r.Interface {
+		switch r.Scope {
+		case ScopeGlobal:
+			return rel.Scope != ScopeContainer
+		case ScopeContainer:
+			return true
+		default:
+			panic(fmt.Errorf("unknown relation scope %q", r.Scope))
+		}
+	}
+	return false
+}
+
+// IsImplicit returns whether the relation is supplied by juju itself,
+// rather than by a charm.
+func (r Relation) IsImplicit() bool {
+	return (r.Name == "juju-info" &&
+		r.Interface == "juju-info" &&
+		r.Role == RoleProvider)
+}
+
 // Meta represents all the known content that may be defined
 // within a charm's metadata.yaml file.
 type Meta struct {

=== modified file 'charm/meta_test.go'
--- charm/meta_test.go	2013-08-08 09:17:27 +0000
+++ charm/meta_test.go	2013-08-14 14:03:11 +0000
@@ -417,3 +417,81 @@
 		c.Assert(input, DeepEquals, output)
 	}
 }
+
+var implementedByTests = []struct {
+	ifce     string
+	name     string
+	role     charm.RelationRole
+	scope    charm.RelationScope
+	match    bool
+	implicit bool
+}{
+	{"ifce-pro", "pro", charm.RoleProvider, charm.ScopeGlobal, true, false},
+	{"blah", "pro", charm.RoleProvider, charm.ScopeGlobal, false, false},
+	{"ifce-pro", "blah", charm.RoleProvider, charm.ScopeGlobal, false, false},
+	{"ifce-pro", "pro", charm.RoleRequirer, charm.ScopeGlobal, false, false},
+	{"ifce-pro", "pro", charm.RoleProvider, charm.ScopeContainer, true, false},
+
+	{"juju-info", "juju-info", charm.RoleProvider, charm.ScopeGlobal, true, true},
+	{"blah", "juju-info", charm.RoleProvider, charm.ScopeGlobal, false, false},
+	{"juju-info", "blah", charm.RoleProvider, charm.ScopeGlobal, false, false},
+	{"juju-info", "juju-info", charm.RoleRequirer, charm.ScopeGlobal, false, false},
+	{"juju-info", "juju-info", charm.RoleProvider, charm.ScopeContainer, true, true},
+
+	{"ifce-req", "req", charm.RoleRequirer, charm.ScopeGlobal, true, false},
+	{"blah", "req", charm.RoleRequirer, charm.ScopeGlobal, false, false},
+	{"ifce-req", "blah", charm.RoleRequirer, charm.ScopeGlobal, false, false},
+	{"ifce-req", "req", charm.RolePeer, charm.ScopeGlobal, false, false},
+	{"ifce-req", "req", charm.RoleRequirer, charm.ScopeContainer, true, false},
+
+	{"juju-info", "info", charm.RoleRequirer, charm.ScopeContainer, true, false},
+	{"blah", "info", charm.RoleRequirer, charm.ScopeContainer, false, false},
+	{"juju-info", "blah", charm.RoleRequirer, charm.ScopeContainer, false, false},
+	{"juju-info", "info", charm.RolePeer, charm.ScopeContainer, false, false},
+	{"juju-info", "info", charm.RoleRequirer, charm.ScopeGlobal, false, false},
+
+	{"ifce-peer", "peer", charm.RolePeer, charm.ScopeGlobal, true, false},
+	{"blah", "peer", charm.RolePeer, charm.ScopeGlobal, false, false},
+	{"ifce-peer", "blah", charm.RolePeer, charm.ScopeGlobal, false, false},
+	{"ifce-peer", "peer", charm.RoleProvider, charm.ScopeGlobal, false, false},
+	{"ifce-peer", "peer", charm.RolePeer, charm.ScopeContainer, true, false},
+}
+
+func (s *MetaSuite) TestImplementedBy(c *C) {
+	for i, t := range implementedByTests {
+		c.Logf("test %d", i)
+		r := charm.Relation{
+			Interface: t.ifce,
+			Name:      t.name,
+			Role:      t.role,
+			Scope:     t.scope,
+		}
+		c.Assert(r.ImplementedBy(&dummyCharm{}), Equals, t.match)
+		c.Assert(r.IsImplicit(), Equals, t.implicit)
+	}
+}
+
+type dummyCharm struct{}
+
+func (c *dummyCharm) Config() *charm.Config {
+	panic("unused")
+}
+
+func (c *dummyCharm) Revision() int {
+	panic("unused")
+}
+
+func (c *dummyCharm) Meta() *charm.Meta {
+	return &charm.Meta{
+		Provides: map[string]charm.Relation{
+			"pro": {Interface: "ifce-pro", Scope: charm.ScopeGlobal},
+		},
+		Requires: map[string]charm.Relation{
+			"req":  {Interface: "ifce-req", Scope: charm.ScopeGlobal},
+			"info": {Interface: "juju-info", Scope: charm.ScopeContainer},
+		},
+		Peers: map[string]charm.Relation{
+			"peer": {Interface: "ifce-peer", Scope: charm.ScopeGlobal},
+		},
+	}
+}

=== modified file 'state/endpoint.go'
--- state/endpoint.go	2013-07-09 10:32:23 +0000
+++ state/endpoint.go	2013-08-14 14:03:11 +0000
@@ -46,47 +46,6 @@
 		counterpartRole(ep.Role) == other.Role
 }
 
-// ImplementedBy returns whether the endpoint is implemented by the supplied charm.
-func (ep Endpoint) ImplementedBy(ch charm.Charm) bool {
-	if ep.IsImplicit() {
-		return true
-	}
-	var m map[string]charm.Relation
-	switch ep.Role {
-	case charm.RoleProvider:
-		m = ch.Meta().Provides
-	case charm.RoleRequirer:
-		m = ch.Meta().Requires
-	case charm.RolePeer:
-		m = ch.Meta().Peers
-	default:
-		panic(fmt.Errorf("unknown relation role %q", ep.Role))
-	}
-	rel, found := m[ep.Name]
-	if !found {
-		return false
-	}
-	if rel.Interface == ep.Interface {
-		switch ep.Scope {
-		case charm.ScopeGlobal:
-			return rel.Scope != charm.ScopeContainer
-		case charm.ScopeContainer:
-			return true
-		default:
-			panic(fmt.Errorf("unknown relation scope %q", ep.Scope))
-		}
-	}
-	return false
-}
-
-// IsImplicit returns whether the endpoint is supplied by juju itself,
-// rather than by a charm.
-func (ep Endpoint) IsImplicit() bool {
-	return (ep.Name == "juju-info" &&
-		ep.Interface == "juju-info" &&
-		ep.Role == charm.RoleProvider)
-}
-
 type epSlice []Endpoint
 
 var roleOrder = map[charm.RelationRole]int{

=== modified file 'state/endpoint_test.go'
--- state/endpoint_test.go	2013-07-09 10:32:23 +0000
+++ state/endpoint_test.go	2013-08-14 14:03:11 +0000
@@ -77,84 +77,3 @@
 	c.Assert(ep1.CanRelateTo(ep2), Equals, false)
 	c.Assert(ep2.CanRelateTo(ep1), Equals, false)
 }
-
-type dummyCharm struct{}
-
-func (c *dummyCharm) Config() *charm.Config {
-	panic("unused")
-}
-
-func (c *dummyCharm) Revision() int {
-	panic("unused")
-}
-
-func (c *dummyCharm) Meta() *charm.Meta {
-	return &charm.Meta{
-		Provides: map[string]charm.Relation{
-			"pro": {Interface: "ifce-pro", Scope: charm.ScopeGlobal},
-		},
-		Requires: map[string]charm.Relation{
-			"req":  {Interface: "ifce-req", Scope: charm.ScopeGlobal},
-			"info": {Interface: "juju-info", Scope: charm.ScopeContainer},
-		},
-		Peers: map[string]charm.Relation{
-			"peer": {Interface: "ifce-peer", Scope: charm.ScopeGlobal},
-		},
-	}
-}
-
-var implementedByTests = []struct {
-	ifce     string
-	name     string
-	role     charm.RelationRole
-	scope    charm.RelationScope
-	match    bool
-	implicit bool
-}{
-	{"ifce-pro", "pro", charm.RoleProvider, charm.ScopeGlobal, true, false},
-	{"blah", "pro", charm.RoleProvider, charm.ScopeGlobal, false, false},
-	{"ifce-pro", "blah", charm.RoleProvider, charm.ScopeGlobal, false, false},
-	{"ifce-pro", "pro", charm.RoleRequirer, charm.ScopeGlobal, false, false},
-	{"ifce-pro", "pro", charm.RoleProvider, charm.ScopeContainer, true, false},
-
-	{"juju-info", "juju-info", charm.RoleProvider, charm.ScopeGlobal, true, true},
-	{"blah", "juju-info", charm.RoleProvider, charm.ScopeGlobal, false, false},
-	{"juju-info", "blah", charm.RoleProvider, charm.ScopeGlobal, false, false},
-	{"juju-info", "juju-info", charm.RoleRequirer, charm.ScopeGlobal, false, false},
-	{"juju-info", "juju-info", charm.RoleProvider, charm.ScopeContainer, true, true},
-
-	{"ifce-req", "req", charm.RoleRequirer, charm.ScopeGlobal, true, false},
-	{"blah", "req", charm.RoleRequirer, charm.ScopeGlobal, false, false},
-	{"ifce-req", "blah", charm.RoleRequirer, charm.ScopeGlobal, false, false},
-	{"ifce-req", "req", charm.RolePeer, charm.ScopeGlobal, false, false},
-	{"ifce-req", "req", charm.RoleRequirer, charm.ScopeContainer, true, false},
-
-	{"juju-info", "info", charm.RoleRequirer, charm.ScopeContainer, true, false},
-	{"blah", "info", charm.RoleRequirer, charm.ScopeContainer, false, false},
-	{"juju-info", "blah", charm.RoleRequirer, charm.ScopeContainer, false, false},
-	{"juju-info", "info", charm.RolePeer, charm.ScopeContainer, false, false},
-	{"juju-info", "info", charm.RoleRequirer, charm.ScopeGlobal, false, false},
-
-	{"ifce-peer", "peer", charm.RolePeer, charm.ScopeGlobal, true, false},
-	{"blah", "peer", charm.RolePeer, charm.ScopeGlobal, false, false},
-	{"ifce-peer", "blah", charm.RolePeer, charm.ScopeGlobal, false, false},
-	{"ifce-peer", "peer", charm.RoleProvider, charm.ScopeGlobal, false, false},
-	{"ifce-peer", "peer", charm.RolePeer, charm.ScopeContainer, true, false},
-}
-
-func (s *EndpointSuite) TestImplementedBy(c *C) {
-	for i, t := range implementedByTests {
-		c.Logf("test %d", i)
-		ep := state.Endpoint{
-			ServiceName: "x",
-			Relation: charm.Relation{
-				Interface: t.ifce,
-				Name:      t.name,
-				Role:      t.role,
-				Scope:     t.scope,
-			},
-		}
-		c.Assert(ep.ImplementedBy(&dummyCharm{}), Equals, t.match)
-		c.Assert(ep.IsImplicit(), Equals, t.implicit)
-	}
-}