Merge pull request juju#17635 from SimonRichardson/charm-state

juju#17635

The change set adds simple getter and setter methods, along with getting the charm metadata. Getting the charm metadata from the database is quite involved. We have to select from a lot of tables to populate the metadata. This is really wasteful if the code only wants the relations. It is also not time efficient to get all the metadata as row based type. If a charm metadata is fully populated it could result in thousands of rows to populate. Having spoken with others, it was then decided to not use row-based requests for getting metadata. Instead, we'll fallback to multiple selects.
The advantage of doing it this way is that we can reuse the individual methods to expose calls to get just relations or
storage.

There is a potential problem with arrays. Duplicates of values are possible in the charm metadata and we don't prevent that in any part of the library. Also, tables are sets of rows, not ordered rows based on insertion time. The solution was to keep a tracking index column that ensures that we preserve the insertion order and that we use that as the unique key.

Unfortunately, I ran into a sqlair bug[1], so had to change tack and ensure that we could get the right information out that we wanted. This meant that we stuck to the error conditions. If the charm doesn't exist we return NotFound, rather than the default case.

Actions, config, manifest, and LXD profiles will be added in a future PR, as this is already getting difficult to review in one patch.


 1. canonical/sqlair#154

<!-- 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

## QA steps

The schema should apply nicely.

Regression tests around bootstrap and deployment should be done.

```sh
$ juju bootstrap lxd test --build-agent
$ juju add-model default
$ juju deploy ubuntu
```

## Links

**Jira card:** [JUJU-6015](https://warthogs.atlassian.net/browse/JUJU-6015)



[JUJU-6015]: https://warthogs.atlassian.net/browse/JUJU-6015?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

core/charm/id.go

@@ -23,6 +23,15 @@ func NewID() (ID, error) {
 	return ID(uuid.String()), nil
 }
 
+// ParseID returns a new ID from the given string. If the string is not a valid
+// uuid an error satisfying [errors.NotValid] will be returned.
+func ParseID(value string) (ID, error) {
+	if !uuid.IsValidUUIDString(value) {
+		return "", fmt.Errorf("id %q %w", value, errors.NotValid)
+	}
+	return ID(value), nil
+}
+
 // String implements the stringer interface for ID.
 func (u ID) String() string {
 	return string(u)

domain/charm/service/metadata.go

@@ -436,6 +436,7 @@ func encodeMetadataRelation(relations map[string]internalcharm.Relation) (map[st
 		}
 
 		result[k] = charm.Relation{
+			Key:       k,
 			Name:      v.Name,
 			Role:      role,
 			Scope:     scope,

domain/charm/service/metadata_test.go

@@ -87,7 +87,8 @@ var metadataTestCases = [...]struct {
 			Name:  "foo",
 			RunAs: charm.RunAsDefault,
 			Provides: map[string]charm.Relation{
-				"db": {
+				"baz": {
+					Key:       "baz",
 					Name:      "db",
 					Role:      charm.RoleProvider,
 					Interface: "mysql",
@@ -100,7 +101,7 @@ var metadataTestCases = [...]struct {
 		output: internalcharm.Meta{
 			Name: "foo",
 			Provides: map[string]internalcharm.Relation{
-				"db": {
+				"baz": {
 					Name:      "db",
 					Role:      internalcharm.RoleProvider,
 					Interface: "mysql",
@@ -117,7 +118,8 @@ var metadataTestCases = [...]struct {
 			Name:  "foo",
 			RunAs: charm.RunAsDefault,
 			Requires: map[string]charm.Relation{
-				"db": {
+				"baz": {
+					Key:       "baz",
 					Name:      "db",
 					Role:      charm.RoleProvider,
 					Interface: "mysql",
@@ -130,7 +132,7 @@ var metadataTestCases = [...]struct {
 		output: internalcharm.Meta{
 			Name: "foo",
 			Requires: map[string]internalcharm.Relation{
-				"db": {
+				"baz": {
 					Name:      "db",
 					Role:      internalcharm.RoleProvider,
 					Interface: "mysql",
@@ -147,7 +149,8 @@ var metadataTestCases = [...]struct {
 			Name:  "foo",
 			RunAs: charm.RunAsDefault,
 			Peers: map[string]charm.Relation{
-				"db": {
+				"baz": {
+					Key:       "baz",
 					Name:      "db",
 					Role:      charm.RoleProvider,
 					Interface: "mysql",
@@ -160,7 +163,7 @@ var metadataTestCases = [...]struct {
 		output: internalcharm.Meta{
 			Name: "foo",
 			Peers: map[string]internalcharm.Relation{
-				"db": {
+				"baz": {
 					Name:      "db",
 					Role:      internalcharm.RoleProvider,
 					Interface: "mysql",

domain/charm/service/service.go

@@ -40,11 +40,6 @@ type State interface {
 	// If the charm does not exist, a NotFound error is returned.
 	GetCharmIDByRevision(ctx context.Context, name string, revision int) (corecharm.ID, error)
 
-	// GetCharmIDByLatestRevision returns the charm ID by the natural key, for
-	// the latest revision.
-	// If the charm does not exist, a NotFound error is returned.
-	GetCharmIDByLatestRevision(ctx context.Context, name string) (corecharm.ID, error)
-
 	// IsControllerCharm returns whether the charm is a controller charm.
 	// If the charm does not exist, a NotFound error is returned.
 	IsControllerCharm(ctx context.Context, id corecharm.ID) (bool, error)
@@ -123,7 +118,7 @@ func (s *Service) GetCharmID(ctx context.Context, args charm.GetCharmArgs) (core
 		return s.st.GetCharmIDByRevision(ctx, args.Name, *rev)
 	}
 
-	return s.st.GetCharmIDByLatestRevision(ctx, args.Name)
+	return "", charmerrors.NotFound
 }
 
 // IsControllerCharm returns whether the charm is a controller charm.

domain/charm/service/service_test.go

@@ -35,15 +35,10 @@ var _ = gc.Suite(&serviceSuite{})
 func (s *serviceSuite) TestGetCharmID(c *gc.C) {
 	defer s.setupMocks(c).Finish()
 
-	id := charmtesting.GenCharmID(c)
-
-	s.state.EXPECT().GetCharmIDByLatestRevision(gomock.Any(), "foo").Return(id, nil)
-
-	result, err := s.service.GetCharmID(context.Background(), domaincharm.GetCharmArgs{
+	_, err := s.service.GetCharmID(context.Background(), domaincharm.GetCharmArgs{
 		Name: "foo",
 	})
-	c.Assert(err, jc.ErrorIsNil)
-	c.Check(result, gc.Equals, id)
+	c.Assert(err, jc.ErrorIs, charmerrors.NotFound)
 }
 
 func (s *serviceSuite) TestGetCharmIDInvalidName(c *gc.C) {
@@ -72,17 +67,6 @@ func (s *serviceSuite) TestGetCharmIDWithRevision(c *gc.C) {
 	c.Check(result, gc.Equals, id)
 }
 
-func (s *serviceSuite) TestGetCharmIDErrorNotFound(c *gc.C) {
-	defer s.setupMocks(c).Finish()
-
-	s.state.EXPECT().GetCharmIDByLatestRevision(gomock.Any(), "foo").Return("", charmerrors.NotFound)
-
-	_, err := s.service.GetCharmID(context.Background(), domaincharm.GetCharmArgs{
-		Name: "foo",
-	})
-	c.Assert(err, jc.ErrorIs, charmerrors.NotFound)
-}
-
 func (s *serviceSuite) TestIsControllerCharm(c *gc.C) {
 	defer s.setupMocks(c).Finish()