state;api: Implement RelationUnitsWatcher in API

state;api: Implement RelationUnitsWatcher in API

This implements the last needed bit in the server-side
uniter API: RelationUnitsWatcher. There are a few steps
needed before the actual implementation:
1. Move RelationUnitsChange and UnitSettings from
state to params.
2. Unexport the concrete state.RelationUnitsWatcher
and define a RelationUnitsWatcher as interface, as
with the other watchers.
3. Implement state/testing/RelationUnitsWatcherC.

Finally, UniterAPI.WatchRelationUnits() as implemented.

A few drive-by fixes were done as well:
1. Changed state/testing/watcher.go's gocheck import
and imports order to match the other files.
2. Replaced state/relationunit_test.go's local asserts
with the statetesting's RelationUnitsWatcherC's ones.
3. Added a test in RelationUnitsWatcherC.AssertChange
a check to ensure unit settings versions always increase
on a change (we're not checking their exact values in
other tests becasue they're volatile; we just check the
Changed field of the RelationUnitsChange contains what
we expected).

In a follow-up, the client-side API RelationUnitsWatcher
will be implemented.

https://code.launchpad.net/~dimitern/juju-core/119-apiserver-relationunitswatcher/+merge/183856

(do not edit description out of merge proposal)

[dimitern, 2013-09-04 11:40:48]

Please take a look.

[fwereade, 2013-09-04 13:30:24]

Missing a test, but LGTM otherwise. TYVM.

https://codereview.appspot.com/13523043/diff/1/state/apiserver/uniter/uniter.go
File state/apiserver/uniter/uniter.go (right):

https://codereview.appspot.com/13523043/diff/1/state/apiserver/uniter/uniter....
state/apiserver/uniter/uniter.go:881: // changes to each given pair or unit and
relation. See also
changes to every unit in the supplied relation that is visible to the supplied
unit?

https://codereview.appspot.com/13523043/diff/1/state/apiserver/uniter/uniter_...
File state/apiserver/uniter/uniter_test.go (right):

https://codereview.appspot.com/13523043/diff/1/state/apiserver/uniter/uniter_...
state/apiserver/uniter/uniter_test.go:1272: wc.AssertNoChange()
Needs just one more test here, in which we make a change and check it's
reported.

[dimitern, 2013-09-04 15:49:43]

Please take a look.

https://codereview.appspot.com/13523043/diff/1/state/apiserver/uniter/uniter.go
File state/apiserver/uniter/uniter.go (right):

https://codereview.appspot.com/13523043/diff/1/state/apiserver/uniter/uniter....
state/apiserver/uniter/uniter.go:881: // changes to each given pair or unit and
relation. See also
On 2013/09/04 13:30:24, fwereade wrote:
> changes to every unit in the supplied relation that is visible to the supplied
> unit?

Done.

https://codereview.appspot.com/13523043/diff/1/state/apiserver/uniter/uniter_...
File state/apiserver/uniter/uniter_test.go (right):

https://codereview.appspot.com/13523043/diff/1/state/apiserver/uniter/uniter_...
state/apiserver/uniter/uniter_test.go:1272: wc.AssertNoChange()
On 2013/09/04 13:30:24, fwereade wrote:
> Needs just one more test here, in which we make a change and check it's
> reported.

Done.

[fwereade, 2013-09-04 16:12:53]

I think we can manage a couple of little simplifications -- if they make sense
to you, LGTM with those included.

https://codereview.appspot.com/13523043/diff/7001/state/settings.go
File state/settings.go (right):

https://codereview.appspot.com/13523043/diff/7001/state/settings.go#newcode108
state/settings.go:108: return params.UnitSettings{c.txnRevno}
This seems a bit weird to me.

https://codereview.appspot.com/13523043/diff/7001/state/testing/watcher.go
File state/testing/watcher.go (right):

https://codereview.appspot.com/13523043/diff/7001/state/testing/watcher.go#ne...
state/testing/watcher.go:218: func (c RelationUnitsWatcherC)
AssertChange(changed map[string]params.UnitSettings, departed []string) {
Shouldn't changed just be an []string now?

[dimitern, 2013-09-04 16:46:57]

Please take a look.

https://codereview.appspot.com/13523043/diff/7001/state/settings.go
File state/settings.go (right):

https://codereview.appspot.com/13523043/diff/7001/state/settings.go#newcode108
state/settings.go:108: return params.UnitSettings{c.txnRevno}
On 2013/09/04 16:12:53, fwereade wrote:
> This seems a bit weird to me.

Yeah, it is - I tried to do a bit of transition from using s.Map() in tests to
this, but as you suggested we can just have (changed []string, departed
[]string) and don't care about the settings version.

https://codereview.appspot.com/13523043/diff/7001/state/testing/watcher.go
File state/testing/watcher.go (right):

https://codereview.appspot.com/13523043/diff/7001/state/testing/watcher.go#ne...
state/testing/watcher.go:218: func (c RelationUnitsWatcherC)
AssertChange(changed map[string]params.UnitSettings, departed []string) {
On 2013/09/04 16:12:53, fwereade wrote:
> Shouldn't changed just be an []string now?

Done.