Separate cleanup to its own struct in order to remove unneeded parameters

Author: Aline Abler

apiserver/billing/odoostorage/odoo/odoo8/odoo8.go

@@ -76,12 +76,24 @@ func NewOdoo8Storage(odooURL string, debugTransport bool, conf Config) *Odoo8Sto
 	}
 }
 
+func NewFailedRecordScrubber(odooURL string, debugTransport bool) *FailedRecordScrubber {
+	return &FailedRecordScrubber{
+		sessionCreator: func(ctx context.Context) (client.QueryExecutor, error) {
+			return client.Open(ctx, odooURL, client.ClientOptions{UseDebugLogger: debugTransport})
+		},
+	}
+}
+
 type Odoo8Storage struct {
 	config Config
 
 	sessionCreator func(ctx context.Context) (client.QueryExecutor, error)
 }
 
+type FailedRecordScrubber struct {
+	sessionCreator func(ctx context.Context) (client.QueryExecutor, error)
+}
+
 func (s *Odoo8Storage) Get(ctx context.Context, name string) (*billingv1.BillingEntity, error) {
 	company, accountingContact, err := s.get(ctx, name)
 	if err != nil {
@@ -278,7 +290,9 @@ func (s *Odoo8Storage) Update(ctx context.Context, be *billingv1.BillingEntity)
 	return nil
 }
 
-func (s *Odoo8Storage) CleanupIncompleteRecords(ctx context.Context, minAge time.Duration) error {
+func (s *FailedRecordScrubber) CleanupIncompleteRecords(ctx context.Context, minAge time.Duration) error {
+	// CleanupIncompleteRecords looks for partner records in Odoo that still have the "inflight" flag set despite being older than `minAge`. Those records are then deleted.
+	// Such records might come into existence due to a partially failed creation request.
 	l := klog.FromContext(ctx)
 	l.Info("Looking for stale inflight partner records...")
 
@@ -298,9 +312,9 @@ func (s *Odoo8Storage) CleanupIncompleteRecords(ctx context.Context, minAge time
 	ids := []int{}
 
 	for _, record := range inflightRecords {
-		updateTime := record.CreationTimestamp.ToTime()
+		createdTime := record.CreationTimestamp.ToTime()
 
-		if updateTime.Before(time.Now().Add(-1 * minAge)) {
+		if createdTime.Before(time.Now().Add(-1 * minAge)) {
 			ids = append(ids, record.ID)
 			l.Info("Preparing to delete inflight partner record", "name", record.Name, "id", record.ID)
 		}

apiserver/billing/odoostorage/odoo/odoo8/odoo8_test.go

@@ -411,8 +411,19 @@ func createStorage(t *testing.T) (*gomock.Controller, *clientmock.MockQueryExecu
 	}
 }
 
+func createFailedRecordScrubber(t *testing.T) (*gomock.Controller, *clientmock.MockQueryExecutor, *FailedRecordScrubber) {
+	ctrl := gomock.NewController(t)
+	mock := clientmock.NewMockQueryExecutor(ctrl)
+
+	return ctrl, mock, &FailedRecordScrubber{
+		sessionCreator: func(ctx context.Context) (client.QueryExecutor, error) {
+			return mock, nil
+		},
+	}
+}
+
 func TestCleanup(t *testing.T) {
-	ctrl, mock, subject := createStorage(t)
+	ctrl, mock, subject := createFailedRecordScrubber(t)
 	defer ctrl.Finish()
 
 	tn := time.Now()

cleanup.go

@@ -9,7 +9,6 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 
 	"github.com/appuio/control-api/apiserver/billing/odoostorage/odoo/odoo8"
-	"github.com/appuio/control-api/apiserver/billing/odoostorage/odoo/odoo8/countries"
 )
 
 // APICommand creates a new command allowing to start the API server
@@ -20,32 +19,17 @@ func CleanupCommand() *cobra.Command {
 
 	odooUrl := cmd.Flags().String("billing-entity-odoo8-url", "http://localhost:8069", "URL of the Odoo instance to use for billing entities")
 	debugTransport := cmd.Flags().Bool("billing-entity-odoo8-debug-transport", false, "Enable debug logging for the Odoo transport")
-	countryList := cmd.Flags().String("billing-entity-odoo8-country-list", "countries.yaml", "Path to the country list file in the format of [{name: \"Germany\", code: \"DE\", id: 81},...]")
-	accountingContactDisplayName := cmd.Flags().String("billing-entity-odoo8-accounting-contact-display-name", "Accounting", "Display name of the accounting contact")
-	languagePreference := cmd.Flags().String("billing-entity-odoo8-language-preference", "en_US", "Language preference of the Odoo record")
-	paymentTerm := cmd.Flags().Int("billing-entity-odoo8-payment-term-id", 2, "Payment term ID of the Odoo record")
 	minAge := cmd.Flags().Duration("billing-entity-odoo8-cleanup-after", time.Hour, "Clean up only records older than this")
 
 	cmd.Run = func(cmd *cobra.Command, args []string) {
 		ctx := ctrl.SetupSignalHandler()
 		l := klog.FromContext(ctx)
-		countryIDs, err := countries.LoadCountryIDs(*countryList)
-		if err != nil {
-			l.Error(err, "Unable to load country list")
-			os.Exit(1)
-		}
-		storage := odoo8.NewOdoo8Storage(
+		scrubber := odoo8.NewFailedRecordScrubber(
 			*odooUrl,
 			*debugTransport,
-			odoo8.Config{
-				AccountingContactDisplayName: *accountingContactDisplayName,
-				LanguagePreference:           *languagePreference,
-				PaymentTermID:                *paymentTerm,
-				CountryIDs:                   countryIDs,
-			},
 		)
 
-		err = storage.CleanupIncompleteRecords(ctx, *minAge)
+		err := scrubber.CleanupIncompleteRecords(ctx, *minAge)
 		if err != nil {
 			l.Error(err, "Unable to clean up incomplete records")
 			os.Exit(1)