Skip to content

chore: optimize make:command for auto-generated Signature and simplified method receivers #821

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.

Sign up for GitHub

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

Merged
almas-x merged 2 commits into from
Jan 17, 2025
Merged

chore: optimize make:command for auto-generated Signature and simplified method receivers #821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84f4bd7
chore: optimize make:command for auto-generated Signature and simplif…
almas-x Jan 16, 2025
3b855ed
add custom command default category
almas-x Jan 16, 2025
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
24 changes: 12 additions & 12 deletions auth/console/jwt_secret_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,27 @@ func NewJwtSecretCommand(config config.Config) *JwtSecretCommand {
}

// Signature The name and signature of the console command.
func (receiver *JwtSecretCommand) Signature() string {
func (r *JwtSecretCommand) Signature() string {
return "jwt:secret"
}

// Description The console command description.
func (receiver *JwtSecretCommand) Description() string {
func (r *JwtSecretCommand) Description() string {
return "Set the JWTAuth secret key used to sign the tokens"
}

// Extend The console command extend.
func (receiver *JwtSecretCommand) Extend() command.Extend {
func (r *JwtSecretCommand) Extend() command.Extend {
return command.Extend{
Category: "jwt",
}
}

// Handle Execute the console command.
func (receiver *JwtSecretCommand) Handle(ctx console.Context) error {
key := receiver.generateRandomKey()
func (r *JwtSecretCommand) Handle(ctx console.Context) error {
key := r.generateRandomKey()

if err := receiver.setSecretInEnvironmentFile(key); err != nil {
if err := r.setSecretInEnvironmentFile(key); err != nil {
ctx.Error(err.Error())

return nil
Expand All @@ -53,19 +53,19 @@ func (receiver *JwtSecretCommand) Handle(ctx console.Context) error {
}

// generateRandomKey Generate a random key for the application.
func (receiver *JwtSecretCommand) generateRandomKey() string {
func (r *JwtSecretCommand) generateRandomKey() string {
return str.Random(32)
}
Comment on lines +56 to 58

@coderabbitai coderabbitai Bot Jan 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using crypto/rand for JWT secret generation.

The current implementation uses str.Random which might not provide cryptographically secure randomness needed for JWT secrets.

-func (r *JwtSecretCommand) generateRandomKey() string {
-       return str.Random(32)
+func (r *JwtSecretCommand) generateRandomKey() string {
+       // Use crypto/rand for cryptographic security
+       bytes := make([]byte, 32)
+       if _, err := rand.Read(bytes); err != nil {
+               return str.Random(32) // Fallback to existing implementation
+       }
+       return hex.EncodeToString(bytes)
}

Committable suggestion skipped: line range outside the PR's diff.


// setSecretInEnvironmentFile Set the application key in the environment file.
func (receiver *JwtSecretCommand) setSecretInEnvironmentFile(key string) error {
currentKey := receiver.config.GetString("jwt.secret")
func (r *JwtSecretCommand) setSecretInEnvironmentFile(key string) error {
currentKey := r.config.GetString("jwt.secret")

if currentKey != "" {
return errors.New("exist jwt secret")
}

err := receiver.writeNewEnvironmentFileWith(key)
err := r.writeNewEnvironmentFileWith(key)

if err != nil {
return err
Expand All @@ -75,13 +75,13 @@ func (receiver *JwtSecretCommand) setSecretInEnvironmentFile(key string) error {
}

// writeNewEnvironmentFileWith Write a new environment file with the given key.
func (receiver *JwtSecretCommand) writeNewEnvironmentFileWith(key string) error {
func (r *JwtSecretCommand) writeNewEnvironmentFileWith(key string) error {
content, err := os.ReadFile(support.EnvPath)
if err != nil {
return err
}

newContent := strings.Replace(string(content), "JWT_SECRET="+receiver.config.GetString("jwt.secret"), "JWT_SECRET="+key, 1)
newContent := strings.Replace(string(content), "JWT_SECRET="+r.config.GetString("jwt.secret"), "JWT_SECRET="+key, 1)

err = os.WriteFile(support.EnvPath, []byte(newContent), 0644)
if err != nil {
Expand Down
14 changes: 7 additions & 7 deletions auth/console/policy_make_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,17 @@ func NewPolicyMakeCommand() *PolicyMakeCommand {
}

// Signature The name and signature of the console command.
func (receiver *PolicyMakeCommand) Signature() string {
func (r *PolicyMakeCommand) Signature() string {
return "make:policy"
}

// Description The console command description.
func (receiver *PolicyMakeCommand) Description() string {
func (r *PolicyMakeCommand) Description() string {
return "Create a new policy class"
}

// Extend The console command extend.
func (receiver *PolicyMakeCommand) Extend() command.Extend {
func (r *PolicyMakeCommand) Extend() command.Extend {
return command.Extend{
Category: "make",
Flags: []command.Flag{
Expand All @@ -42,14 +42,14 @@ func (receiver *PolicyMakeCommand) Extend() command.Extend {
}

// Handle Execute the console command.
func (receiver *PolicyMakeCommand) Handle(ctx console.Context) error {
func (r *PolicyMakeCommand) Handle(ctx console.Context) error {
m, err := supportconsole.NewMake(ctx, "policy", ctx.Argument(0), filepath.Join("app", "policies"))
if err != nil {
ctx.Error(err.Error())
return nil
}

if err := file.Create(m.GetFilePath(), receiver.populateStub(receiver.getStub(), m.GetPackageName(), m.GetStructName())); err != nil {
if err := file.Create(m.GetFilePath(), r.populateStub(r.getStub(), m.GetPackageName(), m.GetStructName())); err != nil {
return err
}

Expand All @@ -58,12 +58,12 @@ func (receiver *PolicyMakeCommand) Handle(ctx console.Context) error {
return nil
}

func (receiver *PolicyMakeCommand) getStub() string {
func (r *PolicyMakeCommand) getStub() string {
return PolicyStubs{}.Policy()
}

// populateStub Populate the place-holders in the command stub.
func (receiver *PolicyMakeCommand) populateStub(stub string, packageName, structName string) string {
func (r *PolicyMakeCommand) populateStub(stub string, packageName, structName string) string {
stub = strings.ReplaceAll(stub, "DummyPolicy", structName)
stub = strings.ReplaceAll(stub, "DummyPackage", packageName)

Expand Down
10 changes: 5 additions & 5 deletions cache/console/clear_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,25 @@ func NewClearCommand(cache cache.Cache) *ClearCommand {
}

// Signature The name and signature of the console command.
func (receiver *ClearCommand) Signature() string {
func (r *ClearCommand) Signature() string {
return "cache:clear"
}

// Description The console command description.
func (receiver *ClearCommand) Description() string {
func (r *ClearCommand) Description() string {
return "Flush the application cache"
}

// Extend The console command extend.
func (receiver *ClearCommand) Extend() command.Extend {
func (r *ClearCommand) Extend() command.Extend {
return command.Extend{
Category: "cache",
}
}

// Handle Execute the console command.
func (receiver *ClearCommand) Handle(ctx console.Context) error {
if receiver.cache.Flush() {
func (r *ClearCommand) Handle(ctx console.Context) error {
if r.cache.Flush() {
ctx.Success("Application cache cleared")
} else {
ctx.Error("Clear Application cache Failed")
Expand Down
14 changes: 7 additions & 7 deletions console/console/build_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,17 @@ func NewBuildCommand(config config.Config) *BuildCommand {
}

// Signature The name and signature of the console command.
func (receiver *BuildCommand) Signature() string {
func (r *BuildCommand) Signature() string {
return "build"
}

// Description The console command description.
func (receiver *BuildCommand) Description() string {
func (r *BuildCommand) Description() string {
return "Build the application"
}

// Extend The console command extend.
func (receiver *BuildCommand) Extend() command.Extend {
func (r *BuildCommand) Extend() command.Extend {
return command.Extend{
Category: "build",
Flags: []command.Flag{
Expand All @@ -59,9 +59,9 @@ func (receiver *BuildCommand) Extend() command.Extend {
}

// Handle Execute the console command.
func (receiver *BuildCommand) Handle(ctx console.Context) error {
func (r *BuildCommand) Handle(ctx console.Context) error {
var err error
if receiver.config.GetString("app.env") == "production" {
if r.config.GetString("app.env") == "production" {
ctx.Warning("**************************************")
ctx.Warning("* Application In Production! *")
ctx.Warning("**************************************")
Expand Down Expand Up @@ -99,7 +99,7 @@ func (receiver *BuildCommand) Handle(ctx console.Context) error {

if err := ctx.Spinner("Building...", console.SpinnerOption{
Action: func() error {
return receiver.build(os, generateCommand(ctx.Option("name"), ctx.OptionBool("static")))
return r.build(os, generateCommand(ctx.Option("name"), ctx.OptionBool("static")))
},
}); err != nil {
ctx.Error(fmt.Sprintf("Build error: %v", err))
Expand All @@ -110,7 +110,7 @@ func (receiver *BuildCommand) Handle(ctx console.Context) error {
return nil
}

func (receiver *BuildCommand) build(system string, command []string) error {
func (r *BuildCommand) build(system string, command []string) error {
os.Setenv("CGO_ENABLED", "0")
os.Setenv("GOOS", system)
os.Setenv("GOARCH", "amd64")
Expand Down
20 changes: 10 additions & 10 deletions console/console/key_generate_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,25 @@ func NewKeyGenerateCommand(config config.Config) *KeyGenerateCommand {
}

// Signature The name and signature of the console command.
func (receiver *KeyGenerateCommand) Signature() string {
func (r *KeyGenerateCommand) Signature() string {
return "key:generate"
}

// Description The console command description.
func (receiver *KeyGenerateCommand) Description() string {
func (r *KeyGenerateCommand) Description() string {
return "Set the application key"
}

// Extend The console command extend.
func (receiver *KeyGenerateCommand) Extend() command.Extend {
func (r *KeyGenerateCommand) Extend() command.Extend {
return command.Extend{
Category: "key",
}
}

// Handle Execute the console command.
func (receiver *KeyGenerateCommand) Handle(ctx console.Context) error {
if receiver.config.GetString("app.env") == "production" {
func (r *KeyGenerateCommand) Handle(ctx console.Context) error {
if r.config.GetString("app.env") == "production" {
color.Warningln("**************************************")
color.Warningln("* Application In Production! *")
color.Warningln("**************************************")
Expand All @@ -58,8 +58,8 @@ func (receiver *KeyGenerateCommand) Handle(ctx console.Context) error {
}
}

key := receiver.generateRandomKey()
if err := receiver.writeNewEnvironmentFileWith(key); err != nil {
key := r.generateRandomKey()
if err := r.writeNewEnvironmentFileWith(key); err != nil {
ctx.Error(err.Error())

return nil
Expand All @@ -71,18 +71,18 @@ func (receiver *KeyGenerateCommand) Handle(ctx console.Context) error {
}

// generateRandomKey Generate a random key for the application.
func (receiver *KeyGenerateCommand) generateRandomKey() string {
func (r *KeyGenerateCommand) generateRandomKey() string {
return str.Random(32)
}
Comment on lines +74 to 76

@coderabbitai coderabbitai Bot Jan 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using crypto/rand for application key generation.

Similar to the JWT secret generation, the application key should use cryptographically secure random number generation.

-func (r *KeyGenerateCommand) generateRandomKey() string {
-       return str.Random(32)
+func (r *KeyGenerateCommand) generateRandomKey() string {
+       bytes := make([]byte, 32)
+       if _, err := rand.Read(bytes); err != nil {
+               return str.Random(32) // Fallback to existing implementation
+       }
+       return hex.EncodeToString(bytes)
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (r *KeyGenerateCommand) generateRandomKey() string {
return str.Random(32)
}
func (r *KeyGenerateCommand) generateRandomKey() string {
bytes := make([]byte, 32)
if _, err := rand.Read(bytes); err != nil {
return str.Random(32) // Fallback to existing implementation
}
return hex.EncodeToString(bytes)
}


// writeNewEnvironmentFileWith Write a new environment file with the given key.
func (receiver *KeyGenerateCommand) writeNewEnvironmentFileWith(key string) error {
func (r *KeyGenerateCommand) writeNewEnvironmentFileWith(key string) error {
content, err := os.ReadFile(support.EnvPath)
if err != nil {
return err
}

newContent := strings.Replace(string(content), "APP_KEY="+receiver.config.GetString("app.key"), "APP_KEY="+key, 1)
newContent := strings.Replace(string(content), "APP_KEY="+r.config.GetString("app.key"), "APP_KEY="+key, 1)

err = os.WriteFile(support.EnvPath, []byte(newContent), 0644)
if err != nil {
Expand Down
16 changes: 9 additions & 7 deletions console/console/make_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/goravel/framework/contracts/console/command"
supportconsole "github.com/goravel/framework/support/console"
"github.com/goravel/framework/support/file"
"github.com/goravel/framework/support/str"
)

type MakeCommand struct {
Expand All @@ -18,31 +19,31 @@ func NewMakeCommand() *MakeCommand {
}

// Signature The name and signature of the console command.
func (receiver *MakeCommand) Signature() string {
func (r *MakeCommand) Signature() string {

@coderabbitai coderabbitai Bot Jan 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Add test coverage for command interface methods

The following methods in MakeCommand lack test coverage:

  • Signature()
  • Description()
  • Extend()

While Handle() is well tested, these interface methods should also be covered to ensure correct command registration and help text display.

🔗 Analysis chain

Verify test coverage for the modified methods.

While the changes look good, static analysis indicates that some of the modified methods lack test coverage.

Run this script to analyze test coverage:

Also applies to: 27-27, 32-32, 39-39, 46-46, 55-55, 60-60, 63-63

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check test coverage for modified methods

# Search for existing tests
rg -l "TestMakeCommand" --type go

# Check if methods are covered in tests
rg "func.*MakeCommand.*Signature|func.*MakeCommand.*Description|func.*MakeCommand.*Extend" --type go

Length of output: 5171

Script:

#!/bin/bash
# Check test coverage in make_command_test.go
echo "=== Content of make_command_test.go ==="
cat console/console/make_command_test.go

echo -e "\n=== Searching for specific test cases ==="
rg "func Test.*MakeCommand.*(Signature|Description|Extend)" console/console/make_command_test.go

Length of output: 2329

Script:

#!/bin/bash
# Check other test files for coverage
echo "=== Content of foundation/console/test_make_command_test.go ==="
cat foundation/console/test_make_command_test.go

echo -e "\n=== Checking foundation/application.go for tests ==="
cat foundation/application.go | grep -A 10 "TestMakeCommand"

Length of output: 2491

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 22-22: console/console/make_command.go#L22
Added line #L22 was not covered by tests

return "make:command"
}

// Description The console command description.
func (receiver *MakeCommand) Description() string {
func (r *MakeCommand) Description() string {
return "Create a new Artisan command"
}

// Extend The console command extend.
func (receiver *MakeCommand) Extend() command.Extend {
func (r *MakeCommand) Extend() command.Extend {
return command.Extend{
Category: "make",
}
}

// Handle Execute the console command.
func (receiver *MakeCommand) Handle(ctx console.Context) error {
func (r *MakeCommand) Handle(ctx console.Context) error {
m, err := supportconsole.NewMake(ctx, "command", ctx.Argument(0), filepath.Join("app", "console", "commands"))
if err != nil {
ctx.Error(err.Error())
return nil
}

if err := file.Create(m.GetFilePath(), receiver.populateStub(receiver.getStub(), m.GetPackageName(), m.GetStructName())); err != nil {
if err := file.Create(m.GetFilePath(), r.populateStub(r.getStub(), m.GetPackageName(), m.GetStructName())); err != nil {
return err
}

Expand All @@ -51,14 +52,15 @@ func (receiver *MakeCommand) Handle(ctx console.Context) error {
return nil
}

func (receiver *MakeCommand) getStub() string {
func (r *MakeCommand) getStub() string {
return Stubs{}.Command()
}

// populateStub Populate the place-holders in the command stub.
func (receiver *MakeCommand) populateStub(stub string, packageName, structName string) string {
func (r *MakeCommand) populateStub(stub string, packageName, structName string) string {
stub = strings.ReplaceAll(stub, "DummyCommand", structName)
stub = strings.ReplaceAll(stub, "DummyPackage", packageName)
stub = strings.ReplaceAll(stub, "DummySignature", str.Of(structName).Kebab().Prepend("app:").String())

@hwbrzzl hwbrzzl Jan 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why add the app: prefix?

@almas-x almas-x Jan 16, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I noticed that Laravel adds an app: prefix for user-created commands, and I think it’s quite reasonable as it helps distinguish custom commands from framework or package-provided ones, making the command structure more organized.


return stub
}
2 changes: 2 additions & 0 deletions console/console/make_command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ func TestMakeCommand(t *testing.T) {
mockContext.EXPECT().Success("Console command created successfully").Once()
assert.Nil(t, makeCommand.Handle(mockContext))
assert.True(t, file.Exists("app/console/commands/clean_cache.go"))
assert.True(t, file.Contain("app/console/commands/clean_cache.go", "app:clean-cache"))

mockContext.EXPECT().Argument(0).Return("CleanCache").Once()
mockContext.EXPECT().OptionBool("force").Return(false).Once()
Expand All @@ -37,6 +38,7 @@ func TestMakeCommand(t *testing.T) {
assert.True(t, file.Exists("app/console/commands/Goravel/clean_cache.go"))
assert.True(t, file.Contain("app/console/commands/Goravel/clean_cache.go", "package Goravel"))
assert.True(t, file.Contain("app/console/commands/Goravel/clean_cache.go", "type CleanCache struct"))
assert.True(t, file.Contain("app/console/commands/Goravel/clean_cache.go", "app:clean-cache"))

assert.Nil(t, file.Remove("app"))
}
14 changes: 7 additions & 7 deletions console/console/stubs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package console
type Stubs struct {
}

func (receiver Stubs) Command() string {
func (r Stubs) Command() string {
return `package DummyPackage

import (
Expand All @@ -15,22 +15,22 @@ type DummyCommand struct {
}

// Signature The name and signature of the console command.
func (receiver *DummyCommand) Signature() string {
return "command:name"
func (r *DummyCommand) Signature() string {
return "DummySignature"
}

// Description The console command description.
func (receiver *DummyCommand) Description() string {
func (r *DummyCommand) Description() string {
return "Command description"
}

// Extend The console command extend.
func (receiver *DummyCommand) Extend() command.Extend {
return command.Extend{}
func (r *DummyCommand) Extend() command.Extend {
return command.Extend{Category: "app"}
}

// Handle Execute the console command.
func (receiver *DummyCommand) Handle(ctx console.Context) error {
func (r *DummyCommand) Handle(ctx console.Context) error {

return nil
}
Expand Down
Loading