Merge pull request #157 from Nordix/callback-draft

Refactor Porch code to remove callback from PackageRevisionDraft to repository for Draft closing

pkg/cache/memory/cache_test.go

@@ -112,7 +112,7 @@ func TestPublishedLatest(t *testing.T) {
 	if err := update.UpdateLifecycle(ctx, api.PackageRevisionLifecyclePublished); err != nil {
 		t.Fatalf("UpdateLifecycle failed; %v", err)
 	}
-	closed, err := update.Close(ctx, "")
+	closed, err := cachedRepo.ClosePackageRevisionDraft(ctx, update, "")
 	if err != nil {
 		t.Fatalf("Close failed: %v", err)
 	}
@@ -158,7 +158,7 @@ func TestDeletePublishedMain(t *testing.T) {
 	if err := update.UpdateLifecycle(ctx, api.PackageRevisionLifecyclePublished); err != nil {
 		t.Fatalf("UpdateLifecycle failed; %v", err)
 	}
-	closed, err := update.Close(ctx, "")
+	closed, err := cachedRepo.ClosePackageRevisionDraft(ctx, update, "")
 	if err != nil {
 		t.Fatalf("Close failed: %v", err)
 	}

pkg/cache/memory/repository.go

@@ -1,4 +1,4 @@
-// Copyright 2022,2024 The kpt and Nephio Authors
+// Copyright 2022, 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -168,29 +168,56 @@ func (r *cachedRepository) getCachedPackages(ctx context.Context, forceRefresh b
 }
 
 func (r *cachedRepository) CreatePackageRevision(ctx context.Context, obj *v1alpha1.PackageRevision) (repository.PackageRevisionDraft, error) {
-	created, err := r.repo.CreatePackageRevision(ctx, obj)
+	return r.repo.CreatePackageRevision(ctx, obj)
+}
+
+func (r *cachedRepository) ClosePackageRevisionDraft(ctx context.Context, prd repository.PackageRevisionDraft, version string) (repository.PackageRevision, error) {
+	ctx, span := tracer.Start(ctx, "cachedRepository::ClosePackageRevisionDraft", trace.WithAttributes())
+	defer span.End()
+
+	v, err := r.Version(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	if v != r.lastVersion {
+		_, _, err = r.refreshAllCachedPackages(ctx)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	revisions, err := r.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{
+		Package: prd.GetName(),
+	})
 	if err != nil {
 		return nil, err
 	}
 
-	return &cachedDraft{
-		PackageRevisionDraft: created,
-		cache:                r,
-	}, nil
+	var publishedRevisions []string
+	for _, rev := range revisions {
+		if v1alpha1.LifecycleIsPublished(rev.Lifecycle()) {
+			publishedRevisions = append(publishedRevisions, rev.Key().Revision)
+		}
+	}
+
+	nextVersion, err := repository.NextRevisionNumber(publishedRevisions)
+	if err != nil {
+		return nil, err
+	}
+
+	if closed, err := r.repo.ClosePackageRevisionDraft(ctx, prd, nextVersion); err != nil {
+		return nil, err
+	} else {
+		return r.update(ctx, closed)
+	}
 }
 
 func (r *cachedRepository) UpdatePackageRevision(ctx context.Context, old repository.PackageRevision) (repository.PackageRevisionDraft, error) {
 	// Unwrap
 	unwrapped := old.(*cachedPackageRevision).PackageRevision
-	created, err := r.repo.UpdatePackageRevision(ctx, unwrapped)
-	if err != nil {
-		return nil, err
-	}
 
-	return &cachedDraft{
-		PackageRevisionDraft: created,
-		cache:                r,
-	}, nil
+	return r.repo.UpdatePackageRevision(ctx, unwrapped)
 }
 
 func (r *cachedRepository) update(ctx context.Context, updated repository.PackageRevision) (*cachedPackageRevision, error) {

pkg/engine/engine.go

@@ -1,4 +1,4 @@
-// Copyright 2022,2024 The kpt and Nephio Authors
+// Copyright 2022, 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -184,7 +184,7 @@ func (cad *cadEngine) CreatePackageRevision(ctx context.Context, repositoryObj *
 	}
 
 	// Updates are done.
-	repoPkgRev, err := draft.Close(ctx, "")
+	repoPkgRev, err := repo.ClosePackageRevisionDraft(ctx, draft, "")
 	if err != nil {
 		return nil, err
 	}
@@ -309,7 +309,7 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, version string,
 	}
 
 	// Updates are done.
-	repoPkgRev, err = draft.Close(ctx, version)
+	repoPkgRev, err = repo.ClosePackageRevisionDraft(ctx, draft, version)
 	if err != nil {
 		return nil, err
 	}
@@ -509,7 +509,7 @@ func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj
 		return nil, renderStatus, err
 	}
 	// No lifecycle change when updating package resources; updates are done.
