juju-core

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

  1. 102-state-endpoint-refactoring
  2. Merge into trunk
Proposed by Dimiter Naydenov
Status: Merged
Approved by: Dimiter Naydenov
Approved revision: no longer in the source branch.
Merged at revision: 1660
Proposed branch: lp:~dimitern/juju-core/102-state-endpoint-refactoring
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 278 lines (+119/-122)
4 files modified
charm/meta.go (+41/-0)
charm/meta_test.go (+78/-0)
state/endpoint.go (+0/-41)
state/endpoint_test.go (+0/-81)
To merge this branch: bzr merge lp:~dimitern/juju-core/102-state-endpoint-refactoring
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+180141@code.launchpad.net

Commit message

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

Description of the change

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/

To post a comment you must log in.
Revision history for this message
Dimiter Naydenov (dimitern) wrote :

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

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

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

Revision history for this message
John A Meinel (jameinel) wrote :

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

LGTM

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

Revision history for this message
Roger Peppe (rogpeppe) wrote :

LGTM

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
48 Scope RelationScope48 Scope RelationScope
49}49}
5050
51// ImplementedBy returns whether the relation is implemented by the supplied charm.
52func (r Relation) ImplementedBy(ch Charm) bool {
53 if r.IsImplicit() {
54 return true
55 }
56 var m map[string]Relation
57 switch r.Role {
58 case RoleProvider:
59 m = ch.Meta().Provides
60 case RoleRequirer:
61 m = ch.Meta().Requires
62 case RolePeer:
63 m = ch.Meta().Peers
64 default:
65 panic(fmt.Errorf("unknown relation role %q", r.Role))
66 }
67 rel, found := m[r.Name]
68 if !found {
69 return false
70 }
71 if rel.Interface == r.Interface {
72 switch r.Scope {
73 case ScopeGlobal:
74 return rel.Scope != ScopeContainer
75 case ScopeContainer:
76 return true
77 default:
78 panic(fmt.Errorf("unknown relation scope %q", r.Scope))
79 }
80 }
81 return false
82}
83
84// IsImplicit returns whether the relation is supplied by juju itself,
85// rather than by a charm.
86func (r Relation) IsImplicit() bool {
87 return (r.Name == "juju-info" &&
88 r.Interface == "juju-info" &&
89 r.Role == RoleProvider)
90}
91
51// Meta represents all the known content that may be defined92// Meta represents all the known content that may be defined
52// within a charm's metadata.yaml file.93// within a charm's metadata.yaml file.
53type Meta struct {94type Meta struct {
5495
=== 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 @@
417 c.Assert(input, DeepEquals, output)417 c.Assert(input, DeepEquals, output)
418 }418 }
419}419}
420
421var implementedByTests = []struct {
422 ifce string
423 name string
424 role charm.RelationRole
425 scope charm.RelationScope
426 match bool
427 implicit bool
428}{
429 {"ifce-pro", "pro", charm.RoleProvider, charm.ScopeGlobal, true, false},
430 {"blah", "pro", charm.RoleProvider, charm.ScopeGlobal, false, false},
431 {"ifce-pro", "blah", charm.RoleProvider, charm.ScopeGlobal, false, false},
432 {"ifce-pro", "pro", charm.RoleRequirer, charm.ScopeGlobal, false, false},
433 {"ifce-pro", "pro", charm.RoleProvider, charm.ScopeContainer, true, false},
434
435 {"juju-info", "juju-info", charm.RoleProvider, charm.ScopeGlobal, true, true},
436 {"blah", "juju-info", charm.RoleProvider, charm.ScopeGlobal, false, false},
437 {"juju-info", "blah", charm.RoleProvider, charm.ScopeGlobal, false, false},
438 {"juju-info", "juju-info", charm.RoleRequirer, charm.ScopeGlobal, false, false},
439 {"juju-info", "juju-info", charm.RoleProvider, charm.ScopeContainer, true, true},
440
441 {"ifce-req", "req", charm.RoleRequirer, charm.ScopeGlobal, true, false},
442 {"blah", "req", charm.RoleRequirer, charm.ScopeGlobal, false, false},
443 {"ifce-req", "blah", charm.RoleRequirer, charm.ScopeGlobal, false, false},
444 {"ifce-req", "req", charm.RolePeer, charm.ScopeGlobal, false, false},
445 {"ifce-req", "req", charm.RoleRequirer, charm.ScopeContainer, true, false},
446
447 {"juju-info", "info", charm.RoleRequirer, charm.ScopeContainer, true, false},
448 {"blah", "info", charm.RoleRequirer, charm.ScopeContainer, false, false},
449 {"juju-info", "blah", charm.RoleRequirer, charm.ScopeContainer, false, false},
450 {"juju-info", "info", charm.RolePeer, charm.ScopeContainer, false, false},
451 {"juju-info", "info", charm.RoleRequirer, charm.ScopeGlobal, false, false},
452
453 {"ifce-peer", "peer", charm.RolePeer, charm.ScopeGlobal, true, false},
454 {"blah", "peer", charm.RolePeer, charm.ScopeGlobal, false, false},
455 {"ifce-peer", "blah", charm.RolePeer, charm.ScopeGlobal, false, false},
456 {"ifce-peer", "peer", charm.RoleProvider, charm.ScopeGlobal, false, false},
457 {"ifce-peer", "peer", charm.RolePeer, charm.ScopeContainer, true, false},
458}
459
460func (s *MetaSuite) TestImplementedBy(c *C) {
461 for i, t := range implementedByTests {
462 c.Logf("test %d", i)
463 r := charm.Relation{
464 Interface: t.ifce,
465 Name: t.name,
466 Role: t.role,
467 Scope: t.scope,
468 }
469 c.Assert(r.ImplementedBy(&dummyCharm{}), Equals, t.match)
470 c.Assert(r.IsImplicit(), Equals, t.implicit)
471 }
472}
473
474type dummyCharm struct{}
475
476func (c *dummyCharm) Config() *charm.Config {
477 panic("unused")
478}
479
480func (c *dummyCharm) Revision() int {
481 panic("unused")
482}
483
484func (c *dummyCharm) Meta() *charm.Meta {
485 return &charm.Meta{
486 Provides: map[string]charm.Relation{
487 "pro": {Interface: "ifce-pro", Scope: charm.ScopeGlobal},
488 },
489 Requires: map[string]charm.Relation{
490 "req": {Interface: "ifce-req", Scope: charm.ScopeGlobal},
491 "info": {Interface: "juju-info", Scope: charm.ScopeContainer},
492 },
493 Peers: map[string]charm.Relation{
494 "peer": {Interface: "ifce-peer", Scope: charm.ScopeGlobal},
495 },
496 }
497}
420498
=== 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 @@
46 counterpartRole(ep.Role) == other.Role46 counterpartRole(ep.Role) == other.Role
47}47}
4848
49// ImplementedBy returns whether the endpoint is implemented by the supplied charm.
50func (ep Endpoint) ImplementedBy(ch charm.Charm) bool {
51 if ep.IsImplicit() {
52 return true
53 }
54 var m map[string]charm.Relation
55 switch ep.Role {
56 case charm.RoleProvider:
57 m = ch.Meta().Provides
58 case charm.RoleRequirer:
59 m = ch.Meta().Requires
60 case charm.RolePeer:
61 m = ch.Meta().Peers
62 default:
63 panic(fmt.Errorf("unknown relation role %q", ep.Role))
64 }
65 rel, found := m[ep.Name]
66 if !found {
67 return false
68 }
69 if rel.Interface == ep.Interface {
70 switch ep.Scope {
71 case charm.ScopeGlobal:
72 return rel.Scope != charm.ScopeContainer
73 case charm.ScopeContainer:
74 return true
75 default:
76 panic(fmt.Errorf("unknown relation scope %q", ep.Scope))
77 }
78 }
79 return false
80}
81
82// IsImplicit returns whether the endpoint is supplied by juju itself,
83// rather than by a charm.
84func (ep Endpoint) IsImplicit() bool {
85 return (ep.Name == "juju-info" &&
86 ep.Interface == "juju-info" &&
87 ep.Role == charm.RoleProvider)
88}
89
90type epSlice []Endpoint49type epSlice []Endpoint
9150
92var roleOrder = map[charm.RelationRole]int{51var roleOrder = map[charm.RelationRole]int{
9352
=== 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 @@
77 c.Assert(ep1.CanRelateTo(ep2), Equals, false)77 c.Assert(ep1.CanRelateTo(ep2), Equals, false)
78 c.Assert(ep2.CanRelateTo(ep1), Equals, false)78 c.Assert(ep2.CanRelateTo(ep1), Equals, false)
79}79}
80
81type dummyCharm struct{}
82
83func (c *dummyCharm) Config() *charm.Config {
84 panic("unused")
85}
86
87func (c *dummyCharm) Revision() int {
88 panic("unused")
89}
90
91func (c *dummyCharm) Meta() *charm.Meta {
92 return &charm.Meta{
93 Provides: map[string]charm.Relation{
94 "pro": {Interface: "ifce-pro", Scope: charm.ScopeGlobal},
95 },
96 Requires: map[string]charm.Relation{
97 "req": {Interface: "ifce-req", Scope: charm.ScopeGlobal},
98 "info": {Interface: "juju-info", Scope: charm.ScopeContainer},
99 },
100 Peers: map[string]charm.Relation{
101 "peer": {Interface: "ifce-peer", Scope: charm.ScopeGlobal},
102 },
103 }
104}
105
106var implementedByTests = []struct {
107 ifce string
108 name string
109 role charm.RelationRole
110 scope charm.RelationScope
111 match bool
112 implicit bool
113}{
114 {"ifce-pro", "pro", charm.RoleProvider, charm.ScopeGlobal, true, false},
115 {"blah", "pro", charm.RoleProvider, charm.ScopeGlobal, false, false},
116 {"ifce-pro", "blah", charm.RoleProvider, charm.ScopeGlobal, false, false},
117 {"ifce-pro", "pro", charm.RoleRequirer, charm.ScopeGlobal, false, false},
118 {"ifce-pro", "pro", charm.RoleProvider, charm.ScopeContainer, true, false},
119
120 {"juju-info", "juju-info", charm.RoleProvider, charm.ScopeGlobal, true, true},
121 {"blah", "juju-info", charm.RoleProvider, charm.ScopeGlobal, false, false},
122 {"juju-info", "blah", charm.RoleProvider, charm.ScopeGlobal, false, false},
123 {"juju-info", "juju-info", charm.RoleRequirer, charm.ScopeGlobal, false, false},
124 {"juju-info", "juju-info", charm.RoleProvider, charm.ScopeContainer, true, true},
125
126 {"ifce-req", "req", charm.RoleRequirer, charm.ScopeGlobal, true, false},
127 {"blah", "req", charm.RoleRequirer, charm.ScopeGlobal, false, false},
128 {"ifce-req", "blah", charm.RoleRequirer, charm.ScopeGlobal, false, false},
129 {"ifce-req", "req", charm.RolePeer, charm.ScopeGlobal, false, false},
130 {"ifce-req", "req", charm.RoleRequirer, charm.ScopeContainer, true, false},
131
132 {"juju-info", "info", charm.RoleRequirer, charm.ScopeContainer, true, false},
133 {"blah", "info", charm.RoleRequirer, charm.ScopeContainer, false, false},
134 {"juju-info", "blah", charm.RoleRequirer, charm.ScopeContainer, false, false},
135 {"juju-info", "info", charm.RolePeer, charm.ScopeContainer, false, false},
136 {"juju-info", "info", charm.RoleRequirer, charm.ScopeGlobal, false, false},
137
138 {"ifce-peer", "peer", charm.RolePeer, charm.ScopeGlobal, true, false},
139 {"blah", "peer", charm.RolePeer, charm.ScopeGlobal, false, false},
140 {"ifce-peer", "blah", charm.RolePeer, charm.ScopeGlobal, false, false},
141 {"ifce-peer", "peer", charm.RoleProvider, charm.ScopeGlobal, false, false},
142 {"ifce-peer", "peer", charm.RolePeer, charm.ScopeContainer, true, false},
143}
144
145func (s *EndpointSuite) TestImplementedBy(c *C) {
146 for i, t := range implementedByTests {
147 c.Logf("test %d", i)
148 ep := state.Endpoint{
149 ServiceName: "x",
150 Relation: charm.Relation{
151 Interface: t.ifce,
152 Name: t.name,
153 Role: t.role,
154 Scope: t.scope,
155 },
156 }
157 c.Assert(ep.ImplementedBy(&dummyCharm{}), Equals, t.match)
158 c.Assert(ep.IsImplicit(), Equals, t.implicit)
159 }
160}

Subscribers

People subscribed via source and target branches

to status/vote changes: