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

feat: optimize artisan check #238

Merged
merged 10 commits into from
Jul 15, 2023
Merged
Show file tree
Hide file tree
Changes from 8 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
6 changes: 5 additions & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@ jobs:
- uses: actions/setup-go@v4
with:
go-version: 'stable'
- name: Lint 🎨
cache: false
- name: Lint
uses: golangci/golangci-lint-action@v3
with:
skip-cache: true
skip-pkg-cache: true
skip-build-cache: true
version: latest
args: --timeout=30m ./...
61 changes: 61 additions & 0 deletions auth/console/jwt_secret_command_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package console

import (
"os"
"testing"

"github.com/stretchr/testify/assert"

configmock "github.com/goravel/framework/contracts/config/mocks"
consolemocks "github.com/goravel/framework/contracts/console/mocks"
"github.com/goravel/framework/support"
"github.com/goravel/framework/support/file"
)

func TestJwtSecretCommand(t *testing.T) {
mockConfig := &configmock.Config{}
mockConfig.On("GetString", "jwt.secret").Return("").Twice()

jwtSecretCommand := NewJwtSecretCommand(mockConfig)
mockContext := &consolemocks.Context{}

assert.False(t, file.Exists(".env"))
err := file.Create(".env", "JWT_SECRET=\n")
assert.Nil(t, err)

assert.Nil(t, jwtSecretCommand.Handle(mockContext))

assert.True(t, file.Exists(".env"))
env, err := os.ReadFile(".env")
assert.Nil(t, err)
assert.True(t, len(env) > 10)
assert.Nil(t, file.Remove(".env"))

mockConfig.AssertExpectations(t)
}

func TestJwtSecretCommandWithCustomEnvFile(t *testing.T) {
support.EnvPath = "config.conf"

mockConfig := &configmock.Config{}
mockConfig.On("GetString", "jwt.secret").Return("").Twice()

jwtSecretCommand := NewJwtSecretCommand(mockConfig)
mockContext := &consolemocks.Context{}

assert.False(t, file.Exists("config.conf"))
err := file.Create("config.conf", "JWT_SECRET=\n")
assert.Nil(t, err)

assert.Nil(t, jwtSecretCommand.Handle(mockContext))

assert.True(t, file.Exists("config.conf"))
env, err := os.ReadFile("config.conf")
assert.Nil(t, err)
assert.True(t, len(env) > 10)
assert.Nil(t, file.Remove("config.conf"))

support.EnvPath = ".env"

mockConfig.AssertExpectations(t)
}
2 changes: 1 addition & 1 deletion auth/console/policy_make_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/stretchr/testify/assert"
)

