Merge pull request juju#18963 from gfouillet/v4/dqlite/resources/remo…

…ve-origin-rev-recordstoredresourcs

juju#18963

Since the new way to use resources tables is to add a resource and move association through application_resource and unit_resource table, StoreResources doesn't need to update origin or revision anymore. It will always upload a resource with valid origin and revision.

So this commit remove those fields from all layers in the call stack.

## Checklist

- [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~
~- [ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps

Unit test still pass.

Behavior won't be perfect until other PR which fixes resources are landed, however, above steps should works:

```sh
juju deploy juju-qa-test qa
```
Wait for IDLE, until

```sh
App Version Status Scale Charm Channel Rev Exposed Message
qa active 1 juju-qa-test latest/stable 25 no hello
```

```sh
juju config qa foo-file=true
```
Status should update:

```sh
App Version Status Scale Charm Channel Rev Exposed Message
qa active 1 juju-qa-test latest/stable 25 no resource line one: testing two.
```

```sh
echo "spam" > test.txt
juju attach-resource qa3 foo-file=./test.txt
```
Status should update:

```sh
App Version Status Scale Charm Channel Rev Exposed Message
qa active 1 juju-qa-test latest/stable 25 no resource line one: spam
```

> [!NOTE]
> Migration won't work properly because it requires this PR to be landed: juju#18916

Migrate the model to another controller.

Status should be ok on the new controller after a while

```sh
App Version Status Scale Charm Channel Rev Exposed Message
qa active 1 juju-qa-test latest/stable 25 no resource line one: spam
```

apiserver/internal/handlers/resources/resourcemigration.go

@@ -125,21 +125,38 @@ func (h *resourcesMigrationUploadHandler) processPost(
 		return empty, internalerrors.Errorf("extracting resource details from request: %w", err)
 	}
 
+	// Check that the resource associated with the UUID has the expected Origin
+	// and revision, to prevent updating the blob of another revision or origin
+	// that may have been updated between the start of the http call and the
+	// retrieval of resource UUID from application name and resource name.
+	res, err := resourceService.GetResource(ctx, resUUID)
+	if err != nil {
+		return empty, internalerrors.Errorf("getting resource %s on application %s: %w", appName,
+			resourceName, err)
+	}
+	if res.Origin != details.origin || (res.Origin != charmresource.OriginUpload && res.Revision != details.revision) {
+		// In this case, we won't interrupt migration, because the user will
+		// be able to recover after the migration. However, we log an error in
+		// order to make it notice something get wrong and why.
+		h.logger.Errorf(ctx, "resource upload failed: resource %s on application %s has unexpected origin or revision: %s != %s, %d != %d",
+			appName, resourceName, res.Origin, details.origin, res.Revision, details.revision,
+		)
+		return res, nil
+	}
+	retrievedBy, retrievedByType := determineRetrievedBy(query)
+
 	reader, err := h.downloader.Download(r.Context(), r.Body, details.fingerprint.String(), details.size)
 	if err != nil {
 		return empty, internalerrors.Errorf("validating resource size and hash: %w", err)
 	}
 
-	retrievedBy, retrievedByType := determineRetrievedBy(query)
 	err = resourceService.StoreResource(ctx, resource.StoreResourceArgs{
 		ResourceUUID:    resUUID,
 		Reader:          reader,
 		RetrievedBy:     retrievedBy,
 		RetrievedByType: retrievedByType,
 		Size:            details.size,
 		Fingerprint:     details.fingerprint,
-		Origin:          details.origin,
-		Revision:        details.revision,
 	})
 	if err != nil {
 		return empty, internalerrors.Capture(err)

apiserver/internal/handlers/resources/resourcemigration_test.go

@@ -163,6 +163,13 @@ func (s *resourcesUploadSuite) TestServeUploadApplicationStoreResourceError(c *g
 	).Return("res-uuid", nil)
 	s.downloader.EXPECT().Download(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
 	s.resourceService.EXPECT().StoreResource(gomock.Any(), gomock.Any()).Return(errors.New("cannot store resource"))
+	s.resourceService.EXPECT().GetResource(gomock.Any(), resource.UUID("res-uuid")).Return(resource.Resource{
+		UUID: "res-uuid",
+		Resource: charmresource.Resource{
+			Origin:   s.origin,
+			Revision: s.revision,
+		},
+	}, nil)
 
 	// Act
 	response, err := http.Post(s.srv.URL+migrateResourcesPrefix+"?"+query.Encode(), "application/octet-stream", nil)
@@ -192,8 +199,6 @@ func (s *resourcesUploadSuite) TestServeUploadApplicationGetResourceError(c *gc.
 		"app-name",
 		"resource-name",
 	).Return("res-uuid", nil)
-	s.downloader.EXPECT().Download(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
-	s.resourceService.EXPECT().StoreResource(gomock.Any(), gomock.Any()).Return(nil)
 	s.resourceService.EXPECT().GetResource(gomock.Any(), gomock.Any()).Return(resource.Resource{}, errors.New(
 		"cannot get resource"))
 
@@ -257,15 +262,17 @@ func (s *resourcesUploadSuite) TestServeUploadApplication(c *gc.C) {
 		ResourceUUID:    "res-uuid",
 		Reader:          http.NoBody,
 		RetrievedByType: resource.Application,
-		Origin:          s.origin,
-		Revision:        s.revision,
 		Fingerprint:     s.fingerprint,
 		Size:            s.size,
 	}).Return(nil)
 	s.resourceService.EXPECT().GetResource(gomock.Any(), resource.UUID("res-uuid")).Return(resource.Resource{
-		UUID:      "res-uuid",
+		UUID: "res-uuid",
+		Resource: charmresource.Resource{
+			Origin:   s.origin,
+			Revision: s.revision,
+		},
 		Timestamp: now,
-	}, nil)
+	}, nil).Times(2)
 
 	// Act
 	response, err := http.Post(s.srv.URL+migrateResourcesPrefix+"?"+query.Encode(), "application/octet-stream", http.NoBody)
@@ -299,7 +306,6 @@ func (s *resourcesUploadSuite) TestServeUploadApplicationRetrievedByUser(c *gc.C
 		"origin":      {"upload"},
 		"size":        {s.sizeStr},
 		"fingerprint": {s.fingerprintStr},
-		"revision":    {s.revisionStr},
 	}
 	s.resourceService.EXPECT().GetResourceUUIDByApplicationAndResourceName(
 		gomock.Any(),
@@ -317,15 +323,17 @@ func (s *resourcesUploadSuite) TestServeUploadApplicationRetrievedByUser(c *gc.C
 		Reader:          http.NoBody,
 		RetrievedByType: resource.User,
 		RetrievedBy:     "username",
-		Origin:          charmresource.OriginUpload,
-		Revision:        -1,
 		Fingerprint:     s.fingerprint,
 		Size:            s.size,
 	}).Return(nil)
 	s.resourceService.EXPECT().GetResource(gomock.Any(), resource.UUID("res-uuid")).Return(resource.Resource{
-		UUID:      "res-uuid",
+		UUID: "res-uuid",
+		Resource: charmresource.Resource{
+			Origin:   charmresource.OriginUpload,
+			Revision: -1,
+		},
 		Timestamp: now,
-	}, nil)
+	}, nil).Times(2)
 
 	// Act
 	_, err := http.Post(s.srv.URL+migrateResourcesPrefix+"?"+query.Encode(), "application/octet-stream", http.NoBody)
@@ -365,15 +373,17 @@ func (s *resourcesUploadSuite) TestServeUploadApplicationRetrievedByApplication(
 		Reader:          http.NoBody,
 		RetrievedByType: resource.Application,
 		RetrievedBy:     "app-name",
-		Origin:          s.origin,
-		Revision:        s.revision,
 		Fingerprint:     s.fingerprint,
 		Size:            s.size,
 	}).Return(nil)
 	s.resourceService.EXPECT().GetResource(gomock.Any(), resource.UUID("res-uuid")).Return(resource.Resource{
-		UUID:      "res-uuid",
+		UUID: "res-uuid",
+		Resource: charmresource.Resource{
+			Origin:   s.origin,
+			Revision: s.revision,
+		},
 		Timestamp: now,
-	}, nil)
+	}, nil).Times(2)
 
 	// Act
 	_, err := http.Post(s.srv.URL+migrateResourcesPrefix+"?"+query.Encode(), "application/octet-stream", http.NoBody)
@@ -412,15 +422,17 @@ func (s *resourcesUploadSuite) TestServeUploadApplicationRetrievedByUnit(c *gc.C
 		Reader:          http.NoBody,
 		RetrievedByType: resource.Unit,
 		RetrievedBy:     "app-name/0",
-		Origin:          s.origin,
-		Revision:        s.revision,
 		Fingerprint:     s.fingerprint,
 		Size:            s.size,
 	}).Return(nil)
 	s.resourceService.EXPECT().GetResource(gomock.Any(), resource.UUID("res-uuid")).Return(resource.Resource{
-		UUID:      "res-uuid",
+		UUID: "res-uuid",
+		Resource: charmresource.Resource{
+			Origin:   s.origin,
+			Revision: s.revision,
+		},
 		Timestamp: now,
-	}, nil)
+	}, nil).Times(2)
 
 	// Act
 	_, err := http.Post(s.srv.URL+migrateResourcesPrefix+"?"+query.Encode(), "application/octet-stream", http.NoBody)

