From 9be283968ce5a214d6fe7a99851e3aaa6a8df6e1 Mon Sep 17 00:00:00 2001
From: waigani <waigani@gmail.com>
Date: Tue, 9 Sep 2014 14:40:15 +1200
Subject: [PATCH] state/service: GetOwnerTag return user tag

Update service.GetOwnerTag() to return a names.UserTag instead of a
string.
---
 apiserver/client/client_test.go |  4 +++-
 apiserver/uniter/uniter.go      |  6 +++++-
 juju/deploy_test.go             | 10 +++++++---
 state/service.go                | 10 +++++++---
 state/service_test.go           |  4 +++-
 testing/factory/factory_test.go |  4 +++-
 6 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/apiserver/client/client_test.go b/apiserver/client/client_test.go
index 13416413e9e..643a69c5904 100644
--- a/apiserver/client/client_test.go
+++ b/apiserver/client/client_test.go
@@ -893,7 +893,9 @@ func (s *clientSuite) TestClientServiceDeployServiceOwner(c *gc.C) {
 
 	service, err := s.State.Service("service")
 	c.Assert(err, gc.IsNil)
-	c.Assert(service.GetOwnerTag(), gc.Equals, user.Tag().String())
+	tag, err := service.GetOwnerTag()
+	c.Assert(err, gc.IsNil)
+	c.Assert(tag, gc.Equals, user.Tag())
 }
 
 func (s *clientSuite) deployServiceForTests(c *gc.C, store *charmtesting.MockCharmStore) {
diff --git a/apiserver/uniter/uniter.go b/apiserver/uniter/uniter.go
index d8b81232f8c..805f8de3028 100644
--- a/apiserver/uniter/uniter.go
+++ b/apiserver/uniter/uniter.go
@@ -1250,8 +1250,12 @@ func (u *UniterAPI) GetOwnerTag(args params.Entities) (params.StringResult, erro
 	if err != nil {
 		return nothing, err
 	}
+	ownerTag, err := service.GetOwnerTag()
+	if err != nil {
+		return params.StringResult{}, errors.Trace(err)
+	}
 	return params.StringResult{
-		Result: service.GetOwnerTag(),
+		Result: ownerTag.String(),
 	}, nil
 }
 
diff --git a/juju/deploy_test.go b/juju/deploy_test.go
index b99c944f55f..58b34505684 100644
--- a/juju/deploy_test.go
+++ b/juju/deploy_test.go
@@ -69,11 +69,13 @@ func (s *DeployLocalSuite) TestDeployMinimal(c *gc.C) {
 	s.assertSettings(c, service, charm.Settings{})
 	s.assertConstraints(c, service, constraints.Value{})
 	s.assertMachines(c, service, constraints.Value{})
-	c.Assert(service.GetOwnerTag(), gc.Equals, s.AdminUserTag(c).String())
+	tag, err := service.GetOwnerTag()
+	c.Assert(err, gc.IsNil)
+	c.Assert(tag, gc.Equals, s.AdminUserTag(c))
 }
 
 func (s *DeployLocalSuite) TestDeployOwnerTag(c *gc.C) {
-	s.Factory.MakeUser(c, &factory.UserParams{Name: "foobar"})
+	user := s.Factory.MakeUser(c, &factory.UserParams{Name: "foobar"})
 	service, err := juju.DeployService(s.State,
 		juju.DeployServiceParams{
 			ServiceName:  "bobwithowner",
@@ -81,7 +83,9 @@ func (s *DeployLocalSuite) TestDeployOwnerTag(c *gc.C) {
 			ServiceOwner: "user-foobar",
 		})
 	c.Assert(err, gc.IsNil)
-	c.Assert(service.GetOwnerTag(), gc.Equals, "user-foobar")
+	tag, err := service.GetOwnerTag()
+	c.Assert(err, gc.IsNil)
+	c.Assert(tag, gc.Equals, user.Tag())
 }
 
 func (s *DeployLocalSuite) TestDeploySettings(c *gc.C) {
diff --git a/state/service.go b/state/service.go
index a0e7cbcc6fa..db602620e29 100644
--- a/state/service.go
+++ b/state/service.go
@@ -620,14 +620,18 @@ func (s *Service) addUnitOps(principalName string, asserts bson.D) (string, []tx
 
 // SCHEMACHANGE
 // TODO(mattyw) remove when schema upgrades are possible
-func (s *Service) GetOwnerTag() string {
+func (s *Service) GetOwnerTag() (names.UserTag, error) {
 	owner := s.doc.OwnerTag
 	if owner == "" {
 		// We know that if there was no owner, it was created with an early
 		// version of juju, and that admin was the only user.
-		owner = names.NewUserTag("admin").String()
+		return names.NewUserTag("admin"), nil
 	}
-	return owner
+	tag, err := names.ParseUserTag(s.doc.OwnerTag)
+	if err != nil {
+		return names.UserTag{}, errors.Trace(err)
+	}
+	return tag, nil
 }
 
 // AddUnit adds a new principal unit to the service.
diff --git a/state/service_test.go b/state/service_test.go
index 170a6584fa0..1ac4df0e28a 100644
--- a/state/service_test.go
+++ b/state/service_test.go
@@ -1484,7 +1484,9 @@ func (s *ServiceSuite) TestOwnerTagSchemaProtection(c *gc.C) {
 	service := s.AddTestingService(c, "foobar", s.charm)
 	state.SetServiceOwnerTag(service, "")
 	c.Assert(state.GetServiceOwnerTag(service), gc.Equals, "")
-	c.Assert(service.GetOwnerTag(), gc.Equals, "user-admin")
+	tag, err := service.GetOwnerTag()
+	c.Assert(err, gc.IsNil)
+	c.Assert(tag, gc.Equals, s.owner)
 }
 
 func (s *ServiceSuite) TestNetworks(c *gc.C) {
diff --git a/testing/factory/factory_test.go b/testing/factory/factory_test.go
index a0c3805e61d..0d906ace27f 100644
--- a/testing/factory/factory_test.go
+++ b/testing/factory/factory_test.go
@@ -285,7 +285,9 @@ func (s *factorySuite) TestMakeService(c *gc.C) {
 	c.Assert(service, gc.NotNil)
 
 	c.Assert(service.Name(), gc.Equals, "wordpress")
-	c.Assert(service.GetOwnerTag(), gc.Equals, creator)
+	tag, err := service.GetOwnerTag()
+	c.Assert(err, gc.IsNil)
+	c.Assert(tag.String(), gc.Equals, creator)
 	curl, _ := service.CharmURL()
 	c.Assert(curl, gc.DeepEquals, charm.URL())