From 4ea646689632254a9f2a76231e1d77f0a7f41f08 Mon Sep 17 00:00:00 2001 From: Maksym Kryvchun Date: Sat, 5 Oct 2024 19:48:01 +0300 Subject: [PATCH] fix: delete temporary files on exit (#101) --- internal/app/app_test.go | 3 ++- internal/app/stateinitial_test.go | 3 ++- internal/app/stateloaded_test.go | 3 ++- internal/pkg/source/entry_test.go | 29 +++++++++++++------------- internal/pkg/source/source.go | 33 ++++++++++++++++++++++++------ internal/pkg/source/source_test.go | 23 +++++++++++++++++++++ 6 files changed, 71 insertions(+), 23 deletions(-) diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 4b191db..2326704 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -10,6 +10,7 @@ import ( "github.com/charmbracelet/bubbles/cursor" tea "github.com/charmbracelet/bubbletea" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/hedhyw/json-log-viewer/internal/app" @@ -31,7 +32,7 @@ func newTestModel(tb testing.TB, content []byte) tea.Model { require.NoError(tb, err) model = handleUpdate(model, events.LogEntriesUpdateMsg(entries)) - tb.Cleanup(func() { _ = inputSource.Close() }) + tb.Cleanup(func() { assert.NoError(tb, inputSource.Close()) }) return model } diff --git a/internal/app/stateinitial_test.go b/internal/app/stateinitial_test.go index 213a382..2866b64 100644 --- a/internal/app/stateinitial_test.go +++ b/internal/app/stateinitial_test.go @@ -20,7 +20,8 @@ func TestStateInitial(t *testing.T) { inputSource, err := source.Reader(bytes.NewReader([]byte{}), config.GetDefaultConfig()) require.NoError(t, err) - t.Cleanup(func() { _ = inputSource.Close() }) + + t.Cleanup(func() { assert.NoError(t, inputSource.Close()) }) model := app.NewModel( "-", diff --git a/internal/app/stateloaded_test.go b/internal/app/stateloaded_test.go index f84fad3..1129531 100644 --- a/internal/app/stateloaded_test.go +++ b/internal/app/stateloaded_test.go @@ -201,7 +201,8 @@ func BenchmarkStateLoadedBig(b *testing.B) { inputSource, err := source.Reader(contentReader, cfg) require.NoError(b, err) - b.Cleanup(func() { _ = inputSource.Close() }) + + b.Cleanup(func() { assert.NoError(b, inputSource.Close()) }) logEntries, err := inputSource.ParseLogEntries() if err != nil { diff --git a/internal/pkg/source/entry_test.go b/internal/pkg/source/entry_test.go index 654c70d..41dd019 100644 --- a/internal/pkg/source/entry_test.go +++ b/internal/pkg/source/entry_test.go @@ -249,22 +249,24 @@ func TestLazyLogEntriesFilter(t *testing.T) { {"hello":"world"} `, term) - createEntries := func() (*source.Source, source.LazyLogEntries, source.LazyLogEntry) { + createEntries := func(tb testing.TB) (source.LazyLogEntries, source.LazyLogEntry) { source, err := source.Reader(bytes.NewReader([]byte(logs)), config.GetDefaultConfig()) require.NoError(t, err) + tb.Cleanup(func() { assert.NoError(tb, source.Close()) }) + logEntries, err := source.ParseLogEntries() require.NoError(t, err) logEntry := logEntries.Entries[1] - return source, logEntries, logEntry + return logEntries, logEntry } t.Run("all", func(t *testing.T) { t.Parallel() - source, logEntries, _ := createEntries() - defer source.Close() + + logEntries, _ := createEntries(t) assert.Len(t, logEntries.Entries, logEntries.Len()) }) @@ -272,8 +274,7 @@ func TestLazyLogEntriesFilter(t *testing.T) { t.Run("found_exact", func(t *testing.T) { t.Parallel() - source, logEntries, logEntry := createEntries() - defer source.Close() + logEntries, logEntry := createEntries(t) filtered, err := logEntries.Filter(term) require.NoError(t, err) @@ -286,8 +287,7 @@ func TestLazyLogEntriesFilter(t *testing.T) { t.Run("found_ignore_case", func(t *testing.T) { t.Parallel() - source, logEntries, logEntry := createEntries() - defer source.Close() + logEntries, logEntry := createEntries(t) filtered, err := logEntries.Filter(strings.ToUpper(term)) require.NoError(t, err) @@ -300,8 +300,7 @@ func TestLazyLogEntriesFilter(t *testing.T) { t.Run("empty", func(t *testing.T) { t.Parallel() - source, logEntries, _ := createEntries() - defer source.Close() + logEntries, _ := createEntries(t) filtered, err := logEntries.Filter("") require.NoError(t, err) @@ -311,8 +310,7 @@ func TestLazyLogEntriesFilter(t *testing.T) { t.Run("not_found", func(t *testing.T) { t.Parallel() - source, logEntries, _ := createEntries() - defer source.Close() + logEntries, _ := createEntries(t) filtered, err := logEntries.Filter(term + " - not found!") require.NoError(t, err) @@ -323,8 +321,7 @@ func TestLazyLogEntriesFilter(t *testing.T) { t.Run("seeker_failed", func(t *testing.T) { t.Parallel() - source, logEntries, _ := createEntries() - defer source.Close() + logEntries, _ := createEntries(t) fileName := tests.RequireCreateFile(t, []byte("")) @@ -622,6 +619,8 @@ func parseLazyLogEntry(tb testing.TB, value string, cfg *config.Config) source.L source, err := source.Reader(strings.NewReader(value), cfg) require.NoError(tb, err) + tb.Cleanup(func() { assert.NoError(tb, source.Close()) }) + logEntries, err := source.ParseLogEntries() require.NoError(tb, err) require.Equal(tb, 1, logEntries.Len()) @@ -635,6 +634,8 @@ func parseTableRow(tb testing.TB, value string, cfg *config.Config) table.Row { source, err := source.Reader(strings.NewReader(value+"\n"), cfg) require.NoError(tb, err) + tb.Cleanup(func() { assert.NoError(tb, source.Close()) }) + logEntries, err := source.ParseLogEntries() require.NoError(tb, err) require.Equal(tb, 1, logEntries.Len(), value) diff --git a/internal/pkg/source/source.go b/internal/pkg/source/source.go index 3836986..5251e36 100644 --- a/internal/pkg/source/source.go +++ b/internal/pkg/source/source.go @@ -16,7 +16,9 @@ import ( const ( maxLineSize = 8 * 1024 * 1024 - temporaryFilePattern = "jvl-*.log" + temporaryFilePattern = "jlv-*.log" + + ErrFileTruncated semerr.Error = "file truncated" ) type Source struct { @@ -34,10 +36,29 @@ type Source struct { name string // maxSize is the maximum size of the file we will read. maxSize int64 + // temporaryFiles to remove at the end. + temporaryFiles []string } +// Close implements io.Closer. +// +// It closes and removes temporary files. func (s *Source) Close() error { - return errors.Join(s.file.Close(), s.Seeker.Close()) + errMulti := make([]error, 0, 2+len(s.temporaryFiles)) + + if s.file != nil { + errMulti = append(errMulti, s.file.Close()) + } + + if s.Seeker != nil { + errMulti = append(errMulti, s.Seeker.Close()) + } + + for _, f := range s.temporaryFiles { + errMulti = append(errMulti, os.Remove(f)) + } + + return errors.Join(errMulti...) } // File creates a new Source for reading log entries from a file. @@ -56,7 +77,7 @@ func File(name string, cfg *config.Config) (*Source, error) { source.Seeker, err = os.Open(name) if err != nil { - return nil, errors.Join(err, source.file.Close()) + return nil, errors.Join(err, source.Close()) } source.reader = bufio.NewReaderSize( @@ -85,13 +106,15 @@ func Reader(input io.Reader, cfg *config.Config) (*Source, error) { return nil, fmt.Errorf("creating temporary file: %w", err) } + source.temporaryFiles = append(source.temporaryFiles, source.file.Name()) + // The io.TeeReader will write the input to the is.file as it is read. reader := io.TeeReader(input, source.file) // We can now seek against the data that is read in the input io.Reader. source.Seeker, err = os.Open(source.file.Name()) if err != nil { - return nil, errors.Join(err, source.file.Close()) + return nil, errors.Join(err, source.Close()) } reader = io.LimitReader(reader, source.maxSize) @@ -125,8 +148,6 @@ func (s *Source) CanFollow() bool { return len(s.name) != 0 } -const ErrFileTruncated semerr.Error = "file truncated" - // readLogEntry reads the next LazyLogEntry from the file. func (s *Source) readLogEntry() (LazyLogEntry, error) { for { diff --git a/internal/pkg/source/source_test.go b/internal/pkg/source/source_test.go index e239de6..1ab9048 100644 --- a/internal/pkg/source/source_test.go +++ b/internal/pkg/source/source_test.go @@ -2,6 +2,7 @@ package source_test import ( "bytes" + "os" "strings" "testing" "testing/iotest" @@ -80,6 +81,7 @@ func TestParseLogEntriesFromReaderLimited(t *testing.T) { inputSource, err := source.Reader(reader, cfg) require.NoError(t, err) + defer func() { assert.NoError(t, inputSource.Close()) }() logEntries, err := inputSource.ParseLogEntries() @@ -124,6 +126,12 @@ func TestFile(t *testing.T) { require.NoError(t, err) assert.True(t, source.CanFollow()) + + err = source.Close() + require.NoError(t, err) + + _, err = os.Stat(fileName) + require.NoError(t, err, "The file should exist after closing") }) t.Run("not_found", func(t *testing.T) { @@ -133,3 +141,18 @@ func TestFile(t *testing.T) { require.Error(t, err) }) } + +func TestReaderTemporaryFilesDeleted(t *testing.T) { + t.Parallel() + + source, err := source.Reader( + bytes.NewReader([]byte(t.Name())), + config.GetDefaultConfig(), + ) + require.NoError(t, err) + require.NoError(t, source.Close()) + + _, err = os.Stat(source.Seeker.Name()) + require.Error(t, err) + assert.True(t, os.IsNotExist(err), err) +}