Skip to content

Commit

Permalink
storage: change methods to value receiver
Browse files Browse the repository at this point in the history 
Given:

- None of the methods of the `Storage` are mutating the storage
  itself.
- It must be instantiated to be usable, as there is a strict
  reliance on values.
- The struct itself is light.

This seems to be more fitting.

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
  • Loading branch information
@hiddeco
hiddeco committed May 10, 2023
1 parent 777012b commit a4cc55d
Showing 1 changed file with 17 additions and 17 deletions.
34 changes: 17 additions & 17 deletions internal/controller/storage.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func NewStorage(basePath string, hostname string, artifactRetentionTTL time.Dura
}

// NewArtifactFor returns a new v1.Artifact.
func (s *Storage) NewArtifactFor(kind string, metadata metav1.Object, revision, fileName string) v1.Artifact {
func (s Storage) NewArtifactFor(kind string, metadata metav1.Object, revision, fileName string) v1.Artifact {
path := v1.ArtifactPath(kind, metadata.GetNamespace(), metadata.GetName(), fileName)
artifact := v1.Artifact{
Path: path,
Expand Down Expand Up @@ -118,18 +118,18 @@ func (s Storage) SetHostname(URL string) string {
}

// MkdirAll calls os.MkdirAll for the given v1.Artifact base dir.
func (s *Storage) MkdirAll(artifact v1.Artifact) error {
func (s Storage) MkdirAll(artifact v1.Artifact) error {
dir := filepath.Dir(s.LocalPath(artifact))
return os.MkdirAll(dir, 0o700)
}

// Remove calls os.Remove for the given v1.Artifact path.
func (s *Storage) Remove(artifact v1.Artifact) error {
func (s Storage) Remove(artifact v1.Artifact) error {
return os.Remove(s.LocalPath(artifact))
}

// RemoveAll calls os.RemoveAll for the given v1.Artifact base dir.
func (s *Storage) RemoveAll(artifact v1.Artifact) (string, error) {
func (s Storage) RemoveAll(artifact v1.Artifact) (string, error) {
var deletedDir string
dir := filepath.Dir(s.LocalPath(artifact))
// Check if the dir exists.
Expand All @@ -141,7 +141,7 @@ func (s *Storage) RemoveAll(artifact v1.Artifact) (string, error) {
}

// RemoveAllButCurrent removes all files for the given v1.Artifact base dir, excluding the current one.
func (s *Storage) RemoveAllButCurrent(artifact v1.Artifact) ([]string, error) {
func (s Storage) RemoveAllButCurrent(artifact v1.Artifact) ([]string, error) {
deletedFiles := []string{}
localPath := s.LocalPath(artifact)
dir := filepath.Dir(localPath)
Expand Down Expand Up @@ -174,7 +174,7 @@ func (s *Storage) RemoveAllButCurrent(artifact v1.Artifact) ([]string, error) {
// 1. collect all artifact files with an expired ttl
// 2. if we satisfy maxItemsToBeRetained, then return
// 3. else, collect all artifact files till the latest n files remain, where n=maxItemsToBeRetained
func (s *Storage) getGarbageFiles(artifact v1.Artifact, totalCountLimit, maxItemsToBeRetained int, ttl time.Duration) (garbageFiles []string, _ error) {
func (s Storage) getGarbageFiles(artifact v1.Artifact, totalCountLimit, maxItemsToBeRetained int, ttl time.Duration) (garbageFiles []string, _ error) {
localPath := s.LocalPath(artifact)
dir := filepath.Dir(localPath)
artifactFilesWithCreatedTs := make(map[time.Time]string)
Expand Down Expand Up @@ -261,7 +261,7 @@ func (s *Storage) getGarbageFiles(artifact v1.Artifact, totalCountLimit, maxItem

// GarbageCollect removes all garbage files in the artifact dir according to the provided
// retention options.
func (s *Storage) GarbageCollect(ctx context.Context, artifact v1.Artifact, timeout time.Duration) ([]string, error) {
func (s Storage) GarbageCollect(ctx context.Context, artifact v1.Artifact, timeout time.Duration) ([]string, error) {
delFilesChan := make(chan []string)
errChan := make(chan error)
// Abort if it takes more than the provided timeout duration.
Expand Down Expand Up @@ -323,7 +323,7 @@ func stringInSlice(a string, list []string) bool {
}

// ArtifactExist returns a boolean indicating whether the v1.Artifact exists in storage and is a regular file.
func (s *Storage) ArtifactExist(artifact v1.Artifact) bool {
func (s Storage) ArtifactExist(artifact v1.Artifact) bool {
fi, err := os.Lstat(s.LocalPath(artifact))
if err != nil {
return false
Expand All @@ -334,7 +334,7 @@ func (s *Storage) ArtifactExist(artifact v1.Artifact) bool {
// VerifyArtifact verifies if the Digest of the v1.Artifact matches the digest
// of the file in Storage. It returns an error if the digests don't match, or
// if it can't be verified.
func (s *Storage) VerifyArtifact(artifact v1.Artifact) error {
func (s Storage) VerifyArtifact(artifact v1.Artifact) error {
if artifact.Digest == "" {
return fmt.Errorf("artifact has no digest")
}
Expand Down Expand Up @@ -382,7 +382,7 @@ func SourceIgnoreFilter(ps []gitignore.Pattern, domain []string) ArchiveFileFilt
// directories and any ArchiveFileFilter matches. While archiving, any environment specific data (for example,
// the user and group name) is stripped from file headers.
// If successful, it sets the digest and last update time on the artifact.
func (s *Storage) Archive(artifact *v1.Artifact, dir string, filter ArchiveFileFilter) (err error) {
func (s Storage) Archive(artifact *v1.Artifact, dir string, filter ArchiveFileFilter) (err error) {
if f, err := os.Stat(dir); os.IsNotExist(err) || !f.IsDir() {
return fmt.Errorf("invalid dir path: %s", dir)
}
Expand Down Expand Up @@ -504,7 +504,7 @@ func (s *Storage) Archive(artifact *v1.Artifact, dir string, filter ArchiveFileF

// AtomicWriteFile atomically writes the io.Reader contents to the v1.Artifact path.
// If successful, it sets the digest and last update time on the artifact.
func (s *Storage) AtomicWriteFile(artifact *v1.Artifact, reader io.Reader, mode os.FileMode) (err error) {
func (s Storage) AtomicWriteFile(artifact *v1.Artifact, reader io.Reader, mode os.FileMode) (err error) {
localPath := s.LocalPath(*artifact)
tf, err := os.CreateTemp(filepath.Split(localPath))
if err != nil {
Expand Down Expand Up @@ -546,7 +546,7 @@ func (s *Storage) AtomicWriteFile(artifact *v1.Artifact, reader io.Reader, mode

// Copy atomically copies the io.Reader contents to the v1.Artifact path.
// If successful, it sets the digest and last update time on the artifact.
func (s *Storage) Copy(artifact *v1.Artifact, reader io.Reader) (err error) {
func (s Storage) Copy(artifact *v1.Artifact, reader io.Reader) (err error) {
localPath := s.LocalPath(*artifact)
tf, err := os.CreateTemp(filepath.Split(localPath))
if err != nil {
Expand Down Expand Up @@ -584,7 +584,7 @@ func (s *Storage) Copy(artifact *v1.Artifact, reader io.Reader) (err error) {

// CopyFromPath atomically copies the contents of the given path to the path of the v1.Artifact.
// If successful, the digest and last update time on the artifact is set.
func (s *Storage) CopyFromPath(artifact *v1.Artifact, path string) (err error) {
func (s Storage) CopyFromPath(artifact *v1.Artifact, path string) (err error) {
f, err := os.Open(path)
if err != nil {
return err
Expand All @@ -599,7 +599,7 @@ func (s *Storage) CopyFromPath(artifact *v1.Artifact, path string) (err error) {
}

// CopyToPath copies the contents in the (sub)path of the given artifact to the given path.
func (s *Storage) CopyToPath(artifact *v1.Artifact, subPath, toPath string) error {
func (s Storage) CopyToPath(artifact *v1.Artifact, subPath, toPath string) error {
// create a tmp directory to store artifact
tmp, err := os.MkdirTemp("", "flux-include-")
if err != nil {
Expand Down Expand Up @@ -638,7 +638,7 @@ func (s *Storage) CopyToPath(artifact *v1.Artifact, subPath, toPath string) erro
}

// Symlink creates or updates a symbolic link for the given v1.Artifact and returns the URL for the symlink.
func (s *Storage) Symlink(artifact v1.Artifact, linkName string) (string, error) {
func (s Storage) Symlink(artifact v1.Artifact, linkName string) (string, error) {
localPath := s.LocalPath(artifact)
dir := filepath.Dir(localPath)
link := filepath.Join(dir, linkName)
Expand All @@ -660,14 +660,14 @@ func (s *Storage) Symlink(artifact v1.Artifact, linkName string) (string, error)
}

// Lock creates a file lock for the given v1.Artifact.
func (s *Storage) Lock(artifact v1.Artifact) (unlock func(), err error) {
func (s Storage) Lock(artifact v1.Artifact) (unlock func(), err error) {
lockFile := s.LocalPath(artifact) + ".lock"
mutex := lockedfile.MutexAt(lockFile)
return mutex.Lock()
}

// LocalPath returns the secure local path of the given artifact (that is: relative to the Storage.BasePath).
func (s *Storage) LocalPath(artifact v1.Artifact) string {
func (s Storage) LocalPath(artifact v1.Artifact) string {
if artifact.Path == "" {
return ""
}
Expand Down

0 comments on commit a4cc55d

Please sign in to comment.