-	repoPkgRev, err := draft.Close(ctx, "")
+	repoPkgRev, err := repo.ClosePackageRevisionDraft(ctx, draft, "")
 	if err != nil {
 		return nil, renderStatus, err
 	}
@@ -562,7 +562,7 @@ func (cad *cadEngine) RecloneAndReplay(ctx context.Context, parentPR repository.
 		return nil, err
 	}
 
-	repoPkgRev, err := draft.Close(ctx, version)
+	repoPkgRev, err := repo.ClosePackageRevisionDraft(ctx, draft, version)
 
 	if err != nil {
 		return nil, err

pkg/git/draft.go

@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2022, 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -63,14 +63,6 @@ func (d *gitPackageRevisionDraft) UpdateLifecycle(ctx context.Context, new v1alp
 	return nil
 }
 
-// Finish round of updates.
-func (d *gitPackageRevisionDraft) Close(ctx context.Context, version string) (repository.PackageRevision, error) {
-	ctx, span := tracer.Start(ctx, "gitPackageDraft::Close", trace.WithAttributes())
-	defer span.End()
-
-	return d.parent.CloseDraft(ctx, version, d)
-}
-
 func (d *gitPackageRevisionDraft) GetName() string {
 	packageDirectory := d.parent.directory
 	packageName := strings.TrimPrefix(d.path, packageDirectory+"/")

pkg/git/git.go

@@ -1480,10 +1480,15 @@ func (r *gitRepository) UpdateDraftResources(ctx context.Context, draft *gitPack
 	return nil
 }
 
-func (r *gitRepository) CloseDraft(ctx context.Context, version string, d *gitPackageRevisionDraft) (*gitPackageRevision, error) {
+func (r *gitRepository) ClosePackageRevisionDraft(ctx context.Context, prd repository.PackageRevisionDraft, version string) (repository.PackageRevision, error) {
+	ctx, span := tracer.Start(ctx, "GitRepository::UpdateLifecycle", trace.WithAttributes())
+	defer span.End()
+
 	r.mutex.Lock()
 	defer r.mutex.Unlock()
 
+	d := prd.(*gitPackageRevisionDraft)
+
 	refSpecs := newPushRefSpecBuilder()
 	draftBranch := createDraftName(d.path, d.workspaceName)
 	proposedBranch := createProposedName(d.path, d.workspaceName)

pkg/git/git_test.go

@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2022, 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -180,7 +180,7 @@ func (g GitSuite) TestGitPackageRoundTrip(t *testing.T) {
 			t.Fatalf("draft.UpdateResources(%#v, %#v) failed: %v", newResources, task, err)
 		}
 
-		revision, err := draft.Close(ctx, "v1")
+		revision, err := repo.ClosePackageRevisionDraft(ctx, draft, "v1")
 		if err != nil {
 			t.Fatalf("draft.Close() failed: %v", err)
 		}
@@ -207,7 +207,7 @@ func (g GitSuite) TestGitPackageRoundTrip(t *testing.T) {
 		if err := update.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecyclePublished); err != nil {
 			t.Fatalf("UpdateLifecycle failed: %v", err)
 		}
-		approved, err := update.Close(ctx, "v1")
+		approved, err := repo.ClosePackageRevisionDraft(ctx, update, "v1")
 		if err != nil {
 			t.Fatalf("Close() of %q, %q failed: %v", packageName, workspace, err)
 		}
@@ -418,7 +418,7 @@ func (g GitSuite) TestListPackagesTrivial(t *testing.T) {
 	}); err != nil {
 		t.Fatalf("UpdateResources() failed: %v", err)
 	}
-	newRevision, err := draft.Close(ctx, "")
+	newRevision, err := git.ClosePackageRevisionDraft(ctx, draft, "")
 	if err != nil {
 		t.Fatalf("draft.Close() failed: %v", err)
 	}
@@ -506,7 +506,7 @@ func (g GitSuite) TestCreatePackageInTrivialRepository(t *testing.T) {
 	}); err != nil {
 		t.Fatalf("UpdateResources() failed: %v", err)
 	}
-	newRevision, err := draft.Close(ctx, "")
+	newRevision, err := git.ClosePackageRevisionDraft(ctx, draft, "")
 	if err != nil {
 		t.Fatalf("draft.Close() failed: %v", err)
 	}
@@ -688,7 +688,7 @@ func (g GitSuite) TestApproveDraft(t *testing.T) {
 
 	update.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecyclePublished)
 
-	new, err := update.Close(ctx, "v1")
+	new, err := git.ClosePackageRevisionDraft(ctx, update, "v1")
 	if err != nil {
 		t.Fatalf("Close failed: %v", err)
 	}
@@ -750,7 +750,7 @@ func (g GitSuite) TestApproveDraftWithHistory(t *testing.T) {
 
 	update.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecyclePublished)
 
-	new, err := update.Close(ctx, "v1")
+	new, err := git.ClosePackageRevisionDraft(ctx, update, "v1")
 	if err != nil {
 		t.Fatalf("Close failed: %v", err)
 	}

pkg/oci/mutate.go

@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2022, 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -55,7 +55,7 @@ func (r *ociRepository) CreatePackageRevision(ctx context.Context, obj *v1alpha1
 	}
 
 	// digestName := ImageDigestName{}
-	return &ociPackageDraft{
+	return &ociPackageRevisionDraft{
 		packageName: packageName,
 		parent:      r,
 		tasks:       []v1alpha1.Task{},
@@ -92,7 +92,7 @@ func (r *ociRepository) UpdatePackageRevision(ctx context.Context, old repositor
 		return nil, fmt.Errorf("error fetching image %q: %w", ref, err)
 	}
 
-	return &ociPackageDraft{
+	return &ociPackageRevisionDraft{
 		packageName: packageName,
 		parent:      r,
 		tasks:       []v1alpha1.Task{},
@@ -102,7 +102,7 @@ func (r *ociRepository) UpdatePackageRevision(ctx context.Context, old repositor
 	}, nil
 }
 
-type ociPackageDraft struct {
+type ociPackageRevisionDraft struct {
 	packageName string
 
 	created time.Time
@@ -118,10 +118,10 @@ type ociPackageDraft struct {
 	lifecycle v1alpha1.PackageRevisionLifecycle // New value of the package revision lifecycle
 }
 
-var _ repository.PackageRevisionDraft = (*ociPackageDraft)(nil)
+var _ repository.PackageRevisionDraft = (*ociPackageRevisionDraft)(nil)
 
-func (p *ociPackageDraft) UpdateResources(ctx context.Context, new *v1alpha1.PackageRevisionResources, task *v1alpha1.Task) error {
-	_, span := tracer.Start(ctx, "ociPackageDraft::UpdateResources", trace.WithAttributes())
+func (p *ociPackageRevisionDraft) UpdateResources(ctx context.Context, new *v1alpha1.PackageRevisionResources, task *v1alpha1.Task) error {
+	_, span := tracer.Start(ctx, "ociPackageRevisionDraft::UpdateResources", trace.WithAttributes())
 	defer span.End()
 
 	buf := bytes.NewBuffer(nil)
@@ -190,20 +190,22 @@ func (p *ociPackageDraft) UpdateResources(ctx context.Context, new *v1alpha1.Pac
 	return nil
 }
 
-func (p *ociPackageDraft) UpdateLifecycle(ctx context.Context, new v1alpha1.PackageRevisionLifecycle) error {
+func (p *ociPackageRevisionDraft) UpdateLifecycle(ctx context.Context, new v1alpha1.PackageRevisionLifecycle) error {
 	p.lifecycle = new
 	return nil
 }
 
-func (p *ociPackageDraft) GetName() string {
+func (p *ociPackageRevisionDraft) GetName() string {
 	return p.packageName
 }
 
 // Finish round of updates.
-func (p *ociPackageDraft) Close(ctx context.Context, version string) (repository.PackageRevision, error) {
-	ctx, span := tracer.Start(ctx, "ociPackageDraft::Close", trace.WithAttributes())
+func (r *ociRepository) ClosePackageRevisionDraft(ctx context.Context, prd repository.PackageRevisionDraft, version string) (repository.PackageRevision, error) {
+	ctx, span := tracer.Start(ctx, "ociRepository::ClosePackageRevisionDraft", trace.WithAttributes())
 	defer span.End()
 
+	p := prd.(*ociPackageRevisionDraft)
+
 	ref := p.tag
 	option := remote.WithAuthFromKeychain(gcrane.Keychain)
 

pkg/repository/fake/repository.go

@@ -61,6 +61,10 @@ func (r *Repository) CreatePackageRevision(_ context.Context, pr *v1alpha1.Packa
 	return nil, nil
 }
 
+func (r *Repository) ClosePackageRevisionDraft(ctx context.Context, prd repository.PackageRevisionDraft, version string) (repository.PackageRevision, error) {
+	return nil, nil
+}
+
 func (r *Repository) DeletePackageRevision(context.Context, repository.PackageRevision) error {
 	return nil
 }