refactor: log viewer

.codecov.yml

@@ -1,2 +1,7 @@
+coverage:
+  status:
+    project: off
+    patch: off
+
 github_checks:
   annotations: false

config/config.go

@@ -8,10 +8,9 @@ import (
 	"sigs.k8s.io/yaml"
 )
 
+// Config is the config for the using of reviewbot.
 type Config struct {
-	GlobalDefaultConfig GlobalConfig     `json:"globalDefaultConfig,omitempty"`
-	LogStorageConfig    LogStorageConfig `json:"logStorageConfig,omitempty"`
-	ServerAddr          string           `json:"serverAddr,omitempty"`
+	GlobalDefaultConfig GlobalConfig `json:"globalDefaultConfig,omitempty"`
 
 	// CustomConfig is the custom linter config.
 	// e.g.
@@ -39,12 +38,6 @@ type GlobalConfig struct {
 	JavaStyleCheckRuleConfig string `json:"javastylecheckruleConfig,omitempty"`
 }
 
-type LogStorageConfig struct {
-	CustomRemoteConfigs map[string]any `json:"customRemoteConfigs"`
-}
-
-type GithubConfig struct{}
-
 type Linter struct {
 	// Enable is whether to enable this linter, if false, linter still run but not report.
 	Enable *bool `json:"enable,omitempty"`

internal/linters/github.go

@@ -153,11 +153,17 @@ func CreateGithubChecks(ctx context.Context, a Agent, lintErrs map[string][]Lint
 		},
 		Output: &github.CheckRunOutput{
 			Title:       github.String(fmt.Sprintf("%s found %d issues related to your changes", linterName, len(annotations))),
-			Summary:     github.String(fmt.Sprintf("[full log ](%s)\n", a.LinterLogViewUrl) + Reference),
 			Annotations: annotations,
 		},
 	}
 
+	logURL := a.GenLogViewUrl()
+	if logURL == "" {
+		check.Output.Summary = github.String(Reference)
+	} else {
+		check.Output.Summary = github.String(fmt.Sprintf("[full log ](%s)\n", logURL) + Reference)
+	}
+
 	if len(annotations) > 0 {
 		check.Conclusion = github.String("failure")
 	} else {

internal/linters/go/gomodcheck/gomodcheck.go

@@ -26,8 +26,7 @@
 		log.Errorf("gomodchecks parse output failed: %v", err)
 		return err
 	}
 	return linters.Report(log, a, parsedOutput)
-
 }
 
 func goModCheckOutput(log *xlog.Logger, a linters.Agent) (map[string][]linters.LinterOutput, error) {

internal/linters/java/pmdcheck/pmdcheck.go

@@ -48,6 +48,7 @@ func pmdCheckHandler(ctx context.Context, a linters.Agent) error {
 	a.LinterConfig.Args = append(append(a.LinterConfig.Args, javaFiles...), "-R", checkrulePath)
 	return linters.GeneralHandler(plog, a, linters.ExecRun, pmdcheckParser)
 }
+
 func argsApply(log *xlog.Logger, a linters.Agent) linters.Agent {
 	config := a.LinterConfig
 	if len(config.Command) == 1 && config.Command[0] == linterName {

internal/linters/linters.go

@@ -100,10 +100,12 @@ type LinterOutput struct {
 
 // Agent knows necessary information in order to run linters.
 type Agent struct {
+	// ID each linter execution has a unique id.
+	ID string
 	// Context is the context of the agent.
 	Context context.Context
-	// LogStorages is the way to contral log storage
-	LogStorage storage.Storage
+	// Storage knows how to store and retrieve the linter logs.
+	Storage storage.Storage
 	// Runner is the way to run the linter.	like docker, local, etc.
 	Runner runner.Runner
 	// GitHubClient is the GitHub client.
@@ -120,12 +122,11 @@ type Agent struct {
 	LinterName string
 	// RepoDir is the repo directory.
 	RepoDir string
-	// LinterUuid is the uinque id of onece linter exec .
-	LinterUuid string
-	// LinterLogStoragePath is the path of log storage
-	LinterLogStoragePath string
-	// LinterLogStoragePath is the path of log storage
-	LinterLogViewUrl string
+
+	// GenLogKey generates the log key.
+	GenLogKey func() string
+	// GenLogViewUrl generates the log view url.
+	GenLogViewUrl func() string
 }
 
 const CommentFooter = `
@@ -181,12 +182,9 @@ func ExecRun(a Agent) ([]byte, error) {
 		return nil, fmt.Errorf("failed to read linter output: %w", err)
 	}
 
-	ctx := context.Background()
-	err = a.LogStorage.Writer(ctx, a.LinterLogStoragePath, output)
+	err = a.Storage.Write(a.Context, a.GenLogKey(), output)
 	if err != nil {
 		log.Errorf("write to storage was failed %v", err)
-	} else {
-		log.Info("write to storage was successful")
 	}
 
 	return output, nil

internal/linters/linters_test.go

@@ -20,11 +20,13 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"os"
 	"reflect"
 	"testing"
 
 	"github.com/qiniu/reviewbot/config"
 	"github.com/qiniu/reviewbot/internal/runner"
+	"github.com/qiniu/reviewbot/internal/storage"
 	"github.com/qiniu/x/xlog"
 )
 
@@ -227,6 +229,12 @@ func TestExecRun(t *testing.T) {
 	}
 	for _, tc := range tcs {
 		t.Run(tc.id, func(t *testing.T) {
+			storage, err := storage.NewLocalStorage(os.TempDir())
+			if err != nil {
+				t.Errorf("failed to create local storage: %v", err)
+			}
+			tc.input.Storage = storage
+			tc.input.GenLogKey = func() string { return "test" }
 			tc.input.Runner = runner.NewLocalRunner()
 			tc.input.LinterConfig.Modifier = config.NewBaseModifier()
 			tc.input.Context = context.WithValue(context.Background(), config.EventGUIDKey, "test")

internal/runner/runner.go

@@ -18,14 +18,17 @@ import (
 	"github.com/qiniu/x/xlog"
 )
 
+// Runner defines the interface for how to run the linter.
 type Runner interface {
+	// Prepare prepares the linter for running.
 	Prepare(ctx context.Context, cfg *config.Linter) error
+
+	// Run runs the linter and returns the output.
 	Run(ctx context.Context, cfg *config.Linter) (io.ReadCloser, error)
 }
 
 // LocalRunner is a runner that runs the linter locally.
-type LocalRunner struct {
-}
+type LocalRunner struct{}
 
 func NewLocalRunner() Runner {
 	return &LocalRunner{}

internal/storage/file.go

@@ -5,26 +5,33 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"sync"
 
 	"github.com/qiniu/x/log"
 )
 
 type LocalStorage struct {
-	mu sync.Mutex
+	rootDir string
 }
 
-func NewLocalStorage() Storage {
-	return &LocalStorage{}
+func NewLocalStorage(rootDir string) (Storage, error) {
+	rootDir, err := filepath.Abs(rootDir)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get absolute path: %w", err)
+	}
+	if err := os.MkdirAll(rootDir, 0o755); err != nil {
+		return nil, fmt.Errorf("failed to make log dir: %w", err)
+	}
+	return &LocalStorage{rootDir: rootDir}, nil
 }
 
-func (*LocalStorage) Writer(ctx context.Context, path string, content []byte) error {
-	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+func (l *LocalStorage) Write(ctx context.Context, path string, content []byte) error {
+	logFile := filepath.Join(l.rootDir, path, DefaultLogName)
+	log.Infof("writing log to %s", logFile)
+	if err := os.MkdirAll(filepath.Dir(logFile), 0o755); err != nil {
 		log.Errorf("failed to make log dir: %v", err)
 	}
-	dir, _ := os.Getwd()
-	log.Infof("pwd: %s", dir)
-	file, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644)
+
+	file, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644)
 	if err != nil {
 		return fmt.Errorf("failed to open file: %w", err)
 	}
@@ -42,14 +49,12 @@ func (*LocalStorage) Writer(ctx context.Context, path string, content []byte) er
 	return nil
 }
 
-func (g *LocalStorage) Reader(ctx context.Context, path string) ([]byte, error) {
-	filePath := path
+func (l *LocalStorage) Read(ctx context.Context, path string) ([]byte, error) {
+	filePath := filepath.Join(l.rootDir, path, DefaultLogName)
 	if _, err := os.Stat(filePath); os.IsNotExist(err) {
 		return nil, fmt.Errorf("file does not exist: %s", filePath)
 	}
 
-	g.mu.Lock()
-	defer g.mu.Unlock()
 	data, err := os.ReadFile(filePath)
 	if err != nil {
 		return nil, fmt.Errorf("error reading file: %w", err)

internal/storage/git.go

@@ -0,0 +1,19 @@
+package storage
+
+import (
+	"context"
+)
+
+type GitStorage struct{}
+
+func NewGitStorage() (Storage, error) {
+	return &GitStorage{}, nil
+}
+
+func (g *GitStorage) Write(ctx context.Context, key string, content []byte) error {
+	return nil
+}
+
+func (g *GitStorage) Read(ctx context.Context, key string) ([]byte, error) {
+	return nil, nil
+}

internal/storage/s3.go

@@ -0,0 +1,21 @@
+package storage
+
+import "context"
+
+type S3Storage struct {
+	credential string
+}
+
+func NewS3Storage(credential string) (Storage, error) {
+	return &S3Storage{
+		credential: credential,
+	}, nil
+}
+
+func (s *S3Storage) Write(ctx context.Context, key string, content []byte) error {
+	return nil
+}
+
+func (s *S3Storage) Read(ctx context.Context, key string) ([]byte, error) {
+	return nil, nil
+}

internal/storage/storage.go

@@ -3,7 +3,13 @@ package storage
 import "context"
 
 type Storage interface {
-	Writer(ctx context.Context, path string, content []byte) error
+	// Write writes the content to the specified key.
+	Write(ctx context.Context, key string, content []byte) error
 
-	Reader(ctx context.Context, path string) ([]byte, error)
+	// Read reads the content from the specified key.
+	Read(ctx context.Context, key string) ([]byte, error)
 }
+
+const (
+	DefaultLogName = "log.txt"
+)

internal/storage/storage_test.go

@@ -0,0 +1,65 @@
+package storage_test
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/qiniu/reviewbot/internal/storage"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFileStorage(t *testing.T) {
+	tempDir, err := os.MkdirTemp("", "file_storage_test")
+	require.NoError(t, err)
+	defer os.RemoveAll(tempDir)
+	fs, err := storage.NewLocalStorage(tempDir)
+	require.NoError(t, err)
+
+	t.Run("Write and Read", func(t *testing.T) {
+		ctx := context.Background()
+		content := []byte("测试内容")
+		path := "test.txt"
+		err := fs.Write(ctx, path, content)
+		require.NoError(t, err)
+		readContent, err := fs.Read(ctx, path)
+		require.NoError(t, err)
+		assert.Equal(t, content, readContent)
+	})
+
+	t.Run("Read Non-existent File", func(t *testing.T) {
+		ctx := context.Background()
+		_, err := fs.Read(ctx, "non_existent.txt")
+		require.Error(t, err)
+	})
+
+	t.Run("Write to Non-existent Directory", func(t *testing.T) {
+		ctx := context.Background()
+		content := []byte("测试内容")
+		path := filepath.Join("non", "existent", "dir", "test.txt")
+
+		err := fs.Write(ctx, path, content)
+		require.NoError(t, err)
+
+		readContent, err := fs.Read(ctx, path)
+		require.NoError(t, err)
+		require.Equal(t, content, readContent)
+	})
+
+	t.Run("Overwrite Existing File", func(t *testing.T) {
+		ctx := context.Background()
+		path := "overwrite.txt"
+		err := fs.Write(ctx, path, []byte("原始内容"))
+		require.NoError(t, err)
+
+		newContent := []byte("新内容")
+		err = fs.Write(ctx, path, newContent)
+		require.NoError(t, err)
+
+		readContent, err := fs.Read(ctx, path)
+		require.NoError(t, err)
+		require.Equal(t, newContent, readContent)
+	})
+}