feat: add JSONL tuple import support #530
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ## Walkthrough
Support for the JSON Lines (`.jsonl`) file format was added for reading and parsing tuple data in the application. Documentation, code, and tests were updated to handle `.jsonl` files for tuple write and delete commands, including examples and new test cases. New fixture and test data files in `.jsonl` format were also introduced.
## Changes
| File(s) | Change Summary |
|--------------------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------|
| README.md | Updated documentation to mention `.jsonl` support for tuple write/delete commands and added usage examples. |
| internal/tuplefile/read.go | Added JSONL parsing support with a new helper function; updated main reader to handle `.jsonl` files. |
| cmd/tuple/write_test.go, cmd/tuple/delete_test.go | Added test cases to verify correct parsing and error handling for `.jsonl` tuple files. |
| cmd/tuple/testdata/tuples.jsonl, tests/fixtures/basic-tuples.jsonl | Added new JSONL test data and fixture files with tuple entries for use in tests. |
| tests/tuple-test-cases.yaml | Added a new test case to verify tuple writing using a `.jsonl` file as input. |
| tests/fixtures/basic-model.fga | Expanded `owner` relation in `group` type to include a new `inOfficeIP` condition checking IP address within a CIDR range. |
| tests/fixtures/basic-tuples.json | Added a trailing newline character at the end of the file. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant CLI
participant TupleFileReader
participant Store
User->>CLI: Run tuple write/delete with --file file.jsonl
CLI->>TupleFileReader: ReadTupleFile(file.jsonl)
TupleFileReader->>TupleFileReader: parseTuplesFromJSONL(data)
TupleFileReader-->>CLI: Parsed tuples
CLI->>Store: Write/Delete tuples
Store-->>CLI: Operation result
CLI-->>User: Output result |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
README.md (1)
705-705: Fix markdown list style consistency.The static analysis tool flagged inconsistent list styles. Consider using consistent dash formatting for unordered lists.
-* `--file`: Specifies the file name, `json`, `jsonl`, `yaml` and `csv` files are supported +- `--file`: Specifies the file name, `json`, `jsonl`, `yaml` and `csv` files are supported-* `--file`: Specifies the file name, `yaml`, `json`, and `jsonl` files are supported +- `--file`: Specifies the file name, `yaml`, `json`, and `jsonl` files are supportedAlso applies to: 850-850
internal/tuplefile/read.go (1)
47-75: Well-implemented JSONL parser with good error handling.The implementation correctly handles the JSONL format (one JSON object per line) and provides informative error messages with line numbers for debugging.
However, there's a minor whitespace linting issue to address:
for scanner.Scan() { lineNum++ line := strings.TrimSpace(scanner.Text()) + if line == "" { continue }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(3 hunks)cmd/tuple/delete_test.go(2 hunks)cmd/tuple/testdata/tuples.jsonl(1 hunks)cmd/tuple/write_test.go(1 hunks)internal/tuplefile/read.go(3 hunks)tests/fixtures/basic-tuples.jsonl(1 hunks)tests/tuple-test-cases.yaml(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/tuplefile/read.go (1)
internal/clierrors/clierrors.go (1)
EmptyTuplesFileError(43-45)
🪛 GitHub Actions: Build, Test and Publish
tests/tuple-test-cases.yaml
[error] 1-1: Integration test '001b - it successfully writes tuples to a store using jsonl' failed: Query "successful.0.user" did not match a path.
🪛 markdownlint-cli2 (0.17.2)
README.md
705-705: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
850-850: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
🪛 GitHub Check: Lints
internal/tuplefile/read.go
[failure] 54-54:
only one cuddle assignment allowed before if statement (wsl)
[failure] 71-71:
error returned from external package is unwrapped: sig: func github.com/openfga/cli/internal/clierrors.EmptyTuplesFileError(extName string) error (wrapcheck)
🔇 Additional comments (11)
tests/fixtures/basic-tuples.jsonl (1)
1-1: LGTM! Well-formatted JSONL fixture.The fixture correctly follows JSONL format with a valid tuple containing the required fields (user, relation, object).
tests/tuple-test-cases.yaml (1)
11-16: Confirm JSONL tuple write output structureThe new JSONL test (
001bin tests/tuple-test-cases.yaml lines 11–16) is identical to the JSON test but is failing because the pathsuccessful.0.userisn’t found when using the JSONL fixture. We need to verify whether the CLI’s JSONL mode truly emits the same JSON structure.Please run the following locally and compare:
# Capture JSON output fga tuple write \ --file=./tests/fixtures/basic-tuples.json \ --max-tuples-per-write=1 \ --max-parallel-requests=1 \ --store-id=$(./tests/scripts/test-data/get-store-id.sh) \ --model-id=$(./tests/scripts/test-data/get-model-id.sh) \ > json_output.txt # Capture JSONL output fga tuple write \ --file=./tests/fixtures/basic-tuples.jsonl \ --max-tuples-per-write=1 \ --max-parallel-requests=1 \ --store-id=$(./tests/scripts/test-data/get-store-id.sh) \ --model-id=$(./tests/scripts/test-data/get-model-id.sh) \ > jsonl_output.txt # Compare structures diff -u json_output.txt jsonl_output.txt | lessVerify that both outputs contain the same wrapper and array under
successfulso the test’s JSONPath (successful.0.user) will match. If they differ, update either the CLI’s JSONL handling or the test expectations accordingly.cmd/tuple/testdata/tuples.jsonl (1)
1-3: Excellent test data coverage for JSONL format.The test data includes good variety: basic tuples and conditional tuples with context. The JSONL format is correctly implemented with one JSON object per line.
cmd/tuple/delete_test.go (2)
123-143: Well-implemented JSONL parsing test.The test correctly validates JSONL file parsing with expected tuple structure that matches the test data. Good consistency with existing test patterns.
203-207: Appropriate error handling for empty JSONL files.The error message format is consistent with other file format error handling and properly identifies the JSONL format in the error message.
README.md (3)
705-705: Documentation correctly updated for JSONL support.The file parameter description now properly includes
jsonlformat alongside the other supported formats.
754-761: Excellent JSONL format example.The example clearly demonstrates the JSONL format including both basic tuples and conditional tuples with context. This will help users understand the expected format.
850-850: Consistent documentation for delete command.The delete command documentation properly includes
jsonlformat support, maintaining consistency with the write command documentation.cmd/tuple/write_test.go (1)
151-175: LGTM! Comprehensive test coverage for JSONL parsing.The test case follows the established pattern and provides good coverage by testing tuples with and without conditions, ensuring the JSONL parser handles various scenarios correctly.
internal/tuplefile/read.go (2)
4-6: Good addition of required imports for JSONL parsing.The added imports (
bufio,bytes,encoding/json) are necessary for the line-by-line JSON parsing implementation.
32-33: LGTM! Consistent integration with existing file format handling.The new
.jsonlcase follows the same pattern as other supported formats.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
Testing
make test(fails: docker not found)https://chatgpt.com/codex/tasks/task_e_68652d7d38408322b8aac442bc24a5a9
Summary by CodeRabbit
New Features
Tests