Merge pull request #103 from davecheney/106-parsetag-refactor

Update for juju/names refactor part 3

* Replace all the calls to ParseTag(tag, hint) to a specific ParseXXXTag method. 
* Where ParseTag is not called with a hint, adjust as necessary, there is one call for this in the codebase.
* Add some grossness where code like

type Unit struct { tag string }
func (u *Unit) Name() { _, id, err := names.ParseTag(tag, names.UnitTagKind); if err != nil { panic(err) } }

Into something a little more explicit using type specific ParseXXXTag methods.

In the next branch rather than storing the string form of the tag inside the Unit structure (without validation I may add), then parsing it (and possibly panicing) to get its Id, store a real Tag value in the struct then calls to name can just return the Id of the tag (or a more specific value available on the Tag's implementation type).

dependencies.tsv

@@ -13,7 +13,7 @@ github.com/juju/cmd	git	e2d7df426d54b663f8616901b75d1ccfafcd9997
 github.com/juju/errgo	git	26caa7d4884f6048c5687fa44c410e5d5d95e554	
 github.com/juju/errors	git	075df0417dbcc39d24ee18248d2f8d6e3eed598b	
 github.com/juju/loggo	git	7e8c70b24b80b95b2284f09306aac0bd93588db8	
-github.com/juju/names	git	973fc2b9bc21d975e5c7ccdc273d1169ba55c53d	
+github.com/juju/names	git	556cb4237985ba1c986c79822533026abfef2788	
 github.com/juju/ratelimit	git	0025ab75db6c6eaa4ffff0240c2c9e617ad1a0eb	
 github.com/juju/schema	git	1887e824896b58a0f376fb8517b5eebb825828b8	
 github.com/juju/testing	git	b0941ff1bf4d3db1ffbbe09f75fa8383eae5764c	

juju/api.go

@@ -355,7 +355,7 @@ func cacheAPIInfo(info configstore.EnvironInfo, apiInfo *api.Info) (err error) {
 	defer errors.Contextf(&err, "failed to cache API credentials")
 	environUUID := ""
 	if apiInfo.EnvironTag != "" {
-		tag, err := names.ParseTag(apiInfo.Tag, names.EnvironTagKind)
+		tag, err := names.ParseEnvironTag(apiInfo.Tag)
 		if err != nil {
 			return err
 		}
@@ -366,7 +366,7 @@ func cacheAPIInfo(info configstore.EnvironInfo, apiInfo *api.Info) (err error) {
 		CACert:      string(apiInfo.CACert),
 		EnvironUUID: environUUID,
 	})
-	tag, err := names.ParseTag(apiInfo.Tag, names.UserTagKind)
+	tag, err := names.ParseUserTag(apiInfo.Tag)
 	if err != nil {
 		return err
 	}
@@ -395,7 +395,7 @@ func cacheChangedAPIInfo(info configstore.EnvironInfo, st apiState) error {
 	newEnvironTag := st.EnvironTag()
 	changed := false
 	if newEnvironTag != "" {
-		tag, err := names.ParseTag(newEnvironTag, names.EnvironTagKind)
+		tag, err := names.ParseEnvironTag(newEnvironTag)
 		if err == nil {
 			if environUUID := tag.Id(); endpoint.EnvironUUID != environUUID {
 				changed = true

state/api/apiclient.go

@@ -129,9 +129,9 @@ func Open(info *Info, opts DialOpts) (*State, error) {
 	}
 	pool.AddCert(xcert)
 
-	environUUID := ""
+	var environUUID string
 	if info.EnvironTag != "" {
-		tag, err := names.ParseTag(info.EnvironTag, names.EnvironTagKind)
+		tag, err := names.ParseEnvironTag(info.EnvironTag)
 		if err != nil {
 			return nil, err
 		}

state/api/deployer/unit.go

@@ -23,11 +23,15 @@ func (u *Unit) Tag() string {
 
 // Name returns the unit's name.
 func (u *Unit) Name() string {
-	tag, err := names.ParseTag(u.tag, names.UnitTagKind)
+	return mustParseUnitTag(u.tag).Id()
+}
+
+func mustParseUnitTag(unitTag string) names.UnitTag {
+	tag, err := names.ParseUnitTag(unitTag)
 	if err != nil {
 		panic(err)
 	}
-	return tag.Id()
+	return tag
 }
 
 // Life returns the unit's lifecycle value.

state/api/firewaller/service.go

@@ -21,11 +21,15 @@ type Service struct {
 
 // Name returns the service name.
 func (s *Service) Name() string {
-	tag, err := names.ParseTag(s.tag, names.ServiceTagKind)
+	return mustParseServiceTag(s.tag).Id()
+}
+
+func mustParseServiceTag(serviceTag string) names.ServiceTag {
+	tag, err := names.ParseServiceTag(serviceTag)
 	if err != nil {
-		panic(fmt.Sprintf("%q is not a valid service tag", s.tag))
+		panic(err)
 	}
-	return tag.Id()
+	return tag
 }
 
 // Watch returns a watcher for observing changes to a service.

state/api/firewaller/unit.go

@@ -22,11 +22,15 @@ type Unit struct {
 
 // Name returns the name of the unit.
 func (u *Unit) Name() string {
-	tag, err := names.ParseTag(u.tag, names.UnitTagKind)
+	return mustParseUnitTag(u.tag).Id()
+}
+
+func mustParseUnitTag(unitTag string) names.UnitTag {
+	tag, err := names.ParseUnitTag(unitTag)
 	if err != nil {
-		panic(fmt.Sprintf("%q is not a valid unit tag", u.tag))
+		panic(err)
 	}
-	return tag.Id()
+	return tag
 }
 
 // Life returns the unit's life cycle value.

state/api/provisioner/machine.go

@@ -27,11 +27,15 @@ func (m *Machine) Tag() string {
 
 // Id returns the machine id.
 func (m *Machine) Id() string {
-	tag, err := names.ParseTag(m.tag, names.MachineTagKind)
+	return mustParseMachineTag(m.tag).Id()
+}
+
+func mustParseMachineTag(machineTag string) names.MachineTag {
+	tag, err := names.ParseMachineTag(machineTag)
 	if err != nil {
-		panic(fmt.Sprintf("%q is not a valid machine tag", m.tag))
+		panic(err)
 	}
-	return tag.Id()
+	return tag
 }
 
 // String returns the machine as a string.

state/api/provisioner/provisioner_test.go

@@ -299,7 +299,7 @@ func (s *provisionerSuite) TestSetInstanceInfo(c *gc.C) {
 			// Last one was ignored, so skip it.
 			break
 		}
-		tag, err := names.ParseTag(networks[i].Tag, names.NetworkTagKind)
+		tag, err := names.ParseNetworkTag(networks[i].Tag)
 		c.Assert(err, gc.IsNil)
 		networkName := tag.Id()
 		network, err := s.State.Network(networkName)

state/api/uniter/relation.go

@@ -25,11 +25,15 @@ type Relation struct {
 
 // String returns the relation as a string.
 func (r *Relation) String() string {
-	tag, err := names.ParseTag(r.tag, names.RelationTagKind)
+	return mustParseRelationTag(r.tag).Id()
+}
+
+func mustParseRelationTag(relationTag string) names.RelationTag {
+	tag, err := names.ParseRelationTag(relationTag)
 	if err != nil {
-		panic(fmt.Sprintf("%q is not a valid relation tag", r.tag))
+		panic(err)
 	}
-	return tag.Id()
+	return tag
 }
 
 // Id returns the integer internal relation key. This is exposed

state/api/uniter/service.go

@@ -25,11 +25,15 @@ type Service struct {
 
 // Name returns the service name.
 func (s *Service) Name() string {
-	tag, err := names.ParseTag(s.tag, names.ServiceTagKind)
+	return mustParseServiceTag(s.tag).Id()
+}
+
+func mustParseServiceTag(serviceTag string) names.ServiceTag {
+	tag, err := names.ParseServiceTag(serviceTag)
 	if err != nil {
-		panic(fmt.Sprintf("%q is not a valid service tag", s.tag))
+		panic(err)
 	}
-	return tag.Id()
+	return tag
 }
 
 // String returns the service as a string.

state/api/uniter/unit.go

@@ -28,11 +28,15 @@ func (u *Unit) Tag() string {
 
 // Name returns the name of the unit.
 func (u *Unit) Name() string {
-	tag, err := names.ParseTag(u.tag, names.UnitTagKind)
+	return mustParseUnitTag(u.tag).Id()
+}
+
+func mustParseUnitTag(unitTag string) names.UnitTag {
+	tag, err := names.ParseUnitTag(unitTag)
 	if err != nil {
-		panic(fmt.Sprintf("%q is not a valid unit tag", u.tag))
+		panic(err)
 	}
-	return tag.Id()
+	return tag
 }
 
 // String returns the unit as a string.

state/apiserver/client/client.go

@@ -238,7 +238,7 @@ var CharmStore charm.Repository = charm.Store
 func networkTagsToNames(tags []string) ([]string, error) {
 	netNames := make([]string, len(tags))
 	for i, tag := range tags {
-		t, err := names.ParseTag(tag, names.NetworkTagKind)
+		t, err := names.ParseNetworkTag(tag)
 		if err != nil {
 			return nil, err
 		}

state/apiserver/client/client_test.go

@@ -694,7 +694,7 @@ func (s *clientSuite) TestClientServiceDeployWithNetworks(c *gc.C) {
 		curl.String(), "service", 3, "", cons, "",
 		[]string{"net1", "net2"},
 	)
-	c.Assert(err, gc.ErrorMatches, `"net1" is not a valid network tag`)
+	c.Assert(err, gc.ErrorMatches, `"net1" is not a valid tag`)
 
 	err = s.APIState.Client().ServiceDeployWithNetworks(
 		curl.String(), "service", 3, "", cons, "",

state/apiserver/deployer/deployer.go

@@ -85,7 +85,7 @@ func (d *DeployerAPI) ConnectionInfo() (result params.DeployerConnectionValues,
 // getAllUnits returns a list of all principal and subordinate units
 // assigned to the given machine.
 func getAllUnits(st *state.State, machineTag string) ([]string, error) {
-	tag, err := names.ParseTag(machineTag, names.MachineTagKind)
+	tag, err := names.ParseMachineTag(machineTag)
 	if err != nil {
 		return nil, err
 	}

state/apiserver/firewaller/firewaller.go

@@ -199,8 +199,8 @@ func getAuthFuncForTagKind(kind string) common.GetAuthFunc {
 				return kind == ""
 			}
 			// Allow only the given tag kind.
-			_, err := names.ParseTag(tag, kind)
-			return err == nil
+			t, err := names.ParseTag(tag)
+			return err == nil && t.Kind() == kind
 		}, nil
 	}
 }

state/apiserver/httphandler.go

@@ -45,7 +45,7 @@ func (h *httpHandler) authenticate(r *http.Request) error {
 		return fmt.Errorf("invalid request format")
 	}
 	// Only allow users, not agents.
-	if _, err := names.ParseTag(tagPass[0], names.UserTagKind); err != nil {
+	if _, err := names.ParseUserTag(tagPass[0]); err != nil {
 		return common.ErrBadCreds
 	}
 	// Ensure the credentials are correct.

state/apiserver/keymanager/keymanager.go

@@ -66,7 +66,7 @@ func NewKeyManagerAPI(
 		return func(tag string) bool {
 			// Are we a machine agent writing the Juju system key.
 			if tag == config.JujuSystemKey {
-				_, err := names.ParseTag(authorizer.GetAuthTag(), names.MachineTagKind)
+				_, err := names.ParseMachineTag(authorizer.GetAuthTag())
 				return err == nil
 			}
 			// Are we writing the auth key for a user.

state/apiserver/networker/networker.go

@@ -39,7 +39,7 @@ func NewNetworkerAPI(
 				// A machine agent can always access its own machine.
 				return true
 			}
-			t, err := names.ParseTag(tag, names.MachineTagKind)
+			t, err := names.ParseMachineTag(tag)
 			if err != nil {
 				// Only machine tags are allowed.
 				return false
@@ -106,7 +106,7 @@ func (n *NetworkerAPI) MachineNetworkInfo(args params.Entities) (params.MachineN
 		if !canAccess(entity.Tag) {
 			err = common.ErrPerm
 		} else {
-			tag, err = names.ParseTag(entity.Tag, names.MachineTagKind)
+			tag, err = names.ParseMachineTag(entity.Tag)
 			if err == nil {
 				id := tag.Id()
 				result.Results[i].Info, err = n.oneMachineInfo(id)

state/apiserver/provisioner/provisioner.go

@@ -58,7 +58,7 @@ func NewProvisionerAPI(
 				// A machine agent can always access its own machine.
 				return true
 			}
-			t, err := names.ParseTag(tag, names.MachineTagKind)
+			t, err := names.ParseMachineTag(tag)
 			if err != nil {
 				return false
 			}
@@ -119,7 +119,7 @@ func (p *ProvisionerAPI) watchOneMachineContainers(arg params.WatchContainer) (p
 	if !canAccess(arg.MachineTag) {
 		return nothing, common.ErrPerm
 	}
-	tag, err := names.ParseTag(arg.MachineTag, names.MachineTagKind)
+	tag, err := names.ParseMachineTag(arg.MachineTag)
 	if err != nil {
 		return nothing, err
 	}
@@ -460,7 +460,7 @@ func networkParamsToStateParams(networks []params.Network, ifaces []params.Netwo
 ) {
 	stateNetworks := make([]state.NetworkInfo, len(networks))
 	for i, network := range networks {
-		tag, err := names.ParseTag(network.Tag, names.NetworkTagKind)
+		tag, err := names.ParseNetworkTag(network.Tag)
 		if err != nil {
 			return nil, nil, err
 		}
@@ -473,7 +473,7 @@ func networkParamsToStateParams(networks []params.Network, ifaces []params.Netwo
 	}
 	stateInterfaces := make([]state.NetworkInterfaceInfo, len(ifaces))
 	for i, iface := range ifaces {
-		tag, err := names.ParseTag(iface.NetworkTag, names.NetworkTagKind)
+		tag, err := names.ParseNetworkTag(iface.NetworkTag)
 		if err != nil {
 			return nil, nil, err
 		}

state/apiserver/provisioner/provisioner_test.go

@@ -1055,7 +1055,7 @@ func (s *withoutStateServerSuite) TestSetInstanceInfo(c *gc.C) {
 			// Last one was ignored, so don't check.
 			break
 		}
-		tag, err := names.ParseTag(networks[i].Tag, names.NetworkTagKind)
+		tag, err := names.ParseNetworkTag(networks[i].Tag)
 		c.Assert(err, gc.IsNil)
 		networkName := tag.Id()
 		network, err := s.State.Network(networkName)

state/apiserver/root.go

@@ -243,7 +243,7 @@ func (r *srvRoot) Upgrader(id string) (upgrader.Upgrader, error) {
 	// Machines get an UpgraderAPI, units get a UnitUpgraderAPI.
 	// This is tested in the state/api/upgrader package since there
 	// are currently no direct srvRoot tests.
-	tag, err := names.ParseTag(r.GetAuthTag(), "")
+	tag, err := names.ParseTag(r.GetAuthTag())
 	if err != nil {
 		return nil, common.ErrPerm
 	}

state/apiserver/uniter/uniter.go

@@ -73,15 +73,15 @@ func NewUniterAPI(st *state.State, resources *common.Resources, authorizer commo
 }
 
 func (u *UniterAPI) getUnit(tag string) (*state.Unit, error) {
-	t, err := names.ParseTag(tag, names.UnitTagKind)
+	t, err := names.ParseUnitTag(tag)
 	if err != nil {
 		return nil, err
 	}
 	return u.st.Unit(t.Id())
 }
 
 func (u *UniterAPI) getService(tag string) (*state.Service, error) {
-	t, err := names.ParseTag(tag, names.ServiceTagKind)
+	t, err := names.ParseServiceTag(tag)
 	if err != nil {
 		return nil, err
 	}
@@ -580,7 +580,7 @@ func (u *UniterAPI) CharmArchiveSha256(args params.CharmURLs) (params.StringResu
 }
 
 func (u *UniterAPI) getRelationAndUnit(canAccess common.AuthFunc, relTag, unitTag string) (*state.Relation, *state.Unit, error) {
-	tag, err := names.ParseTag(relTag, names.RelationTagKind)
+	tag, err := names.ParseRelationTag(relTag)
 	if err != nil {
 		return nil, nil, common.ErrPerm
 	}
@@ -864,7 +864,7 @@ func (u *UniterAPI) checkRemoteUnit(relUnit *state.RelationUnit, remoteUnitTag s
 	// the *Unit, because it might have been removed; but its relation settings will
 	// persist until the relation itself has been removed (and must remain accessible
 	// because the local unit's view of reality may be time-shifted).
-	tag, err := names.ParseTag(remoteUnitTag, names.UnitTagKind)
+	tag, err := names.ParseUnitTag(remoteUnitTag)
 	if err != nil {
 		return "", err
 	}

state/apiserver/upgrader/unitupgrader.go

@@ -115,7 +115,7 @@ func (u *UnitUpgraderAPI) Tools(args params.Entities) (params.ToolsResults, erro
 
 func (u *UnitUpgraderAPI) getAssignedMachine(tag string) (*state.Machine, error) {
 	// Check that we really have a unit tag.
-	t, err := names.ParseTag(tag, names.UnitTagKind)
+	t, err := names.ParseUnitTag(tag)
 	if err != nil {
 		return nil, common.ErrPerm
 	}

state/apiserver/usermanager/usermanager.go

@@ -135,7 +135,7 @@ func (api *UserManagerAPI) UserInfo(args params.Entities) (params.UserInfoResult
 			results.Results[i].Error = common.ServerError(common.ErrPerm)
 			continue
 		}
-		tag, err := names.ParseTag(userArg.Tag, names.UserTagKind)
+		tag, err := names.ParseUserTag(userArg.Tag)
 		if err != nil {
 			results.Results[0].Error = common.ServerError(err)
 			continue

state/apiserver/usermanager/usermanager_test.go

@@ -252,7 +252,7 @@ func (s *userManagerSuite) TestUserInfoNotATagFails(c *gc.C) {
 			{
 				Result: nil,
 				Error: &params.Error{
-					Message: `"notatag" is not a valid user tag`,
+					Message: `"notatag" is not a valid tag`,
 					Code:    "",
 				},
 			},