Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: file exporter path handling #2498

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions exporter/fileexporter/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"context"
"fmt"
"os"
"path/filepath"
"runtime"
"strings"
"sync"
Expand All @@ -23,7 +24,6 @@
Format string

// OutputDir is the location of the directory where to store the exported files
// It should finish with a /
// Default: the current directory
OutputDir string

Expand Down Expand Up @@ -72,7 +72,19 @@
return err
}

filePath := f.OutputDir + "/" + filename
// Handle empty OutputDir and remove trailing slash
outputDir := strings.TrimRight(f.OutputDir, "/")

var filePath string
if outputDir == "" {
filePath = filename
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a test case for when we have no outputDir.

} else {
// Ensure OutputDir exists or create it
if err := os.MkdirAll(outputDir, 0755); err != nil {
return fmt.Errorf("failed to create output directory: %v", err)

Check warning on line 84 in exporter/fileexporter/exporter.go

View check run for this annotation

Codecov / codecov/patch

exporter/fileexporter/exporter.go#L84

Added line #L84 was not covered by tests
}
filePath = filepath.Join(outputDir, filename)
}

if f.Format == "parquet" {
return f.writeParquet(filePath, featureEvents)
Expand Down
114 changes: 107 additions & 7 deletions exporter/fileexporter/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ package fileexporter_test
import (
"context"
"os"
"path/filepath"
"runtime"
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/thomaspoignant/go-feature-flag/exporter"
"github.com/thomaspoignant/go-feature-flag/exporter/fileexporter"
"github.com/thomaspoignant/go-feature-flag/utils/fflog"
Expand All @@ -16,6 +19,13 @@ import (
)

func TestFile_Export(t *testing.T) {
// Create a temporary directory for test file operations
tempDir, err := os.MkdirTemp("", "fileexporter-test")
if err != nil {
t.Fatalf("Failed to create temp dir: %v", err)
}
defer os.RemoveAll(tempDir) // Clean up after tests

hostname, _ := os.Hostname()
type fields struct {
Format string
Expand All @@ -39,6 +49,8 @@ func TestFile_Export(t *testing.T) {
args args
wantErr bool
expected expected
setup func(t *testing.T, dir string)
teardown func(t *testing.T, dir string)
}{
{
name: "all default json",
Expand Down Expand Up @@ -233,10 +245,10 @@ func TestFile_Export(t *testing.T) {
},
},
{
name: "invalid outputdir",
wantErr: true,
name: "non-existent outputdir",
wantErr: false,
fields: fields{
OutputDir: "/tmp/foo/bar/",
OutputDir: filepath.Join(tempDir, "non-existent-dir"),
},
args: args{
featureEvents: []exporter.FeatureEvent{
Expand All @@ -246,10 +258,14 @@ func TestFile_Export(t *testing.T) {
},
{
Kind: "feature", ContextKind: "anonymousUser", UserKey: "EFGH", CreationDate: 1617970701, Key: "random-key",
Variation: "Default", Value: "YO2", Default: false, Source: "SERVER",
Variation: "Default", Value: "YO2", Default: false, Version: "127", Source: "SERVER",
},
},
},
expected: expected{
fileNameRegex: "^flag-variation-" + hostname + "-[0-9]*\\.json$",
content: "./testdata/all_default.json",
},
},
{
name: "invalid filename template",
Expand Down Expand Up @@ -291,13 +307,54 @@ func TestFile_Export(t *testing.T) {
},
},
{
name: "invalid parquet outputdir",
name: "outputdir with invalid permissions",
wantErr: true,
fields: fields{
Format: "parquet",
OutputDir: "/tmp/foo/bar/",
OutputDir: filepath.Join(tempDir, "invalid-permissions-dir"),
},
args: args{
featureEvents: []exporter.FeatureEvent{
{
Kind: "feature", ContextKind: "anonymousUser", UserKey: "ABCD", CreationDate: 1617970547, Key: "random-key",
Variation: "Default", Value: "YO", Default: false, Source: "SERVER",
},
},
},
setup: func(t *testing.T, dir string) {
err := os.MkdirAll(dir, 0755)
assert.NoError(t, err)
err = os.Chmod(dir, 0000) // Remove all permissions
assert.NoError(t, err)
},
teardown: func(t *testing.T, dir string) {
err := os.Chmod(dir, 0755) // Restore permissions for cleanup
assert.NoError(t, err)
},
},
{
name: "outputdir with trailing slash",
wantErr: false,
fields: fields{
Format: "json",
OutputDir: filepath.Join(tempDir, "dir-with-trailing-slash") + "/",
},
args: args{
featureEvents: []exporter.FeatureEvent{
{
Kind: "feature", ContextKind: "anonymousUser", UserKey: "ABCD", CreationDate: 1617970547, Key: "random-key",
Variation: "Default", Value: "YO", Default: false, Source: "SERVER",
},
{
Kind: "feature", ContextKind: "anonymousUser", UserKey: "EFGH", CreationDate: 1617970701, Key: "random-key",
Variation: "Default", Value: "YO2", Default: false, Version: "127", Source: "SERVER",
},
},
},
expected: expected{
fileNameRegex: "^flag-variation-" + hostname + "-[0-9]*\\.json$",
content: "./testdata/all_default.json",
},
args: args{},
},
}
for _, tt := range tests {
Expand All @@ -308,6 +365,14 @@ func TestFile_Export(t *testing.T) {
defer os.Remove(outputDir)
}

if tt.setup != nil {
tt.setup(t, outputDir)
}

if tt.teardown != nil {
defer tt.teardown(t, outputDir)
}

f := &fileexporter.Exporter{
Format: tt.fields.Format,
OutputDir: outputDir,
Expand All @@ -321,6 +386,12 @@ func TestFile_Export(t *testing.T) {
return
}

assert.NoError(t, err)

// Check if the directory was created
_, err = os.Stat(outputDir)
assert.NoError(t, err, "Output directory should exist")

files, _ := os.ReadDir(outputDir)
assert.Equal(t, 1, len(files), "Directory %s should have only one file", outputDir)
assert.Regexp(t, tt.expected.fileNameRegex, files[0].Name(), "Invalid file name")
Expand Down Expand Up @@ -350,3 +421,32 @@ func TestFile_IsBulk(t *testing.T) {
exporter := fileexporter.Exporter{}
assert.True(t, exporter.IsBulk(), "DeprecatedExporter is a bulk exporter")
}

func TestExportWithoutOutputDir(t *testing.T) {
featureEvents := []exporter.FeatureEvent{{
Kind: "feature", ContextKind: "anonymousUser", UserKey: "ABCD", CreationDate: 1617970547, Key: "random-key",
Variation: "Default", Value: "YO", Default: false, Source: "SERVER",
}}

filePrefix := "test-flag-variation-EXAMPLE-"
e := fileexporter.Exporter{
Format: "json",
Filename: filePrefix + "{{ .Timestamp}}.{{ .Format}}",
}
err := e.Export(context.Background(), nil, featureEvents)
require.NoError(t, err)

// check that a file exist
files, err := os.ReadDir("./")
require.NoError(t, err)

countFileWithPrefix := 0
for _, file := range files {
if strings.HasPrefix(file.Name(), filePrefix) {
countFileWithPrefix++
err := os.Remove(file.Name())
require.NoError(t, err)
}
}
assert.True(t, countFileWithPrefix > 0, "At least one file should have been created")
}
2 changes: 1 addition & 1 deletion website/docs/go_module/data_collection/file.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ ffclient.Config{

| Field | Description |
|---|---|
|`OutputDir` | OutputDir is the location of the directory to store the exported files.<br/>It should finish with a `/`. |
|`OutputDir` | OutputDir is the location of the directory to store the exported files. |
|`Format` | _(Optional)_ Format is the output format you want in your exported file.<br/>Available format: **`JSON`**, **`CSV`**, **`Parquet`**.<br/>**Default: `JSON`** |
|`Filename` | _(Optional)_ Filename is the name of your output file.<br/>You can use a templated config to define the name of your exported files.<br/>Available replacements are `{{ .Hostname}}`, `{{ .Timestamp}}` and `{{ .Format}}`<br/>**Default: `flag-variation-{{ .Hostname}}-{{ .Timestamp}}.{{ .Format}}`**|
|`CsvTemplate` | _(Optional)_ CsvTemplate is used if your output format is CSV.<br/>This field will be ignored if you are using format other than CSV.<br/>You can decide which fields you want in your CSV line with a go-template syntax, please check [internal/exporter/feature_event.go](https://github.com/thomaspoignant/go-feature-flag/blob/main/internal/exporter/feature_event.go) to see the available fields.<br/>**Default:** `{{ .Kind}};{{ .ContextKind}};{{ .UserKey}};{{ .CreationDate}};{{ .Key}};{{ .Variation}};{{ .Value}};{{ .Default}}\n` |
Expand Down
Loading
Loading