Skip to content

Commit

Permalink
Fix linting with new golangci-lint
Browse files Browse the repository at this point in the history
Co-authored-by: Jean-Baptiste Lallement <[email protected]>
  • Loading branch information
didrocks and jibel committed Sep 14, 2023
1 parent 21e8e64 commit 1c58c51
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 32 deletions.
30 changes: 17 additions & 13 deletions internal/cache/db.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Package cache handles transaction with an underlying database to cache user and group informations.
package cache

import (
Expand Down Expand Up @@ -109,7 +110,7 @@ func New(cacheDir string) (cache *Cache, err error) {
break
}

if errors.Is(err, errRetryDB{}) {
if errors.Is(err, shouldRetryDBError{}) {
if i == 3 {
return nil, errors.Unwrap(err)
}
Expand Down Expand Up @@ -171,7 +172,7 @@ func openAndInitDB(path, dirtyFlagPath string) (*bbolt.DB, error) {
if err != nil {
if errors.Is(err, bbolt.ErrInvalid) {
cleanDatabase(path, dirtyFlagPath)
return nil, errRetryDB{err: err}
return nil, shouldRetryDBError{err: err}
}
return nil, fmt.Errorf("can't open database file: %v", err)
}
Expand All @@ -197,13 +198,16 @@ func openAndInitDB(path, dirtyFlagPath string) (*bbolt.DB, error) {

// Clean up any unknown buckets
var bucketNamesToDelete [][]byte
tx.ForEach(func(name []byte, b *bbolt.Bucket) error {
err = tx.ForEach(func(name []byte, b *bbolt.Bucket) error {
if slices.Contains(allBucketsNames, string(name)) {
return nil
}
bucketNamesToDelete = append(bucketNamesToDelete, name)
return nil
})
if err != nil {
return err
}
for _, bucketName := range bucketNamesToDelete {
// We are in a RW transaction.
_ = tx.DeleteBucket(bucketName)
Expand Down Expand Up @@ -295,7 +299,7 @@ func getFromBucket[T any, K int | string](bucket bucketWithName, key K) (T, erro

data := bucket.Get(k)
if data == nil {
return r, ErrNoDataFound{key: string(k), bucketName: bucket.name}
return r, NoDataFoundError{key: string(k), bucketName: bucket.name}
}

if err := json.Unmarshal(data, &r); err != nil {
Expand All @@ -305,34 +309,34 @@ func getFromBucket[T any, K int | string](bucket bucketWithName, key K) (T, erro
return r, nil
}

// ErrNoDataFound is returned when we didn’t find a matching entry.
type ErrNoDataFound struct {
// NoDataFoundError is returned when we didn’t find a matching entry.
type NoDataFoundError struct {
key string
bucketName string
}

// Error implements the error interface to return key/bucket name.
func (err ErrNoDataFound) Error() string {
func (err NoDataFoundError) Error() string {
return fmt.Sprintf("no result matching %v in %v", err.key, err.bucketName)
}

// Is makes this error insensitive to the key and bucket name.
func (ErrNoDataFound) Is(target error) bool { return target == ErrNoDataFound{} }
func (NoDataFoundError) Is(target error) bool { return target == NoDataFoundError{} }

// errRetryDB is returned when we want to retry opening the database.
type errRetryDB struct {
// shouldRetryDBError is returned when we want to retry opening the database.
type shouldRetryDBError struct {
err error
}

// Error implements the error interface.
func (err errRetryDB) Error() string {
func (err shouldRetryDBError) Error() string {
return "ErrRetryDB"
}

// Unwrap allows to unwrap original error.
func (err errRetryDB) Unwrap() error {
func (err shouldRetryDBError) Unwrap() error {
return err.err
}

// Is makes this error insensitive to the key and bucket name.
func (errRetryDB) Is(target error) bool { return target == errRetryDB{} }
func (shouldRetryDBError) Is(target error) bool { return target == shouldRetryDBError{} }
17 changes: 8 additions & 9 deletions internal/cache/db_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cache_test

import (
"errors"
"io"
"io/fs"
"os"
Expand Down Expand Up @@ -248,9 +249,8 @@ func TestUpdateFromUserInfo(t *testing.T) {
requireCleanedDatabase(t, c)
}
return
} else {
require.NoError(t, err)
}
require.NoError(t, err)

requireNoDirtyFileInDir(t, cacheDir)

Expand All @@ -259,7 +259,6 @@ func TestUpdateFromUserInfo(t *testing.T) {

want := testutils.LoadWithUpdateFromGolden(t, got)
require.Equal(t, want, got, "Did not get expected database content")

})
}
}
Expand All @@ -274,7 +273,7 @@ func TestUserByID(t *testing.T) {
}{
"Get existing user": {dbFile: "one_user_and_group"},

"Error on missing user": {wantErrType: cache.ErrNoDataFound{}},
"Error on missing user": {wantErrType: cache.NoDataFoundError{}},
"Error on invalid database entry": {dbFile: "invalid_entry_in_userByID", wantErrType: shouldError{}},
}
for name, tc := range tests {
Expand All @@ -300,7 +299,7 @@ func TestUserByName(t *testing.T) {
}{
"Get existing user": {dbFile: "one_user_and_group"},

"Error on missing user": {wantErrType: cache.ErrNoDataFound{}},
"Error on missing user": {wantErrType: cache.NoDataFoundError{}},
"Error on invalid database entry": {dbFile: "invalid_entry_in_userByName", wantErrType: shouldError{}},
}
for name, tc := range tests {
Expand Down Expand Up @@ -354,7 +353,7 @@ func TestGroupByID(t *testing.T) {
}{
"Get existing group": {dbFile: "one_user_and_group"},

"Error on missing group": {wantErrType: cache.ErrNoDataFound{}},
"Error on missing group": {wantErrType: cache.NoDataFoundError{}},
"Error on invalid database entry": {dbFile: "invalid_entry_in_groupByID", wantErrType: shouldError{}},
"Error as missing userByID": {dbFile: "partially_valid_multiple_users_and_groups_groupByID_groupToUsers", wantErrType: shouldError{}},
}
Expand All @@ -381,7 +380,7 @@ func TestGroupByName(t *testing.T) {
}{
"Get existing group": {dbFile: "one_user_and_group"},

"Error on missing group": {wantErrType: cache.ErrNoDataFound{}},
"Error on missing group": {wantErrType: cache.NoDataFoundError{}},
"Error on invalid database entry": {dbFile: "invalid_entry_in_groupByName", wantErrType: shouldError{}},
"Error as missing userByID": {dbFile: "partially_valid_multiple_users_and_groups_groupByID_groupToUsers", wantErrType: shouldError{}},
}
Expand Down Expand Up @@ -459,7 +458,7 @@ UserToGroups: {}

got, err := dumpToYaml(c)
require.NoError(t, err, "Created database should be valid yaml content")
require.Equal(t, want, string(got), "Database should only have empty buckets")
require.Equal(t, want, got, "Database should only have empty buckets")
}

//go:linkname dumpToYaml github.com/ubuntu/authd/internal/cache.(*Cache).dumpToYaml
Expand Down Expand Up @@ -498,7 +497,7 @@ func requireGetAssertions[E any](t *testing.T, got E, wantErrType, err error, c
t.Helper()

if wantErrType != nil {
if (wantErrType == cache.ErrNoDataFound{}) {
if (errors.Is(wantErrType, cache.NoDataFoundError{})) {
require.ErrorIs(t, err, wantErrType, "Should return no data found")
return
}
Expand Down
4 changes: 2 additions & 2 deletions internal/cache/export_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package cache

const (
// DbName is dbName exported for tests
// DbName is dbName exported for tests.
DbName = dbName
// DirtyFlagDbName is dirtyFlagDbName exported for tests
// DirtyFlagDbName is dirtyFlagDbName exported for tests.
DirtyFlagDbName = dirtyFlagDbName
)

Expand Down
2 changes: 1 addition & 1 deletion internal/cache/getgroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func getGroup[K int | string](c *Cache, bucketName string, key K) (Group, error)
g, err := getFromBucket[groupDB](buckets[bucketName], key)
if err != nil {
// no entry is valid, no need to clean the database but return the error.
if !errors.Is(err, ErrNoDataFound{}) {
if !errors.Is(err, NoDataFoundError{}) {
c.requestCleanDatabase()
}
return err
Expand Down
2 changes: 1 addition & 1 deletion internal/cache/getusers.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func getUser[K int | string](c *Cache, bucketName string, key K) (u userDB, err

u, err = getFromBucket[userDB](bucket, key)
if err != nil {
if !errors.Is(err, ErrNoDataFound{}) {
if !errors.Is(err, NoDataFoundError{}) {
c.requestCleanDatabase()
}
return err
Expand Down
7 changes: 7 additions & 0 deletions internal/cache/serialization.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"gopkg.in/yaml.v3"
)

//nolint:unused // This is used for tests, with methods that are using go linking. Not part of exported API.
var redactedTimes = map[string]string{
"AAAAATIME": "2004-10-20T11:06:23Z",
"BBBBBTIME": "2006-06-01T10:08:04Z",
Expand All @@ -21,6 +22,8 @@ var redactedTimes = map[string]string{
}

// redactTime replace current time by a redacted version.
//
//nolint:unused // This is used for tests, with methods that are using go linking. Not part of exported API.
func redactTime(line string) string {
re := regexp.MustCompile(`"LastLogin":"(.*?)"`)
match := re.FindSubmatch([]byte(line))
Expand Down Expand Up @@ -51,6 +54,8 @@ func redactTime(line string) string {
}

// dumpToYaml deserializes the cache database to a writer in a yaml format.
//
//nolint:unused // This is used for tests, with go linking. Not part of exported API.
func (c *Cache) dumpToYaml() (string, error) {
d := make(map[string]map[string]string)

Expand All @@ -75,6 +80,8 @@ func (c *Cache) dumpToYaml() (string, error) {
}

// dbfromYAML loads a yaml formatted of the buckets and dump it into destDir, with its dbname.
//
//nolint:unused // This is used for tests, with go linking. Not part of exported API.
func dbfromYAML(r io.Reader, destDir string) error {
dbPath := filepath.Join(destDir, dbName)
db, err := bbolt.Open(dbPath, 0600, nil)
Expand Down
11 changes: 5 additions & 6 deletions internal/cache/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (c *Cache) UpdateFromUserInfo(u UserInfo) error {

previousGroupsForCurrentUser, err := getFromBucket[userToGroupsDB](buckets[userToGroupsBucketName], userDB.UID)
// No data is valid and means this is the first insertion.
if err != nil && !errors.Is(err, ErrNoDataFound{}) {
if err != nil && !errors.Is(err, NoDataFoundError{}) {
c.requestCleanDatabase()
return err
}
Expand Down Expand Up @@ -99,7 +99,7 @@ func (c *Cache) UpdateFromUserInfo(u UserInfo) error {
// updateUser updates both user buckets with userContent. It handles any potential login rename.
func updateUser(buckets map[string]bucketWithName, userContent userDB) {
existingUser, err := getFromBucket[userDB](buckets[userByIDBucketName], userContent.UID)
if err != nil && !errors.Is(err, ErrNoDataFound{}) {
if err != nil && !errors.Is(err, NoDataFoundError{}) {
slog.Warn(fmt.Sprintf("Could not fetch previous record for user %v: %v", userContent.UID, err))
}

Expand All @@ -117,7 +117,7 @@ func updateUser(buckets map[string]bucketWithName, userContent userDB) {
func updateGroups(buckets map[string]bucketWithName, groupContents []groupDB) {
for _, groupContent := range groupContents {
existingGroup, err := getFromBucket[groupDB](buckets[groupByIDBucketName], groupContent.GID)
if err != nil && !errors.Is(err, ErrNoDataFound{}) {
if err != nil && !errors.Is(err, NoDataFoundError{}) {
slog.Warn(fmt.Sprintf("Could not fetch previous record for group %v: %v", groupContent.GID, err))
}

Expand All @@ -140,7 +140,7 @@ func updateUsersAndGroups(buckets map[string]bucketWithName, uid int, groupConte
currentGIDs = append(currentGIDs, groupContent.GID)
grpToUsers, err := getFromBucket[groupToUsersDB](buckets[groupToUsersBucketName], groupContent.GID)
// No data is valid and means that this is the first time we record it.
if err != nil && !errors.Is(err, ErrNoDataFound{}) {
if err != nil && !errors.Is(err, NoDataFoundError{}) {
return err
}

Expand All @@ -159,7 +159,7 @@ func updateUsersAndGroups(buckets map[string]bucketWithName, uid int, groupConte
}
grpToUsers, err := getFromBucket[groupToUsersDB](buckets[groupToUsersBucketName], previousGID)
// No data means the database was corrupted but we can forgive this (exist in previous user gid but not on system).
if err != nil && !errors.Is(err, ErrNoDataFound{}) {
if err != nil && !errors.Is(err, NoDataFoundError{}) {
return err
}

Expand All @@ -182,7 +182,6 @@ func updateUsersAndGroups(buckets map[string]bucketWithName, uid int, groupConte
_ = buckets[groupByIDBucketName].Delete([]byte(strconv.Itoa(previousGID)))
_ = buckets[groupByNameBucketName].Delete([]byte(group.Name))
_ = buckets[groupToUsersBucketName].Delete([]byte(strconv.Itoa(previousGID)))

}

return nil
Expand Down

0 comments on commit 1c58c51

Please sign in to comment.