Merge pull request #558 from cybertec-postgresql/fix-unused-code

pashagolub · web-flow · commit ef1ae68f6523 · 2023-03-31T14:59:24.000+02:00
[*] remove unused parameters and code
diff --git a/internal/api/api_test.go b/internal/api/api_test.go
@@ -20,14 +20,14 @@ func (r *apihandler) IsReady() bool {
 	return true
 }
 
-func (r *apihandler) StartChain(ctx context.Context, chainID int) error {
+func (r *apihandler) StartChain(_ context.Context, chainID int) error {
 	if chainID == 0 {
 		return errors.New("invalid chain id")
 	}
 	return nil
 }
 
-func (r *apihandler) StopChain(ctx context.Context, chainID int) error {
+func (r *apihandler) StopChain(context.Context, int) error {
 	return nil
 }
 
diff --git a/internal/log/log_test.go b/internal/log/log_test.go
@@ -31,7 +31,7 @@ func TestFileLogger(t *testing.T) {
 	_ = os.Remove("test.log")
 }
 
-func TestPgxLog(t *testing.T) {
+func TestPgxLog(*testing.T) {
 	pgxl := log.NewPgxLogger(log.Init(config.LoggingOpts{LogLevel: "trace"}))
 	var level tracelog.LogLevel
 	for level = tracelog.LogLevelNone; level <= tracelog.LogLevelTrace; level++ {
diff --git a/internal/migrator/migrator_test.go b/internal/migrator/migrator_test.go
@@ -61,11 +61,7 @@ func migrateTest() error {
 		return err
 	}
 	defer db.Release()
-	if err := migrator.Migrate(ctx, db.Conn()); err != nil {
-		return err
-	}
-
-	return nil
+	return migrator.Migrate(ctx, db.Conn())
 }
 
 func mustMigrator(migrator *migrator.Migrator, err error) *migrator.Migrator {
diff --git a/internal/pgengine/migration.go b/internal/pgengine/migration.go
@@ -24,10 +24,7 @@ func (pge *PgEngine) MigrateDb(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	if err := m.Migrate(ctx, conn.Conn()); err != nil {
-		return err
-	}
-	return nil
+	return m.Migrate(ctx, conn.Conn())
 }
 
 // CheckNeedMigrateDb checks need of upgrading database and throws error if that's true
diff --git a/internal/pgengine/pgengine_test.go b/internal/pgengine/pgengine_test.go
@@ -143,7 +143,7 @@ func TestSchedulerFunctions(t *testing.T) {
 		tx, txid, err := pge.StartTransaction(ctx, 0)
 		assert.NoError(t, err, "Should start transaction")
 		assert.Greater(t, txid, 0, "Should return transaction id")
-		assert.NoError(t, pge.GetChainElements(ctx, tx, &chains, 0), "Should no error in clean database")
+		assert.NoError(t, pge.GetChainElements(ctx, &chains, 0), "Should no error in clean database")
 		assert.Empty(t, chains, "Should be empty in clean database")
 		pge.CommitTransaction(ctx, tx)
 	})
@@ -153,7 +153,7 @@ func TestSchedulerFunctions(t *testing.T) {
 		tx, txid, err := pge.StartTransaction(ctx, 0)
 		assert.NoError(t, err, "Should start transaction")
 		assert.Greater(t, txid, 0, "Should return transaction id")
-		assert.NoError(t, pge.GetChainParamValues(ctx, tx, &paramVals, &pgengine.ChainTask{
+		assert.NoError(t, pge.GetChainParamValues(ctx, &paramVals, &pgengine.ChainTask{
 			TaskID:  0,
 			ChainID: 0}), "Should no error in clean database")
 		assert.Empty(t, paramVals, "Should be empty in clean database")
diff --git a/internal/pgengine/transaction.go b/internal/pgengine/transaction.go
@@ -108,7 +108,7 @@ func (pge *PgEngine) MustRollbackToSavepoint(ctx context.Context, tx pgx.Tx, sav
 }
 
 // GetChainElements returns all elements for a given chain
-func (pge *PgEngine) GetChainElements(ctx context.Context, tx pgx.Tx, chainTasks *[]ChainTask, chainID int) error {
+func (pge *PgEngine) GetChainElements(ctx context.Context, chainTasks *[]ChainTask, chainID int) error {
 	const sqlSelectChainTasks = `SELECT task_id, command, kind, run_as, ignore_error, autonomous, database_connection, timeout
 FROM timetable.task WHERE chain_id = $1 ORDER BY task_order ASC`
 	// return Select(ctx, tx, chainTasks, sqlSelectChainTasks, chainID)
@@ -121,7 +121,7 @@ FROM timetable.task WHERE chain_id = $1 ORDER BY task_order ASC`
 }
 
 // GetChainParamValues returns parameter values to pass for task being executed
-func (pge *PgEngine) GetChainParamValues(ctx context.Context, tx pgx.Tx, paramValues *[]string, task *ChainTask) error {
+func (pge *PgEngine) GetChainParamValues(ctx context.Context, paramValues *[]string, task *ChainTask) error {
 	const sqlGetParamValues = `SELECT value FROM timetable.parameter WHERE task_id = $1 AND value IS NOT NULL ORDER BY order_id ASC`
 	// return Select(ctx, tx, paramValues, sqlGetParamValues, task.TaskID)
 	rows, err := pge.ConfigDb.Query(ctx, sqlGetParamValues, task.TaskID)
diff --git a/internal/pgengine/transaction_test.go b/internal/pgengine/transaction_test.go
@@ -136,32 +136,20 @@ func TestGetChainElements(t *testing.T) {
 	pge := pgengine.NewDB(mockPool, "pgengine_unit_test")
 	ctx := context.Background()
 
-	mockPool.ExpectBegin()
 	mockPool.ExpectQuery("SELECT").WithArgs(0).WillReturnError(errors.New("error"))
-	tx, err := mockPool.Begin(ctx)
-	assert.NoError(t, err)
-	assert.Error(t, pge.GetChainElements(ctx, tx, &[]pgengine.ChainTask{}, 0))
+	assert.Error(t, pge.GetChainElements(ctx, &[]pgengine.ChainTask{}, 0))
 
-	mockPool.ExpectBegin()
 	mockPool.ExpectQuery("SELECT").WithArgs(0).WillReturnRows(
 		pgxmock.NewRows([]string{"task_id", "command", "kind", "run_as",
 			"ignore_error", "autonomous", "database_connection", "timeout"}).
 			AddRow(24, "foo", "sql", "user", false, false, "postgres://foo@boo/bar", 0))
-	tx, err = mockPool.Begin(ctx)
-	assert.NoError(t, err)
-	assert.NoError(t, pge.GetChainElements(ctx, tx, &[]pgengine.ChainTask{}, 0))
+	assert.NoError(t, pge.GetChainElements(ctx, &[]pgengine.ChainTask{}, 0))
 
-	mockPool.ExpectBegin()
 	mockPool.ExpectQuery("SELECT").WithArgs(0).WillReturnError(errors.New("error"))
-	tx, err = mockPool.Begin(ctx)
-	assert.NoError(t, err)
-	assert.Error(t, pge.GetChainParamValues(ctx, tx, &[]string{}, &pgengine.ChainTask{}))
+	assert.Error(t, pge.GetChainParamValues(ctx, &[]string{}, &pgengine.ChainTask{}))
 
-	mockPool.ExpectBegin()
 	mockPool.ExpectQuery("SELECT").WithArgs(0).WillReturnRows(pgxmock.NewRows([]string{"s"}).AddRow("foo"))
-	tx, err = mockPool.Begin(ctx)
-	assert.NoError(t, err)
-	assert.NoError(t, pge.GetChainParamValues(ctx, tx, &[]string{}, &pgengine.ChainTask{}))
+	assert.NoError(t, pge.GetChainParamValues(ctx, &[]string{}, &pgengine.ChainTask{}))
 }
 
 func TestSetRole(t *testing.T) {
diff --git a/internal/scheduler/chain.go b/internal/scheduler/chain.go
@@ -188,7 +188,7 @@ func (sch *Scheduler) executeChain(ctx context.Context, chain Chain) {
 	}
 	chainL = chainL.WithField("txid", txid)
 
-	err = sch.pgengine.GetChainElements(ctx, tx, &ChainTasks, chain.ChainID)
+	err = sch.pgengine.GetChainElements(ctx, &ChainTasks, chain.ChainID)
 	if err != nil {
 		chainL.WithError(err).Error("Failed to retrieve chain elements")
 		sch.pgengine.RollbackTransaction(ctx, tx)
@@ -236,7 +236,7 @@ func (sch *Scheduler) executeСhainElement(ctx context.Context, tx pgx.Tx, task
 
 	l := log.GetLogger(ctx)
 
-	err = sch.pgengine.GetChainParamValues(ctx, tx, &paramValues, task)
+	err = sch.pgengine.GetChainParamValues(ctx, &paramValues, task)
 	if err != nil {
 		l.WithError(err).Error("cannot fetch parameters values for chain: ", err)
 		return -1
diff --git a/internal/scheduler/chain_test.go b/internal/scheduler/chain_test.go
@@ -16,7 +16,7 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestSchedulerExclusiveLocking(t *testing.T) {
+func TestSchedulerExclusiveLocking(*testing.T) {
 	sch := &Scheduler{exclusiveMutex: sync.RWMutex{}}
 	sch.Lock(true)
 	sch.Unlock(true)
diff --git a/internal/scheduler/shell_test.go b/internal/scheduler/shell_test.go
@@ -19,7 +19,7 @@ import (
 type testCommander struct{}
 
 // overwrite CombinedOutput function of os/exec so only parameter syntax and return codes are checked...
-func (c testCommander) CombinedOutput(ctx context.Context, command string, args ...string) ([]byte, error) {
+func (c testCommander) CombinedOutput(_ context.Context, command string, args ...string) ([]byte, error) {
 	if strings.HasPrefix(command, "ping") {
 		return []byte(fmt.Sprint(command, args)), nil
 	}
diff --git a/internal/scheduler/tasks.go b/internal/scheduler/tasks.go
@@ -43,26 +43,30 @@ func (sch *Scheduler) executeTask(ctx context.Context, name string, paramValues
 	return
 }
 
-func taskNoOp(ctx context.Context, sch *Scheduler, val string) (stdout string, err error) {
+func taskNoOp(_ context.Context, _ *Scheduler, val string) (stdout string, err error) {
 	return "NoOp task called with value: " + val, nil
 }
 
-func taskSleep(ctx context.Context, sch *Scheduler, val string) (stdout string, err error) {
+func taskSleep(ctx context.Context, _ *Scheduler, val string) (stdout string, err error) {
 	var d int
 	if d, err = strconv.Atoi(val); err != nil {
 		return "", err
 	}
 	dur := time.Duration(d) * time.Second
-	time.Sleep(dur)
-	return "Sleep task called for " + dur.String(), nil
+	select {
+	case <-ctx.Done():
+		return "", ctx.Err()
+	case <-time.After(dur):
+		return "Sleep task called for " + dur.String(), nil
+	}
 }
 
-func taskLog(ctx context.Context, sch *Scheduler, val string) (stdout string, err error) {
+func taskLog(ctx context.Context, _ *Scheduler, val string) (stdout string, err error) {
 	log.GetLogger(ctx).Print(val)
 	return "Logged: " + val, nil
 }
 
-func taskSendMail(ctx context.Context, sch *Scheduler, paramValues string) (stdout string, err error) {
+func taskSendMail(ctx context.Context, _ *Scheduler, paramValues string) (stdout string, err error) {
 	conn := tasks.EmailConn{ServerPort: 587, ContentType: "text/plain"}
 	if err := json.Unmarshal([]byte(paramValues), &conn); err != nil {
 		return "", err
@@ -102,7 +106,7 @@ func taskCopyToFile(ctx context.Context, sch *Scheduler, val string) (stdout str
 	return stdout, err
 }
 
-func taskDownload(ctx context.Context, sch *Scheduler, paramValues string) (stdout string, err error) {
+func taskDownload(ctx context.Context, _ *Scheduler, paramValues string) (stdout string, err error) {
 	type downloadOpts struct {
 		WorkersNum int      `json:"workersnum"`
 		FileUrls   []string `json:"fileurls"`
@@ -118,8 +122,8 @@ func taskDownload(ctx context.Context, sch *Scheduler, paramValues string) (stdo
 	return tasks.DownloadUrls(ctx, opts.FileUrls, opts.DestPath, opts.WorkersNum)
 }
 
-func taskShutdown(ctx context.Context, sch *Scheduler, val string) (stdout string, err error) {
-	sch.l.Debug("Shutdown command received...")
+func taskShutdown(_ context.Context, sch *Scheduler, val string) (stdout string, err error) {
+	sch.l.WithField("message", val).Info("Shutdown command received")
 	sch.Shutdown()
 	return "Shutdown task called", nil
 }
diff --git a/internal/tasks/mail_test.go b/internal/tasks/mail_test.go
@@ -12,7 +12,7 @@ type fakeDialer struct {
 	Dialer
 }
 
-func (d *fakeDialer) DialAndSend(ctx context.Context, m ...*gomail.Message) error {
+func (d *fakeDialer) DialAndSend(context.Context, ...*gomail.Message) error {
 	return nil
 }
 

Original file line number	Diff line number	Diff line change
`@@ -20,14 +20,14 @@ func (r *apihandler) IsReady() bool {`
`20`	`20`	`return true`
`21`	`21`	`}`
`22`	`22`
`23`		`-func (r *apihandler) StartChain(ctx context.Context, chainID int) error {`
	`23`	`+func (r *apihandler) StartChain(_ context.Context, chainID int) error {`
`24`	`24`	`if chainID == 0 {`
`25`	`25`	`return errors.New("invalid chain id")`
`26`	`26`	`}`
`27`	`27`	`return nil`
`28`	`28`	`}`
`29`	`29`
`30`		`-func (r *apihandler) StopChain(ctx context.Context, chainID int) error {`
	`30`	`+func (r *apihandler) StopChain(context.Context, int) error {`
`31`	`31`	`return nil`
`32`	`32`	`}`
`33`	`33`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ func TestFileLogger(t *testing.T) {`
`31`	`31`	`_ = os.Remove("test.log")`
`32`	`32`	`}`
`33`	`33`
`34`		`-func TestPgxLog(t *testing.T) {`
	`34`	`+func TestPgxLog(*testing.T) {`
`35`	`35`	`pgxl := log.NewPgxLogger(log.Init(config.LoggingOpts{LogLevel: "trace"}))`
`36`	`36`	`var level tracelog.LogLevel`
`37`	`37`	`for level = tracelog.LogLevelNone; level <= tracelog.LogLevelTrace; level++ {`
Original file line number	Diff line number	Diff line change
`@@ -61,11 +61,7 @@ func migrateTest() error {`
`61`	`61`	`return err`
`62`	`62`	`}`
`63`	`63`	`defer db.Release()`
`64`		`- if err := migrator.Migrate(ctx, db.Conn()); err != nil {`
`65`		`- return err`
`66`		`- }`
`67`		`-`
`68`		`- return nil`
	`64`	`+ return migrator.Migrate(ctx, db.Conn())`
`69`	`65`	`}`
`70`	`66`
`71`	`67`	`func mustMigrator(migrator migrator.Migrator, err error) migrator.Migrator {`
Original file line number	Diff line number	Diff line change
`@@ -24,10 +24,7 @@ func (pge *PgEngine) MigrateDb(ctx context.Context) error {`
`24`	`24`	`if err != nil {`
`25`	`25`	`return err`
`26`	`26`	`}`
`27`		`- if err := m.Migrate(ctx, conn.Conn()); err != nil {`
`28`		`- return err`
`29`		`- }`
`30`		`- return nil`
	`27`	`+ return m.Migrate(ctx, conn.Conn())`
`31`	`28`	`}`
`32`	`29`
`33`	`30`	`// CheckNeedMigrateDb checks need of upgrading database and throws error if that's true`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ import (`
`16`	`16`	`"github.com/stretchr/testify/assert"`
`17`	`17`	`)`
`18`	`18`
`19`		`-func TestSchedulerExclusiveLocking(t *testing.T) {`
	`19`	`+func TestSchedulerExclusiveLocking(*testing.T) {`
`20`	`20`	`sch := &Scheduler{exclusiveMutex: sync.RWMutex{}}`
`21`	`21`	`sch.Lock(true)`
`22`	`22`	`sch.Unlock(true)`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ import (`
`19`	`19`	`type testCommander struct{}`
`20`	`20`
`21`	`21`	`// overwrite CombinedOutput function of os/exec so only parameter syntax and return codes are checked...`
`22`		`-func (c testCommander) CombinedOutput(ctx context.Context, command string, args ...string) ([]byte, error) {`
	`22`	`+func (c testCommander) CombinedOutput(_ context.Context, command string, args ...string) ([]byte, error) {`
`23`	`23`	`if strings.HasPrefix(command, "ping") {`
`24`	`24`	`return []byte(fmt.Sprint(command, args)), nil`
`25`	`25`	`}`