Merge pull request juju#18434 from Aflynn50/resource-id-to-uuid

juju#18434

Use UUID instead of ID for resources. The value is a UUID so should be called as such to match with other UUID of other entites.

<!-- 
The PR title should match: <type>(optional <scope>): <description>.

Please also ensure all commits in this PR comply with our conventional commits specification:
https://github.com/juju/juju/blob/main/doc/conventional-commits.md
-->

<!-- Why this change is needed and what it does. -->

## Checklist

<!-- If an item is not applicable, use `~strikethrough~`. -->

- [x] Code style: imports ordered, good names, simple structure, etc
- [x] Comments saying why design decisions were made
- [x] Go unit tests, with comments saying what you're testing
- [ ] [Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing
- [x] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages

## QA steps

Juju compiles and unit tests pass

core/resource/resource.go

@@ -35,7 +35,7 @@ import (
 // A resource may also be added to the model as "pending", meaning it
 // is queued up to be used as a resource for the application. Until it is
 // "activated", a pending resources is virtually invisible. There may
-// be more that one pending resource for a given resource ID.
+// be more that one pending resource for a given resource UUID.
 type Resource struct {
 	resource.Resource
 
@@ -78,7 +78,7 @@ func (res Resource) Validate() error {
 	}
 
 	if res.ApplicationID == "" {
-		return errors.NewNotValid(nil, "missing application ID")
+		return errors.NewNotValid(nil, "missing application UUID")
 	}
 
 	// TODO(ericsnow) Require that Username be set if timestamp is?

core/resource/resource_test.go

@@ -51,7 +51,7 @@ func (ResourceSuite) TestValidateUploadPending(c *gc.C) {
 	res := resource.Resource{
 		Resource:      newFullCharmResource(c, "spam"),
 		ID:            "a-application/spam",
-		PendingID:     "some-unique-ID",
+		PendingID:     "some-unique-UUID",
 		ApplicationID: "a-application",
 		Username:      "a-user",
 		Timestamp:     time.Now(),
@@ -109,7 +109,7 @@ func (ResourceSuite) TestValidateMissingApplicationID(c *gc.C) {
 	err := res.Validate()
 
 	c.Check(err, jc.ErrorIs, errors.NotValid)
-	c.Check(err, gc.ErrorMatches, `.*missing application ID.*`)
+	c.Check(err, gc.ErrorMatches, `.*missing application UUID.*`)
 }
 
 func (ResourceSuite) TestValidateMissingUsername(c *gc.C) {

core/resource/testing/resource.go

@@ -126,10 +126,10 @@ func newStubReadCloser(stub *testing.Stub, content string) io.ReadCloser {
 	}
 }
 
-// GenResourceID can be used in testing for generating a charm ID that is
+// GenResourceUUID can be used in testing for generating a resource UUID that is
 // checked for subsequent errors using the test suit's go check instance.
-func GenResourceID(c *gc.C) resource.ID {
-	id, err := resource.NewID()
+func GenResourceUUID(c *gc.C) resource.UUID {
+	id, err := resource.NewUUID()
 	c.Assert(err, jc.ErrorIsNil)
 	return id
 }

core/resource/uuid.go

@@ -0,0 +1,50 @@
+// Copyright 2024 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package resource
+
+import (
+	"fmt"
+
+	"github.com/juju/errors"
+
+	"github.com/juju/juju/internal/uuid"
+)
+
+// UUID represents a resource unique identifier.
+type UUID string
+
+// NewUUID is a convince function for generating a new resource uuid.
+func NewUUID() (UUID, error) {
+	id, err := uuid.NewUUID()
+	if err != nil {
+		return UUID(""), err
+	}
+	return UUID(id.String()), nil
+}
+
+// ParseUUID returns a new UUID from the given string. If the string is not a
+// valid uuid an error satisfying [errors.NotValid] will be returned.
+func ParseUUID(value string) (UUID, error) {
+	if !uuid.IsValidUUIDString(value) {
+		return "", fmt.Errorf("id %q %w", value, errors.NotValid)
+	}
+	return UUID(value), nil
+}
+
+// String implements the stringer interface for UUID.
+func (u UUID) String() string {
+	return string(u)
+}
+
+// Validate ensures the consistency of the UUID. If the uuid is invalid an error
+// satisfying [errors.NotValid] will be returned.
+func (u UUID) Validate() error {
+	if u == "" {
+		return fmt.Errorf("%wuuid cannot be empty", errors.Hide(errors.NotValid))
+	}
+	if !uuid.IsValidUUIDString(string(u)) {
+		return fmt.Errorf("uuid %q %w", u, errors.NotValid)
+	}
+	return nil
+}

core/resource/id_test.go → core/resource/uuid_test.go

@@ -18,7 +18,7 @@ type resourcesSuite struct {
 
 var _ = gc.Suite(&resourcesSuite{})
 
-func (*resourcesSuite) TestIDValidate(c *gc.C) {
+func (*resourcesSuite) TestUUIDValidate(c *gc.C) {
 	tests := []struct {
 		uuid string
 		err  error
@@ -38,7 +38,7 @@ func (*resourcesSuite) TestIDValidate(c *gc.C) {
 
 	for i, test := range tests {
 		c.Logf("test %d: %q", i, test.uuid)
-		err := ID(test.uuid).Validate()
+		err := UUID(test.uuid).Validate()
 
 		if test.err == nil {
 			c.Check(err, gc.IsNil)
@@ -49,7 +49,7 @@ func (*resourcesSuite) TestIDValidate(c *gc.C) {
 	}
 }
 
-func (*resourcesSuite) TestParseID(c *gc.C) {
+func (*resourcesSuite) TestParseUUID(c *gc.C) {
 	tests := []struct {
 		uuid string
 		err  error
@@ -69,7 +69,7 @@ func (*resourcesSuite) TestParseID(c *gc.C) {
 
 	for i, test := range tests {
 		c.Logf("test %d: %q", i, test.uuid)
-		id, err := ParseID(test.uuid)
+		id, err := ParseUUID(test.uuid)
 
 		if test.err == nil {
 			if c.Check(err, gc.IsNil) {

domain/application/resource/fileresourcestore.go

@@ -21,7 +21,7 @@ type fileResourceStore struct {
 // Get the specified resource from the object store.
 func (f fileResourceStore) Get(
 	ctx context.Context,
-	resourceUUID coreresource.ID,
+	resourceUUID coreresource.UUID,
 ) (io.ReadCloser, int64, error) {
 	if err := resourceUUID.Validate(); err != nil {
 		return nil, 0, errors.Errorf("validating resource UUID: %w", err)
@@ -33,7 +33,7 @@ func (f fileResourceStore) Get(
 // storage path. It returns the UUID of the object store metadata.
 func (f fileResourceStore) Put(
 	ctx context.Context,
-	resourceUUID coreresource.ID,
+	resourceUUID coreresource.UUID,
 	r io.Reader,
 	size int64,
 	fingerprint resource.Fingerprint,
@@ -56,7 +56,7 @@ func (f fileResourceStore) Put(
 // Remove the specified resource from the object store.
 func (f fileResourceStore) Remove(
 	ctx context.Context,
-	resourceUUID coreresource.ID,
+	resourceUUID coreresource.UUID,
 ) error {
 	if err := resourceUUID.Validate(); err != nil {
 		return errors.Errorf("validating resource UUID: %w", err)

domain/application/resource/fileresourcestore_test.go

@@ -38,7 +38,7 @@ func (s *fileResourceStoreSuite) SetUpTest(c *gc.C) {
 	fingerprint, err := charmresource.ParseFingerprint(fp)
 	c.Assert(err, jc.ErrorIsNil)
 	s.resource = Resource{
-		ID: resourcestesting.GenResourceID(c),
+		UUID: resourcestesting.GenResourceUUID(c),
 		Resource: charmresource.Resource{
 			Meta: charmresource.Meta{
 				Name: "spam-resource",
@@ -67,15 +67,15 @@ func (s *fileResourceStoreSuite) TestFileResourceStorePut(c *gc.C) {
 	expectedStorageUUID := objectstoretesting.GenObjectStoreUUID(c)
 	s.objectStore.EXPECT().PutAndCheckHash(
 		context.Background(),
-		s.resource.ID.String(),
+		s.resource.UUID.String(),
 		s.file,
 		s.resource.Size,
 		s.resource.Fingerprint.String(),
 	).Return(expectedStorageUUID, nil)
 
 	storageUUID, err := store.Put(
 		context.Background(),
-		s.resource.ID,
+		s.resource.UUID,
 		s.file,
 		s.resource.Size,
 		s.resource.Fingerprint,
@@ -102,7 +102,7 @@ func (s *fileResourceStoreSuite) TestFileResourceStorePutNilReader(c *gc.C) {
 	store := fileResourceStore{s.objectStore}
 	_, err := store.Put(
 		context.Background(),
-		s.resource.ID,
+		s.resource.UUID,
 		nil,
 		s.resource.Size,
 		s.resource.Fingerprint,
@@ -115,7 +115,7 @@ func (s *fileResourceStoreSuite) TestFileResourceStorePutBadFingerprint(c *gc.C)
 	store := fileResourceStore{s.objectStore}
 	_, err := store.Put(
 		context.Background(),
-		s.resource.ID,
+		s.resource.UUID,
 		s.file,
 		s.resource.Size,
 		charmresource.Fingerprint{},
@@ -128,7 +128,7 @@ func (s *fileResourceStoreSuite) TestFileResourceStorePutZeroSize(c *gc.C) {
 	store := fileResourceStore{s.objectStore}
 	_, err := store.Put(
 		context.Background(),
-		s.resource.ID,
+		s.resource.UUID,
 		s.file,
 		0,
 		charmresource.Fingerprint{},
@@ -140,9 +140,9 @@ func (s *fileResourceStoreSuite) TestFileResourceStoreGet(c *gc.C) {
 	defer s.setupMocks(c).Finish()
 	store := fileResourceStore{s.objectStore}
 
-	s.objectStore.EXPECT().Get(context.Background(), s.resource.ID.String()).Return(s.file, s.resource.Size, nil)
+	s.objectStore.EXPECT().Get(context.Background(), s.resource.UUID.String()).Return(s.file, s.resource.Size, nil)
 
-	reader, size, err := store.Get(context.Background(), s.resource.ID)
+	reader, size, err := store.Get(context.Background(), s.resource.UUID)
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(reader, gc.Equals, s.file)
 	c.Assert(size, gc.Equals, s.resource.Size)
@@ -162,9 +162,9 @@ func (s *fileResourceStoreSuite) TestFileResourceStoreRemove(c *gc.C) {
 	defer s.setupMocks(c).Finish()
 	store := fileResourceStore{s.objectStore}
 
-	s.objectStore.EXPECT().Remove(context.Background(), s.resource.ID.String()).Return(nil)
+	s.objectStore.EXPECT().Remove(context.Background(), s.resource.UUID.String()).Return(nil)
 
-	err := store.Remove(context.Background(), s.resource.ID)
+	err := store.Remove(context.Background(), s.resource.UUID)
 	c.Assert(err, jc.ErrorIsNil)
 }
 

domain/application/resource/resourcestore.go

@@ -21,14 +21,14 @@ type ResourceStore interface {
 	// Get returns an io.ReadCloser for a resource in the resource store.
 	Get(
 		ctx context.Context,
-		resourceUUID coreresource.ID,
+		resourceUUID coreresource.UUID,
 	) (r io.ReadCloser, size int64, err error)
 
 	// Put stores data from io.Reader in the resource store at the
 	// using the resourceUUID as the key.
 	Put(
 		ctx context.Context,
-		resourceUUID coreresource.ID,
+		resourceUUID coreresource.UUID,
 		r io.Reader,
 		size int64,
 		fingerprint resource.Fingerprint,
@@ -37,7 +37,7 @@ type ResourceStore interface {
 	// Remove removes a resource from storage.
 	Remove(
 		ctx context.Context,
-		resourceUUID coreresource.ID,
+		resourceUUID coreresource.UUID,
 	) error
 }
 

domain/application/resource/types.go

@@ -72,8 +72,8 @@ type ApplicationResources struct {
 type Resource struct {
 	resource.Resource
 
-	// ID uniquely identifies a resource within the model.
-	ID coreresource.ID
+	// UUID uniquely identifies a resource within the model.
+	UUID coreresource.UUID
 
 	// ApplicationID identifies the application for the resource.
 	ApplicationID application.ID
@@ -146,8 +146,8 @@ type SetUnitResourceArgs struct {
 
 // SetUnitResourceResult is the result data from setting a unit's resource.
 type SetUnitResourceResult struct {
-	// ID uniquely identifies the unit resource within the model.
-	ID coreresource.ID
+	// UUID uniquely identifies the unit resource within the model.
+	UUID coreresource.UUID
 	// Timestamp indicates when the unit started using resource.
 	Timestamp time.Time
 }