interfaces: validate plug/slot uniqueness

interfaces/repo.go

@@ -139,8 +139,10 @@ func (r *Repository) AddPlug(plug *Plug) error {
 	r.m.Lock()
 	defer r.m.Unlock()
 
+	snapName := plug.Snap.Name()
+
 	// Reject snaps with invalid names
-	if err := snap.ValidateName(plug.Snap.Name()); err != nil {
+	if err := snap.ValidateName(snapName); err != nil {
 		return err
 	}
 	// Reject plug with invalid names
@@ -155,13 +157,16 @@ func (r *Repository) AddPlug(plug *Plug) error {
 	if err := i.SanitizePlug(plug); err != nil {
 		return fmt.Errorf("cannot add plug: %v", err)
 	}
-	if _, ok := r.plugs[plug.Snap.Name()][plug.Name]; ok {
-		return fmt.Errorf("cannot add plug, snap %q already has plug %q", plug.Snap.Name(), plug.Name)
+	if _, ok := r.plugs[snapName][plug.Name]; ok {
+		return fmt.Errorf("snap %q has plugs conflicting on name %q", snapName, plug.Name)
+	}
+	if _, ok := r.slots[snapName][plug.Name]; ok {
+		return fmt.Errorf("snap %q has plug and slot conflicting on name %q", snapName, plug.Name)
 	}
-	if r.plugs[plug.Snap.Name()] == nil {
-		r.plugs[plug.Snap.Name()] = make(map[string]*Plug)
+	if r.plugs[snapName] == nil {
+		r.plugs[snapName] = make(map[string]*Plug)
 	}
-	r.plugs[plug.Snap.Name()][plug.Name] = plug
+	r.plugs[snapName][plug.Name] = plug
 	return nil
 }
 
@@ -233,8 +238,10 @@ func (r *Repository) AddSlot(slot *Slot) error {
 	r.m.Lock()
 	defer r.m.Unlock()
 
+	snapName := slot.Snap.Name()
+
 	// Reject snaps with invalid names
-	if err := snap.ValidateName(slot.Snap.Name()); err != nil {
+	if err := snap.ValidateName(snapName); err != nil {
 		return err
 	}
 	// Reject plug with invalid names
@@ -249,13 +256,16 @@ func (r *Repository) AddSlot(slot *Slot) error {
 	if err := i.SanitizeSlot(slot); err != nil {
 		return fmt.Errorf("cannot add slot: %v", err)
 	}
-	if _, ok := r.slots[slot.Snap.Name()][slot.Name]; ok {
-		return fmt.Errorf("cannot add slot, snap %q already has slot %q", slot.Snap.Name(), slot.Name)
+	if _, ok := r.slots[snapName][slot.Name]; ok {
+		return fmt.Errorf("snap %q has slots conflicting on name %q", snapName, slot.Name)
+	}
+	if _, ok := r.plugs[snapName][slot.Name]; ok {
+		return fmt.Errorf("snap %q has plug and slot conflicting on name %q", snapName, slot.Name)
 	}
-	if r.slots[slot.Snap.Name()] == nil {
-		r.slots[slot.Snap.Name()] = make(map[string]*Slot)
+	if r.slots[snapName] == nil {
+		r.slots[snapName] = make(map[string]*Slot)
 	}
-	r.slots[slot.Snap.Name()][slot.Name] = slot
+	r.slots[snapName][slot.Name] = slot
 	return nil
 }
 
@@ -742,6 +752,11 @@ func (e *BadInterfacesError) Error() string {
 // Unknown interfaces and plugs/slots that don't validate are not added.
 // Information about those failures are returned to the caller.
 func (r *Repository) AddSnap(snapInfo *snap.Info) error {
+	err := snap.Validate(snapInfo)
+	if err != nil {
+		return err
+	}
+
 	r.m.Lock()
 	defer r.m.Unlock()
 

interfaces/repo_test.go

@@ -190,15 +190,35 @@ func (s *RepositorySuite) TestAddPlug(c *C) {
 	c.Assert(s.testRepo.Plug(s.plug.Snap.Name(), s.plug.Name), DeepEquals, s.plug)
 }
 
-func (s *RepositorySuite) TestAddPlugClash(c *C) {
+func (s *RepositorySuite) TestAddPlugClashingPlug(c *C) {
 	err := s.testRepo.AddPlug(s.plug)
 	c.Assert(err, IsNil)
 	err = s.testRepo.AddPlug(s.plug)
-	c.Assert(err, ErrorMatches, `cannot add plug, snap "consumer" already has plug "plug"`)
+	c.Assert(err, ErrorMatches, `snap "consumer" has plugs conflicting on name "plug"`)
 	c.Assert(s.testRepo.AllPlugs(""), HasLen, 1)
 	c.Assert(s.testRepo.Plug(s.plug.Snap.Name(), s.plug.Name), DeepEquals, s.plug)
 }
 
+func (s *RepositorySuite) TestAddPlugClashingSlot(c *C) {
+	snapInfo := &snap.Info{SuggestedName: "snap"}
+	plug := &Plug{PlugInfo: &snap.PlugInfo{
+		Snap:      snapInfo,
+		Name:      "clashing",
+		Interface: "interface",
+	}}
+	slot := &Slot{SlotInfo: &snap.SlotInfo{
+		Snap:      snapInfo,
+		Name:      "clashing",
+		Interface: "interface",
+	}}
+	err := s.testRepo.AddSlot(slot)
+	c.Assert(err, IsNil)
+	err = s.testRepo.AddPlug(plug)
+	c.Assert(err, ErrorMatches, `snap "snap" has plug and slot conflicting on name "clashing"`)
+	c.Assert(s.testRepo.AllSlots(""), HasLen, 1)
+	c.Assert(s.testRepo.Slot(slot.Snap.Name(), slot.Name), DeepEquals, slot)
+}
+
 func (s *RepositorySuite) TestAddPlugFailsWithInvalidSnapName(c *C) {
 	plug := &Plug{
 		PlugInfo: &snap.PlugInfo{
@@ -477,13 +497,33 @@ func (s *RepositorySuite) TestAddSlotFailsWithInvalidSnapName(c *C) {
 	c.Assert(s.emptyRepo.AllSlots(""), HasLen, 0)
 }
 
-func (s *RepositorySuite) TestAddSlotFailsForDuplicates(c *C) {
+func (s *RepositorySuite) TestAddSlotClashingSlot(c *C) {
 	// Adding the first slot succeeds
 	err := s.testRepo.AddSlot(s.slot)
 	c.Assert(err, IsNil)
 	// Adding the slot again fails with appropriate error
 	err = s.testRepo.AddSlot(s.slot)
-	c.Assert(err, ErrorMatches, `cannot add slot, snap "producer" already has slot "slot"`)
+	c.Assert(err, ErrorMatches, `snap "producer" has slots conflicting on name "slot"`)
+}
+
+func (s *RepositorySuite) TestAddSlotClashingPlug(c *C) {
+	snapInfo := &snap.Info{SuggestedName: "snap"}
+	plug := &Plug{PlugInfo: &snap.PlugInfo{
+		Snap:      snapInfo,
+		Name:      "clashing",
+		Interface: "interface",
+	}}
+	slot := &Slot{SlotInfo: &snap.SlotInfo{
+		Snap:      snapInfo,
+		Name:      "clashing",
+		Interface: "interface",
+	}}
+	err := s.testRepo.AddPlug(plug)
+	c.Assert(err, IsNil)
+	err = s.testRepo.AddSlot(slot)
+	c.Assert(err, ErrorMatches, `snap "snap" has plug and slot conflicting on name "clashing"`)
+	c.Assert(s.testRepo.AllPlugs(""), HasLen, 1)
+	c.Assert(s.testRepo.Plug(plug.Snap.Name(), plug.Name), DeepEquals, plug)
 }
 
 func (s *RepositorySuite) TestAddSlotFailsWithUnsanitizedSlot(c *C) {