func TestEventMakeCommand(t *testing.T) {
func TestPolicyMakeCommand(t *testing.T) {
policyMakeCommand := &PolicyMakeCommand{}
mockContext := &consolemocks.Context{}
mockContext.On("Argument", 0).Return("").Once()
Expand Down
8 changes: 4 additions & 4 deletions config/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ type Application struct {

func NewApplication(envPath string) *Application {
if !file.Exists(envPath) {
color.Redln("Please create .env and initialize it first.")
color.Warnln("Run command: \ncp .env.example .env && go run . artisan key:generate")
color.Redln("Please create " + envPath + " and initialize it first.")
color.Warnln("Example command: \ncp .env.example .env && go run . artisan key:generate")
os.Exit(0)
}

Expand All @@ -42,13 +42,13 @@ func NewApplication(envPath string) *Application {
if support.Env != support.EnvArtisan {
if appKey == nil {
color.Redln("Please initialize APP_KEY first.")
color.Warnln("Run command: \ngo run . artisan key:generate")
color.Warnln("Example command: \ngo run . artisan key:generate")
os.Exit(0)
}

if len(appKey.(string)) != 32 {
color.Redln("Invalid APP_KEY, please reset it.")
color.Warnln("Run command: \ngo run . artisan key:generate")
color.Warnln("Example command: \ngo run . artisan key:generate")
os.Exit(0)
}
}
Expand Down
41 changes: 24 additions & 17 deletions console/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,36 +42,43 @@ func (c *Application) Register(commands []console.Command) {
}
}

//Call Run an Artisan console command by name.
// Call Run an Artisan console command by name.
func (c *Application) Call(command string) {
c.Run(append([]string{os.Args[0], "artisan"}, strings.Split(command, " ")...), false)
}

//CallAndExit Run an Artisan console command by name and exit.
// CallAndExit Run an Artisan console command by name and exit.
func (c *Application) CallAndExit(command string) {
c.Run(append([]string{os.Args[0], "artisan"}, strings.Split(command, " ")...), true)
}

//Run a command. Args come from os.Args.
// Run a command. Args come from os.Args.
func (c *Application) Run(args []string, exitIfArtisan bool) {
if len(args) >= 2 {
if args[1] == "artisan" {
if len(args) == 2 {
args = append(args, "--help")
}
artisanIndex := -1
for i, arg := range args {
if arg == "artisan" {
artisanIndex = i
break
}
}

if args[2] != "-V" && args[2] != "--version" {
cliArgs := append([]string{args[0]}, args[2:]...)
if err := c.instance.Run(cliArgs); err != nil {
panic(err.Error())
}
if artisanIndex != -1 {
// Add --help if no command argument is provided.
if artisanIndex+1 == len(args) {
args = append(args, "--help")
}

if args[artisanIndex+1] != "-V" && args[artisanIndex+1] != "--version" {
cliArgs := append([]string{args[0]}, args[artisanIndex+1:]...)
if err := c.instance.Run(cliArgs); err != nil {
panic(err.Error())
}
}

printResult(args[2])
printResult(args[artisanIndex+1])

if exitIfArtisan {
os.Exit(0)
}
if exitIfArtisan {
os.Exit(0)
}
}
}
Expand Down
48 changes: 48 additions & 0 deletions console/console/key_generate_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

configmock "github.com/goravel/framework/contracts/config/mocks"
consolemocks "github.com/goravel/framework/contracts/console/mocks"
"github.com/goravel/framework/support"
"github.com/goravel/framework/support/file"
)

Expand Down Expand Up @@ -53,3 +54,50 @@ func TestKeyGenerateCommand(t *testing.T) {

mockConfig.AssertExpectations(t)
}

func TestKeyGenerateCommandWithCustomEnvFile(t *testing.T) {
support.EnvPath = "config.conf"

mockConfig := &configmock.Config{}
mockConfig.On("GetString", "app.env").Return("local").Twice()
mockConfig.On("GetString", "app.key").Return("12345").Once()

keyGenerateCommand := NewKeyGenerateCommand(mockConfig)
mockContext := &consolemocks.Context{}

assert.False(t, file.Exists("config.conf"))

assert.Nil(t, keyGenerateCommand.Handle(mockContext))

err := file.Create("config.conf", "APP_KEY=12345\n")
assert.Nil(t, err)

assert.Nil(t, keyGenerateCommand.Handle(mockContext))
assert.True(t, file.Exists("config.conf"))
env, err := os.ReadFile("config.conf")
assert.Nil(t, err)
assert.True(t, len(env) > 10)

mockConfig.On("GetString", "app.env").Return("production").Once()

reader, writer, err := os.Pipe()
assert.Nil(t, err)
originalStdin := os.Stdin
defer func() { os.Stdin = originalStdin }()
os.Stdin = reader
go func() {
defer writer.Close()
_, err = writer.Write([]byte("no\n"))
assert.Nil(t, err)
}()

assert.Nil(t, keyGenerateCommand.Handle(mockContext))
env, err = os.ReadFile("config.conf")
assert.Nil(t, err)
assert.True(t, len(env) > 10)
assert.Nil(t, file.Remove("config.conf"))

support.EnvPath = ".env"

mockConfig.AssertExpectations(t)
}

Choose a reason for hiding this comment

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

Bug risks:

  • The code modifies the global variable support.EnvPath without restoring its original value after the test. This can affect other tests that rely on the default value of support.EnvPath.

Improvement suggestions:

  • Instead of modifying the global variable support.EnvPath, it would be better to use a local variable within the test function to specify the custom environment file path.
  • The code uses assert statements for checking conditions and error handling. Consider using testing functions like Errorf or FailNow to provide more informative failure messages when assertions fail.
  • There is an opportunity to improve code readability by breaking down the second test function into smaller, more focused test cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Bug risks:

  • The code modifies the global variable support.EnvPath without restoring its original value after the test. This can affect other tests that rely on the default value of support.EnvPath.

Improvement suggestions:

  • Instead of modifying the global variable support.EnvPath, it would be better to use a local variable within the test function to specify the custom environment file path.
  • The code uses assert statements for checking conditions and error handling. Consider using testing functions like Errorf or FailNow to provide more informative failure messages when assertions fail.
  • There is an opportunity to improve code readability by breaking down the second test function into smaller, more focused test cases.

Good idea.

9 changes: 4 additions & 5 deletions crypt/aes.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import (
"encoding/base64"
"errors"
"io"
"os"

"github.com/bytedance/sonic"
"github.com/gookit/color"

"github.com/goravel/framework/contracts/config"
"github.com/goravel/framework/support"
)

type AES struct {
Expand All @@ -23,15 +23,14 @@ type AES struct {
func NewAES(config config.Config) *AES {
key := config.GetString("app.key")

// Don't use AES in artisan key:generate command
args := os.Args
if len(args) >= 3 && args[1] == "artisan" && args[2] == "key:generate" {
// Don't use AES in artisan when the key is empty.
if support.Env == support.EnvArtisan && len(key) == 0 {
return nil
}

// check key length before using it
if len(key) != 16 && len(key) != 24 && len(key) != 32 {
color.Redln("[Crypt] Empty or invalid APP_KEY, please reset it.\nRun command:\ngo run . artisan key:generate")
color.Redln("[Crypt] Empty or invalid APP_KEY, please reset it.\nExample command:\ngo run . artisan key:generate")
return nil
}
keyBytes := []byte(key)
Expand Down
7 changes: 5 additions & 2 deletions foundation/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,11 @@ func setEnv() {
support.Env = support.EnvTest
}
if len(args) >= 2 {
if args[1] == "artisan" {
support.Env = support.EnvArtisan
for _, arg := range args[1:] {
if arg == "artisan" {
support.Env = support.EnvArtisan
break
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion route/gin.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
httpcontract "github.com/goravel/framework/contracts/http"
"github.com/goravel/framework/contracts/route"
goravelhttp "github.com/goravel/framework/http"
"github.com/goravel/framework/support"
)

type Gin struct {
Expand Down Expand Up @@ -120,7 +121,7 @@ func (r *Gin) ServeHTTP(writer http.ResponseWriter, request *http.Request) {
}

func (r *Gin) outputRoutes() {
if r.config.GetBool("app.debug") && !runningInConsole() {
if r.config.GetBool("app.debug") && support.Env != support.EnvArtisan {
for _, item := range r.instance.Routes() {
fmt.Printf("%-10s %s\n", item.Method, colonToBracket(item.Path))
}
Expand Down
7 changes: 0 additions & 7 deletions route/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package route

import (
"fmt"
"os"
"regexp"
"strings"
"time"
Expand Down Expand Up @@ -99,9 +98,3 @@ func mergeSlashForPath(path string) string {

return strings.ReplaceAll(path, "//", "/")
}

func runningInConsole() bool {
args := os.Args

return len(args) >= 2 && args[1] == "artisan"
}
5 changes: 1 addition & 4 deletions support/file/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,7 @@ func Create(file string, content string) error {

func Exists(file string) bool {
_, err := os.Stat(file)
if err != nil {
return os.IsExist(err)
}
return true
return err == nil
}

// Extension Supported types: https://github.com/gabriel-vasile/mimetype/blob/master/supported_mimes.md
Expand Down