apiserver/internal/handlers/resources/resources.go

@@ -167,8 +167,6 @@ func (h *ResourceHandler) upload(service ResourceService, req *http.Request, use
 			RetrievedByType: coreresource.User,
 			Size:            uploaded.size,
 			Fingerprint:     uploaded.fingerprint,
-			Origin:          charmresource.OriginUpload,
-			Revision:        -1,
 		},
 	)
 	if err != nil {

apiserver/internal/handlers/resources/resources_test.go

@@ -257,8 +257,6 @@ func (s *ResourcesHandlerSuite) TestPutSuccess(c *gc.C) {
 		RetrievedByType: coreresource.User,
 		Size:            s.resource.Size,
 		Fingerprint:     s.resource.Fingerprint,
-		Origin:          charmresource.OriginUpload,
-		Revision:        -1,
 	})
 
 	// Second call to GetResource gets resource details after upload.
@@ -347,8 +345,6 @@ func (s *ResourcesHandlerSuite) TestPutSuccessDockerResource(c *gc.C) {
 		RetrievedByType: coreresource.User,
 		Size:            s.resource.Size,
 		Fingerprint:     s.resource.Fingerprint,
-		Origin:          charmresource.OriginUpload,
-		Revision:        -1,
 	})
 
 	// Second call to GetResource gets resource details after upload.
@@ -446,8 +442,6 @@ func (s *ResourcesHandlerSuite) TestPutWithPending(c *gc.C) {
 		RetrievedByType: coreresource.User,
 		Size:            s.resource.Size,
 		Fingerprint:     s.resource.Fingerprint,
-		Origin:          charmresource.OriginUpload,
-		Revision:        -1,
 	})
 
 	// Second call to GetResource gets resource details after upload.

domain/resource/service/resource.go

@@ -397,22 +397,6 @@ func (s *Service) storeResource(
 		return resourceerrors.RetrievedByTypeNotValid
 	}
 
-	if err := args.Origin.Validate(); err != nil {
-		return errors.Errorf("resource origin: %w", err)
-	}
-	if args.Origin == charmresource.OriginUpload && args.Revision != -1 {
-		return errors.Errorf(
-			"resource with origin upload must have positive -1, found %d: %w",
-			args.Revision, resourceerrors.ResourceRevisionNotValid,
-		)
-	}
-	if args.Origin == charmresource.OriginStore && args.Revision < 0 {
-		return errors.Errorf(
-			"resource with origin store must have positive revision, found %d, %w",
-			args.Revision, resourceerrors.ResourceRevisionNotValid,
-		)
-	}
-
 	res, err := s.st.GetResource(ctx, args.ResourceUUID)
 	if err != nil {
 		return errors.Errorf("getting resource: %w", err)
@@ -458,8 +442,6 @@ func (s *Service) storeResource(
 			IncrementCharmModifiedVersion: incrementCharmModifiedVersion,
 			Size:                          args.Size,
 			SHA384:                        args.Fingerprint.String(),
-			Origin:                        args.Origin,
-			Revision:                      args.Revision,
 		},
 	)
 	if err != nil {