Add signup, document create/update commands#1372
Conversation
This command allows users to create a new document in a specified project using the CLI. It accepts an optional --initial-root flag to define the document’s initial YSON content. If the flag is not provided, a default sample object is used that includes all major CRDT value types such as Counter, Text, Tree, BinData, and Date. The command uses admin.Client.CreateDocument and handles YSON parsing using yson.ParseObject. This improves scripting and local testing usability, especially when working with sled-hosted servers or during development without relying on the web console.
This command uses admin.Client.SignUp to create a user, and was added primarily to test the behavior of client.go, including client activation and document attachment, in isolation from the full CLI flow.
This command updates an existing document’s root using the Yorkie CLI. It allows users to specify YSON input with the `--root` flag, enabling structured changes to fields such as Counter, Text, or Tree nodes. The command uses admin.Client.UpdateDocument and yson.ParseObject to apply the changes.
|
|
|
""" WalkthroughTwo new CLI subcommands, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant YorkieAdminClient
User->>CLI: yorkie document create [project] [key] --initial-root '<YSON>'
CLI->>CLI: Parse args & flags
CLI->>YorkieAdminClient: CreateDocument(project, key, root)
YorkieAdminClient-->>CLI: Document created (ID, key)
CLI-->>User: Print document key and ID
sequenceDiagram
participant User
participant CLI
participant YorkieAdminClient
User->>CLI: yorkie document update [project] [key] --root '<YSON>'
CLI->>CLI: Parse args & flags
CLI->>YorkieAdminClient: UpdateDocument(project, key, root)
YorkieAdminClient-->>CLI: Document updated (ID, key)
CLI-->>User: Print confirmation
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (3)📓 Common learningscmd/yorkie/document/create.go (4)cmd/yorkie/document/update.go (5)🧬 Code Graph Analysis (1)cmd/yorkie/document/update.go (3)
🪛 ast-grep (0.38.1)cmd/yorkie/document/create.go[warning] 56-59: Found an insecure gRPC connection using 'grpc.WithInsecure()'. This creates a connection without encryption to a gRPC server. A malicious attacker could tamper with the gRPC message, which could compromise the machine. Instead, establish a secure connection with an SSL certificate using the 'grpc.WithTransportCredentials()' function. You can create a create credentials using a 'tls.Config{}' struct with 'credentials.NewTLS()'. The final fix looks like this: 'grpc.WithTransportCredentials(credentials.NewTLS())'. (grpc-client-insecure-connection-go) cmd/yorkie/document/update.go[warning] 57-61: Found an insecure gRPC connection using 'grpc.WithInsecure()'. This creates a connection without encryption to a gRPC server. A malicious attacker could tamper with the gRPC message, which could compromise the machine. Instead, establish a secure connection with an SSL certificate using the 'grpc.WithTransportCredentials()' function. You can create a create credentials using a 'tls.Config{}' struct with 'credentials.NewTLS()'. The final fix looks like this: 'grpc.WithTransportCredentials(credentials.NewTLS())'. (grpc-client-insecure-connection-go) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (5)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
cmd/yorkie/signup.go (3)
34-36: Consider adding password validation.The
readPassword()function is called without any validation of the password strength or format. Consider adding basic password requirements (minimum length, etc.) to improve security.
84-84: Handle the error from MarkFlagRequired.The return value from
cmd.MarkFlagRequired("username")is being ignored. While this is unlikely to fail, it's better to handle the error for completeness.- _ = cmd.MarkFlagRequired("username") + if err := cmd.MarkFlagRequired("username"); err != nil { + panic(fmt.Sprintf("failed to mark username flag as required: %v", err)) + }
1-1: Fix code formatting.The static analysis tool indicates the file is not properly formatted. Please run
gofmton this file to ensure consistent formatting.gofmt -w cmd/yorkie/signup.gocmd/yorkie/document/create.go (1)
1-1: Fix code formatting.The static analysis tool indicates the file is not properly formatted. Please run
gofmton this file to ensure consistent formatting.gofmt -w cmd/yorkie/document/create.gocmd/yorkie/document/update.go (2)
87-87: Handle the error from MarkFlagRequired.The return value from
cmd.MarkFlagRequired("root")is being ignored. While this is unlikely to fail, it's better to handle the error for completeness.- _ = cmd.MarkFlagRequired("root") + if err := cmd.MarkFlagRequired("root"); err != nil { + panic(fmt.Sprintf("failed to mark root flag as required: %v", err)) + }
1-1: Fix code formatting.The static analysis tool indicates the file is not properly formatted. Please run
gofmton this file to ensure consistent formatting.gofmt -w cmd/yorkie/document/update.go
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/yorkie/document/create.go(1 hunks)cmd/yorkie/document/update.go(1 hunks)cmd/yorkie/signup.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
cmd/yorkie/signup.go (3)
cmd/yorkie/config/config.go (1)
Preload(155-176)admin/client.go (1)
WithInsecure(54-56)server/users/users.go (1)
SignUp(30-47)
cmd/yorkie/document/update.go (3)
cmd/yorkie/config/config.go (1)
LoadAuth(74-86)admin/client.go (1)
WithInsecure(54-56)server/documents/documents.go (1)
UpdateDocument(379-469)
🪛 ast-grep (0.38.1)
cmd/yorkie/signup.go
[warning] 37-37: Found an insecure gRPC connection using 'grpc.WithInsecure()'. This creates a connection without encryption to a gRPC server. A malicious attacker could tamper with the gRPC message, which could compromise the machine. Instead, establish a secure connection with an SSL certificate using the 'grpc.WithTransportCredentials()' function. You can create a create credentials using a 'tls.Config{}' struct with 'credentials.NewTLS()'. The final fix looks like this: 'grpc.WithTransportCredentials(credentials.NewTLS())'.
Context: admin.Dial(rpcAddr, admin.WithInsecure(insecure))
Note: [CWE-300] Channel Accessible by Non-Endpoint. [REFERENCES]
- https://blog.gopheracademy.com/advent-2019/go-grps-and-tls/#connection-without-encryption
(grpc-client-insecure-connection-go)
cmd/yorkie/document/update.go
[warning] 56-60: Found an insecure gRPC connection using 'grpc.WithInsecure()'. This creates a connection without encryption to a gRPC server. A malicious attacker could tamper with the gRPC message, which could compromise the machine. Instead, establish a secure connection with an SSL certificate using the 'grpc.WithTransportCredentials()' function. You can create a create credentials using a 'tls.Config{}' struct with 'credentials.NewTLS()'. The final fix looks like this: 'grpc.WithTransportCredentials(credentials.NewTLS())'.
Context: admin.Dial(
rpcAddr,
admin.WithToken(auth.Token),
admin.WithInsecure(auth.Insecure),
)
Note: [CWE-300] Channel Accessible by Non-Endpoint. [REFERENCES]
- https://blog.gopheracademy.com/advent-2019/go-grps-and-tls/#connection-without-encryption
(grpc-client-insecure-connection-go)
cmd/yorkie/document/create.go
[warning] 69-72: Found an insecure gRPC connection using 'grpc.WithInsecure()'. This creates a connection without encryption to a gRPC server. A malicious attacker could tamper with the gRPC message, which could compromise the machine. Instead, establish a secure connection with an SSL certificate using the 'grpc.WithTransportCredentials()' function. You can create a create credentials using a 'tls.Config{}' struct with 'credentials.NewTLS()'. The final fix looks like this: 'grpc.WithTransportCredentials(credentials.NewTLS())'.
Context: admin.Dial(rpcAddr,
admin.WithToken(auth.Token),
admin.WithInsecure(auth.Insecure),
)
Note: [CWE-300] Channel Accessible by Non-Endpoint. [REFERENCES]
- https://blog.gopheracademy.com/advent-2019/go-grps-and-tls/#connection-without-encryption
(grpc-client-insecure-connection-go)
🪛 golangci-lint (1.64.8)
cmd/yorkie/signup.go
1-1: File is not properly formatted
(gofmt)
cmd/yorkie/document/update.go
1-1: File is not properly formatted
(gofmt)
cmd/yorkie/document/create.go
1-1: File is not properly formatted
(gofmt)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
cmd/yorkie/signup.go (1)
28-54: Well-structured CLI command with good error handling.The signup command implementation follows good practices with proper error handling and resource cleanup. The use of
defer cli.Close()ensures the connection is properly closed.cmd/yorkie/document/create.go (2)
49-98: Well-structured document creation command.The command follows good practices with proper argument validation, authentication loading, and resource cleanup. The use of a default YSON example is helpful for users.
33-47: Comprehensive default YSON example.The default YSON example covers all major data types and is well-structured. This will be helpful for users learning the YSON format.
cmd/yorkie/document/update.go (2)
33-77: Well-structured document update command with good validation.The command properly validates required arguments and flags, loads authentication, and handles errors appropriately. The use of
key.Key()conversion is correct for the document key parameter.
37-45: Excellent argument validation logic.The validation function properly checks for both the required positional arguments and the required
--rootflag. This provides clear error messages to users.
Resolved formatting and style issues reported by `golangci-lint`, including gofmt violations and minor inconsistencies in comments, imports, and code structure.
There was a problem hiding this comment.
Pull Request Overview
Adds three new CLI commands to the Yorkie tooling for user signup and document management via the admin API.
- Implements a
signupcommand to register a new user. - Introduces
document createallowing an optional YSON initial root or a default example. - Adds
document updateto replace a document’s root using a required YSON string.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cmd/yorkie/signup.go | New CLI command for user signup |
| cmd/yorkie/document/create.go | New create command with optional default YSON |
| cmd/yorkie/document/update.go | New update command requiring a --root flag |
Comments suppressed due to low confidence (2)
cmd/yorkie/document/create.go:34
- This new
createcommand lacks unit tests for flag parsing and error conditions (e.g., invalid YSON input); consider adding tests to validate its behavior.
func newCreateDocumentCmd() *cobra.Command {
cmd/yorkie/document/update.go:32
- This new
updatecommand also lacks unit tests for flag parsing, argument validation, and error conditions; consider adding tests to ensure its robustness.
RunE: func(cmd *cobra.Command, args []string) error {
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
cmd/yorkie/document/create.go (1)
71-71: Handle YSON parse errors by usingUnmarshalinstead ofParseObjectThe current call to
yson.ParseObjectwill panic on invalid input. Since there is noParseObjectWithErrorhelper in theysonpackage, switch toyson.Unmarshal, which returns an error you can handle gracefully.- obj := yson.ParseObject(rootStr) + var obj yson.Object + if err := yson.Unmarshal(rootStr, &obj); err != nil { + return fmt.Errorf("failed to parse YSON: %w", err) + }cmd/yorkie/document/update.go (1)
53-53: Clarify the empty schemaKey argument in UpdateDocument callThe final
""you're passing tocli.UpdateDocumentis theschemaKeyparameter. An empty string here tells the server to reuse the document's existing schema rather than updating it. Please add an inline comment or replace""with a well-named constant to make this intent explicit.For example:
- doc, err := cli.UpdateDocument(ctx, projectName, key.Key(documentKey), updateRoot, "") + doc, err := cli.UpdateDocument(ctx, projectName, key.Key(documentKey), updateRoot, "" /* keep existing schema */)
🧹 Nitpick comments (3)
cmd/yorkie/signup.go (1)
1-1: Fix code formatting issues.The file has formatting issues according to static analysis. Please run
gofmtto ensure consistent formatting.gofmt -w cmd/yorkie/signup.gocmd/yorkie/document/create.go (1)
1-1: Fix code formatting issues.The file has formatting issues according to static analysis. Please run
gofmtto ensure consistent formatting.gofmt -w cmd/yorkie/document/create.gocmd/yorkie/document/update.go (1)
1-1: Fix code formatting issues.The file has formatting issues according to static analysis. Please run
gofmtto ensure consistent formatting.gofmt -w cmd/yorkie/document/update.go
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/yorkie/document/create.go(1 hunks)cmd/yorkie/document/update.go(1 hunks)cmd/yorkie/signup.go(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
cmd/yorkie/document/create.go (2)
Learnt from: chacha912
PR: yorkie-team/yorkie#1164
File: test/helper/helper.go:120-125
Timestamp: 2025-02-21T04:54:29.326Z
Learning: In `pkg/document/change/id.go`, the `InitialID` variable represents the initial state where nothing has been edited. Consider using a function like `GetInitialID()` instead of the package-level variable to enforce proper initialization and prevent shared mutable state.
Learnt from: window9u
PR: yorkie-team/yorkie#1156
File: server/backend/config.go:235-266
Timestamp: 2025-02-20T00:58:21.388Z
Learning: In Yorkie server, configuration parsing methods (e.g., `ParseEventWebhookMaxWaitInterval`) use `os.Exit(1)` for error handling as they are executed during server initialization where early termination on invalid configuration is desired.
cmd/yorkie/document/update.go (2)
Learnt from: chacha912
PR: yorkie-team/yorkie#1164
File: test/helper/helper.go:120-125
Timestamp: 2025-02-21T04:54:29.326Z
Learning: In `pkg/document/change/id.go`, the `InitialID` variable represents the initial state where nothing has been edited. Consider using a function like `GetInitialID()` instead of the package-level variable to enforce proper initialization and prevent shared mutable state.
Learnt from: chacha912
PR: yorkie-team/yorkie#1164
File: pkg/document/time/version_vector.go:158-183
Timestamp: 2025-02-21T04:54:17.948Z
Learning: In the Yorkie project, the `VersionVector.Min` and `VersionVector.Max` methods should maintain consistent behavior and documentation. Both methods should modify the receiver in-place for memory efficiency, take pointer parameters, and document their side effects clearly.
cmd/yorkie/signup.go (1)
Learnt from: window9u
PR: yorkie-team/yorkie#1156
File: server/backend/config.go:235-266
Timestamp: 2025-02-20T00:58:21.388Z
Learning: In Yorkie server, configuration parsing methods (e.g., `ParseEventWebhookMaxWaitInterval`) use `os.Exit(1)` for error handling as they are executed during server initialization where early termination on invalid configuration is desired.
🧬 Code Graph Analysis (1)
cmd/yorkie/signup.go (3)
cmd/yorkie/config/config.go (1)
Preload(155-176)admin/client.go (1)
WithInsecure(54-56)server/users/users.go (1)
SignUp(30-47)
🪛 ast-grep (0.38.1)
cmd/yorkie/document/create.go
[warning] 54-57: Found an insecure gRPC connection using 'grpc.WithInsecure()'. This creates a connection without encryption to a gRPC server. A malicious attacker could tamper with the gRPC message, which could compromise the machine. Instead, establish a secure connection with an SSL certificate using the 'grpc.WithTransportCredentials()' function. You can create a create credentials using a 'tls.Config{}' struct with 'credentials.NewTLS()'. The final fix looks like this: 'grpc.WithTransportCredentials(credentials.NewTLS())'.
Context: admin.Dial(rpcAddr,
admin.WithToken(auth.Token),
admin.WithInsecure(auth.Insecure),
)
Note: [CWE-300] Channel Accessible by Non-Endpoint. [REFERENCES]
- https://blog.gopheracademy.com/advent-2019/go-grps-and-tls/#connection-without-encryption
(grpc-client-insecure-connection-go)
cmd/yorkie/document/update.go
[warning] 41-45: Found an insecure gRPC connection using 'grpc.WithInsecure()'. This creates a connection without encryption to a gRPC server. A malicious attacker could tamper with the gRPC message, which could compromise the machine. Instead, establish a secure connection with an SSL certificate using the 'grpc.WithTransportCredentials()' function. You can create a create credentials using a 'tls.Config{}' struct with 'credentials.NewTLS()'. The final fix looks like this: 'grpc.WithTransportCredentials(credentials.NewTLS())'.
Context: admin.Dial(
rpcAddr,
admin.WithToken(auth.Token),
admin.WithInsecure(auth.Insecure),
)
Note: [CWE-300] Channel Accessible by Non-Endpoint. [REFERENCES]
- https://blog.gopheracademy.com/advent-2019/go-grps-and-tls/#connection-without-encryption
(grpc-client-insecure-connection-go)
cmd/yorkie/signup.go
[warning] 22-22: Found an insecure gRPC connection using 'grpc.WithInsecure()'. This creates a connection without encryption to a gRPC server. A malicious attacker could tamper with the gRPC message, which could compromise the machine. Instead, establish a secure connection with an SSL certificate using the 'grpc.WithTransportCredentials()' function. You can create a create credentials using a 'tls.Config{}' struct with 'credentials.NewTLS()'. The final fix looks like this: 'grpc.WithTransportCredentials(credentials.NewTLS())'.
Context: admin.Dial(rpcAddr, admin.WithInsecure(insecure))
Note: [CWE-300] Channel Accessible by Non-Endpoint. [REFERENCES]
- https://blog.gopheracademy.com/advent-2019/go-grps-and-tls/#connection-without-encryption
(grpc-client-insecure-connection-go)
🪛 golangci-lint (1.64.8)
cmd/yorkie/document/create.go
1-1: File is not properly formatted
(gofmt)
cmd/yorkie/document/update.go
1-1: File is not properly formatted
(gofmt)
cmd/yorkie/signup.go
1-1: File is not properly formatted
(gofmt)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
cmd/yorkie/signup.go (1)
19-19: readPassword function verifiedI confirmed that
readPassword()is implemented incmd/yorkie/login.go(lines 88–99), so its invocation incmd/yorkie/signup.gois valid. No further action needed here.cmd/yorkie/document/create.go (1)
93-93: No action needed:SubCmdis correctly definedThe
SubCmdvariable is declared incmd/yorkie/document/document.go(line 24), so it’s available throughout the package:• cmd/yorkie/document/document.go:24 —
SubCmd = &cobra.Command{…}You can safely leave the
SubCmd.AddCommand(cmd)call increate.go.Likely an incorrect or invalid review comment.
cmd/yorkie/document/update.go (1)
73-73: No action needed –SubCmdis defined. Incmd/yorkie/document/document.go(around line 24),SubCmd = &cobra.Command{…}is declared, so the call inupdate.gois valid.Likely an incorrect or invalid review comment.
Resolved formatting and style issues reported by `golangci-lint`, including gofmt violations and minor inconsistencies in comments, imports, and code structure.
Removed unnecessary newline at line 18. Also updated `document create` to handle possible errors from YSON parsing, ensuring that parse failures do not cause unexpected panics.
Added Apache 2.0 license headers to relevant source files to clarify licensing terms and ensure consistency across the codebase.
This reverts commit 306720a.
hackerwins
left a comment
hackerwins
left a comment
There was a problem hiding this comment.
Thanks for your contribution.
I left a few comments.
hackerwins
changed the title
signup, document create, and document update to resolve issue #1367
What this PR does / why we need it:
Adds three new CLI commands:
signup,document create, anddocument update.signup: Provides a minimal CLI interface to calladmin.Client.SignUp.document create: Adds the ability to create a new document usingadmin.Client.CreateDocument, with optional--initial-rootYSON content.document update: Allows modifying the root of an existing document via the--rootflag using parsed YSON. Usesadmin.Client.UpdateDocument.Which issue(s) this PR fixes:
Fixes #1367
Special notes for your reviewer:
I'm unsure about the current behavior of the document update command.
When using admin.Client.UpdateDocument, it seems that the existing root content remains intact, and the --update-root data is merged on top of it.
However, I originally understood this command to replace the entire document root with the new YSON content provided via --update-root.
Could you confirm whether this behavior is expected? If not, should the update logic explicitly clear the root before applying the new content?
Also, regarding the
document createcommand: if--initial-rootis not provided by the user, the code uses the default example document defined in the Yorkie documentation here.The
signupcommand was added primarily for testing purposes. It provides a minimal CLI interface to calladmin.Client.SignUp, which helps verifyclient.gobehavior. Thesignup.gocurrently reuses global variables (username,password,rpcAddr,insecure) and thereadPassword()function fromlogin.go. This reuse was intentional to avoid code duplication.Does this PR introduce a user-facing change?:
Additional documentation:
Checklist:
Summary by CodeRabbit