fix(retention): return 404 for missing policies

api/v2.0/swagger.yaml

@@ -5144,6 +5144,8 @@ paths:
           $ref: '#/responses/401'
         '403':
           $ref: '#/responses/403'
+        '404':
+          $ref: '#/responses/404'
         '500':
           $ref: '#/responses/500'
     put:

src/controller/retention/controller_test.go

@@ -16,8 +16,8 @@ package retention
 
 import (
 	"context"
+	"fmt"
 	"os"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/mock"
@@ -26,6 +26,7 @@ import (
 	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/jobservice/job"
 	"github.com/goharbor/harbor/src/lib"
+	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/orm"
 	"github.com/goharbor/harbor/src/lib/q"
 	"github.com/goharbor/harbor/src/pkg/retention"
@@ -191,8 +192,8 @@ func (s *ControllerTestSuite) TestPolicy() {
 	s.Require().Nil(err)
 
 	p1, err = c.GetRetention(ctx, id)
-	s.Require().NotNil(err)
-	s.Require().True(strings.Contains(err.Error(), "no such Retention policy"))
+	s.Require().True(errors.IsNotFoundErr(err))
+	s.Require().EqualError(err, fmt.Sprintf("retention policy %d not found", id))
 	s.Require().Nil(p1)
 }
 

src/pkg/retention/dao/retention.go

@@ -64,7 +64,7 @@ func GetPolicy(ctx context.Context, id int64) (*models.RetentionPolicy, error) {
 		ID: id,
 	}
 	if err := o.Read(p); err != nil {
-		return nil, err
+		return nil, orm.WrapNotFoundError(err, "retention policy %d not found", id)
 	}
 	return p, nil
 }

src/pkg/retention/dao/retention_test.go

@@ -2,14 +2,15 @@ package dao
 
 import (
 	"encoding/json"
+	"fmt"
 	"os"
-	"strings"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
 
 	"github.com/goharbor/harbor/src/common/dao"
+	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/orm"
 	"github.com/goharbor/harbor/src/lib/q"
 	"github.com/goharbor/harbor/src/pkg/retention/dao/models"
@@ -98,6 +99,7 @@ func TestPolicy(t *testing.T) {
 	assert.Nil(t, err)
 
 	p1, err = GetPolicy(ctx, id)
-	assert.NotNil(t, err)
-	assert.True(t, strings.Contains(err.Error(), "no row found"))
+	assert.True(t, errors.IsNotFoundErr(err))
+	assert.EqualError(t, err, fmt.Sprintf("retention policy %d not found", id))
+	assert.Nil(t, p1)
 }

src/pkg/retention/manager.go

@@ -17,10 +17,8 @@ package retention
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 	"time"
 
-	"github.com/beego/beego/v2/client/orm"
 	"github.com/go-openapi/strfmt"
 
 	"github.com/goharbor/harbor/src/common/utils"
@@ -87,9 +85,6 @@ func (d *DefaultManager) DeletePolicy(ctx context.Context, id int64) error {
 func (d *DefaultManager) GetPolicy(ctx context.Context, id int64) (*policy.Metadata, error) {
 	p1, err := dao.GetPolicy(ctx, id)
 	if err != nil {
-		if err == orm.ErrNoRows {
-			return nil, fmt.Errorf("no such Retention policy with id %v", id)
-		}
 		return nil, err
 	}
 	p := &policy.Metadata{}

src/pkg/retention/manager_test.go

@@ -1,13 +1,14 @@
 package retention
 
 import (
+	"fmt"
 	"os"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 
 	"github.com/goharbor/harbor/src/common/dao"
+	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/orm"
 	"github.com/goharbor/harbor/src/pkg/retention/policy"
 	"github.com/goharbor/harbor/src/pkg/retention/policy/rule"
@@ -81,8 +82,8 @@ func TestPolicy(t *testing.T) {
 	assert.Nil(t, err)
 
 	p1, err = m.GetPolicy(ctx, id)
-	assert.NotNil(t, err)
-	assert.True(t, strings.Contains(err.Error(), "no such Retention policy"))
+	assert.True(t, errors.IsNotFoundErr(err))
+	assert.EqualError(t, err, fmt.Sprintf("retention policy %d not found", id))
 	assert.Nil(t, p1)
 }