feat: support multiple tuple files #506

SoulPancake · 2025-06-05T06:03:40Z

I was giving this feature a shot,
I was able to make it work when you provide the source test files like such

name: Test Store
model_file: ./test.fga
#tuple_file: ./singletuple.yaml
tuple_files:
  - ./tuple.yaml
  - ./tuple_2.yaml
tests:
  - name:  Admin checks
    description: To test what   authorisation on all objects
    check:

Although this is still in draft mode so let me know if this is what we expect, I can finish the tests then

Description

References

closes #506
closes #233

Review Checklist

I have clicked on "allow edits by maintainers".
I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
The correct base branch is being used, if not main
I have added tests to validate that the change in functionality is working as expected

Summary by CodeRabbit

Documentation
- Clarified usage and added examples for single and multiple global tuple files in test configurations.
New Features
- Enabled support for multiple global tuple files to aggregate tuples from various sources during testing.
Bug Fixes
- Enhanced error handling and reporting for tuple loading from files.
Tests
- Added tests covering tuple loading from different file setups and error conditions.
Chores
- Added example tuple files illustrating hierarchical folder relationships.

internal/storetest/storedata.go

SoulPancake · 2025-06-09T05:53:48Z

@aaguiarz Let me know if you could review this as I want to use this feature as well for organising my tuple files

aaguiarz · 2025-06-09T22:10:19Z

@SoulPancake yeah, this makes sense.

Please make sure you add tests to make sure the new format and old format work, and update docs

SoulPancake · 2025-06-10T05:51:25Z

@aaguiarz
What should the expected behaviour be if the tuple file ( single ) and tuple files array both are provided?

SoulPancake · 2025-06-11T03:08:13Z

@aaguiarz What should the expected behaviour be if the tuple file ( single ) and tuple files array both are provided?

My proposal would be

If a tuple file alone is provided, just load those tuples.
If multiple tuple files are provided, load all of them
If both tuple file and tuple files array is provided, load from all the files but do a duplicate check before loading tuples?

…iles

internal/storetest/storedata.go

README.md

this is linked to the changes proposed and introduced in openfga/cli#506

aaguiarz · 2025-06-12T12:25:01Z

@aaguiarz What should the expected behaviour be if the tuple file ( single ) and tuple files array both are provided?

My proposal would be

If a tuple file alone is provided, just load those tuples.

If multiple tuple files are provided, load all of them

If both tuple file and tuple files array is provided, load from all the files but do a duplicate check before loading tuples?

This makes sense. I would not do a duplicate check before loading tuples for now, I think it's better to surface that issue to the user than to hide it.

Thanks a lot for your help with this.

SoulPancake · 2025-06-12T12:48:25Z

I think it's better to surface that issue to the user than to hide it.

So programmatically we still keep the map? and error out on duplicate tuples.

aaguiarz · 2025-06-12T12:49:33Z

I think it's better to surface that issue to the user than to hide it.

So programmatically we still keep the map? and error out on duplicate tuples.

Yes, the test will fail because they are duplicated tuples, and that's OK

SoulPancake · 2025-06-13T05:53:13Z

@aaguiarz

the test will fail because they are duplicated tuples, and that's OK

In that case we can just remove the map entirely and just load up all the tuples from all the files

coderabbitai · 2025-06-17T05:27:52Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

"""

Walkthrough

The changes introduce support for specifying multiple global tuple files in configuration and documentation, update example files to demonstrate this capability, and refactor the tuple loading logic to handle multiple sources and aggregate errors. A comprehensive test suite is added to verify tuple loading behavior under various scenarios.

Changes

File(s)	Change Summary
README.md	Updated documentation to clarify and exemplify usage of `tuples`, `tuple_file`, and new `tuple_files` options.
example/model.fga.yaml	Added `tuple_files` key to support multiple global tuple files in configuration.
example/model_tuples_2.yaml, example/model_tuples_3.yaml	Added new tuple files defining hierarchical parent relationships between folders.
internal/storetest/storedata.go	Refactored tuple loading logic; added support for multiple tuple files, error aggregation, and helper functions.
internal/storetest/storedata_test.go	Introduced tests covering tuple loading from various file configurations and error handling.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant StoreData
    participant FileSystem

    User->>StoreData: Call LoadTuples(basePath)
    StoreData->>FileSystem: Load tuples from Tuples (inline)
    StoreData->>FileSystem: Load tuples from TupleFile (single file)
    StoreData->>FileSystem: Load tuples from TupleFiles (multiple files)
    StoreData->>FileSystem: Load test-specific tuple files
    StoreData->>StoreData: Aggregate all loaded tuples
    StoreData->>User: Return (possibly with aggregated error)

Suggested labels

codex

Suggested reviewers

rhamzeh
"""

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.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

SoulPancake · 2025-06-17T05:29:26Z

@aaguiarz I have removed dedupe logic and updated this like we discussed, Can you please review this?

coderabbitai

Actionable comments posted: 3

🧹 Nitpick comments (4)

example/model_tuples_3.yaml (1)
1-6: Add trailing newline to satisfy linters and POSIX tools

The file is missing a newline at EOF (flagged by yamllint).
Some editors and tooling rely on this – worth adding to keep a clean CI signal.
   object: folder:marketing-Q1↵
+ 
example/model.fga.yaml (1)
9-12: Clarify precedence when both tuple_file and tuple_files are present

The example shows both keys populated, which is helpful, but users may wonder which one “wins” on duplicate paths or conflicting tuples. Consider an inline comment such as:
tuple_file: ./model_tuples.yaml  # loaded as-is, duplicates NOT filtered
tuple_files:                     # additional files – all tuples are appended
to make the behaviour explicit and avoid support questions.
internal/storetest/storedata_test.go (1)

31-37: Minor: reduce duplication to keep test fixtures minimal

tupleFile2 repeats the user:jon … doc1 tuple already present in
tupleFile. If you ever introduce a duplicate-detection mode the test
will become brittle. Consider unique tuples per file or an explicit
expectDuplicates flag to make intent clearer.

internal/storetest/storedata.go (1)

171-173: Minor wording nit

"global tuple %s file" reads awkwardly. Consider "global tuple file %s" for clarity.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bb88057 and 56792d8.

📒 Files selected for processing (6)

README.md (1 hunks)
example/model.fga.yaml (1 hunks)
example/model_tuples_2.yaml (1 hunks)
example/model_tuples_3.yaml (1 hunks)
internal/storetest/storedata.go (3 hunks)
internal/storetest/storedata_test.go (1 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (2)

internal/storetest/storedata.go (1)

internal/tuplefile/read.go (1)

ReadTupleFile (15-40)

internal/storetest/storedata_test.go (1)

internal/storetest/storedata.go (2)

StoreData (73-81)

ModelTest (63-71)

🪛 YAMLlint (1.37.1)

example/model_tuples_3.yaml

[error] 6-6: no new line character at the end of file

(new-line-at-end-of-file)

⏰ Context from checks skipped due to timeout of 90000ms (1)

GitHub Check: Test Release Process

🔇 Additional comments (3)

example/model_tuples_2.yaml (1)

1-7: Looks good – valid YAML and terminates with a newline

No issues spotted.

README.md (1)

552-559: Great addition – docs now cover tuple_files option

The explanatory block is concise and matches the implementation.

internal/storetest/storedata.go (1)

187-195: Collect errors but proceed – good pattern

Nice use of errors.Join to aggregate per-file failures while still loading the remaining tuple files.

internal/storetest/storedata_test.go

internal/storetest/storedata.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

coderabbitai

Actionable comments posted: 1

♻️ Duplicate comments (1)

internal/storetest/storedata.go (1)

125-131: Missing-tuple guard still fires too early
The guard runs before loadTestTuples, so a config that supplies tuples only via Tests[i].TupleFile is incorrectly rejected.
Move the check after loadTestTuples, or include len(storeData.Tests) > 0 in the condition.

🧹 Nitpick comments (3)

internal/storetest/storedata.go (1)

161-232: File fails gofmt
The linter reports formatting issues (e.g., mixed tabs/spaces at line 227).
Run go fmt ./... to keep CI green.

internal/storetest/storedata_test.go (2)

84-86: Loop-variable copy no longer needed (Go 1.22)
Since Go 1.22, t.Run captures the value, not the pointer, of the loop variable.
tc := tc can be deleted without risking the classic capture bug.

1-107: Please run gofmt on the test file
The formatter reports offences at lines 84 and 85.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 56792d8 and 5e6594f.

📒 Files selected for processing (2)

internal/storetest/storedata.go (3 hunks)
internal/storetest/storedata_test.go (1 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (2)

internal/storetest/storedata.go (1)

internal/tuplefile/read.go (1)

ReadTupleFile (15-40)

internal/storetest/storedata_test.go (1)

internal/storetest/storedata.go (2)

StoreData (73-81)

ModelTest (63-71)

🪛 GitHub Check: Lints

internal/storetest/storedata.go

[failure] 227-227:
File is not properly formatted (gofmt)

internal/storetest/storedata_test.go

[failure] 84-84:
File is not properly formatted (gofmt)

[failure] 85-85:
The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)

⏰ Context from checks skipped due to timeout of 90000ms (2)

GitHub Check: Test Release Process
GitHub Check: Tests

internal/storetest/storedata.go

SoulPancake · 2025-06-27T05:32:27Z

Hi @aaguiarz, just wanted to check in—would someone be able to review this sometime soon? Otherwise I'll rebase resolving the conflicts later on

rhamzeh · 2025-06-27T13:41:49Z

Hey @SoulPancake - yeah apologies for the delay. I will be taking a look early next week, most likely monday.

I have not looked extensively at the code yet, but make sure we're disallowing things like loading a tuple file multiple times in an array.

Also @aaguiarz maybe we want to deduplicate tuples before writing? As I assume it will become harder and harder for users to audit whether they have duplicates and duplicate files as this gets more expansive

SoulPancake · 2025-06-27T13:43:27Z

NP, Sure I'll rebase it on Sunday. Thanks @rhamzeh

SoulPancake · 2025-07-01T06:08:45Z

@rhamzeh I have updated the branch for this PR, can you take a look?

rhamzeh · 2025-07-07T15:05:08Z

@SoulPancake - fixing the lint issues your having in a separate PR, will merge to this once those are merged. Any chance you can give the team edit access to this?

SoulPancake · 2025-07-07T15:44:31Z

@rhamzeh I already have allowed access to edit by the maintainers

rhamzeh

@SoulPancake Looks good to me

I do have one thing for consideration - but that could be in a follow up PR - if we decide to do it.

I'm thinking about the duplicate files and duplicate tuples, and how we'll manage them.

I do not think we should error on duplicates, as I can imagine how as user's needs get more complex, the files they're managing can grow. If they have to hunt down which file is causing a duplicate issue that can become a mess
For performance reasons, it makes sense to dedup the file names instead of loading them everytime
Deduping the tuples makes sense as it would prevent the API erroring on duplicates

But we probably need to discuss that in details before doing it - again unrelated to this PR itself.

Thank you for working on it!

rhamzeh · 2025-07-07T22:03:18Z

A review from someone not me is required now that I've modified it. Will ask around :)

feat: support multiple tuple files

ec6634d

SoulPancake requested a review from a team as a code owner June 5, 2025 06:03

SoulPancake marked this pull request as draft June 5, 2025 06:04

SoulPancake commented Jun 5, 2025

View reviewed changes

internal/storetest/storedata.go Outdated Show resolved Hide resolved

Merge branch 'main' into ab/multiple-tuple-files-in-tests

fdb8088

SoulPancake added 3 commits June 11, 2025 11:24

feat: unique tuple check in case of using both tuple_file and tuple_f…

0c175bd

…iles

feat: linter suppress for a moment

bc70125

feat: write tests for load tuple in all cases

6f9fc1e

SoulPancake commented Jun 11, 2025

View reviewed changes

internal/storetest/storedata.go Outdated Show resolved Hide resolved

Merge branch 'main' into ab/multiple-tuple-files-in-tests

0331c66

SoulPancake marked this pull request as ready for review June 11, 2025 12:26

SoulPancake added 3 commits June 11, 2025 18:17

feat: extend dedupe logic for tuple key as well

1007053

feat: add the lint fixes

58f9f83

feat: update readme doc

283b247

SoulPancake requested review from a team as code owners June 12, 2025 05:40

SoulPancake commented Jun 12, 2025

View reviewed changes

README.md Show resolved Hide resolved

SoulPancake added a commit to SoulPancake/openfga.dev that referenced this pull request Jun 12, 2025

feat: documentation changes for multiple tuple files support in tests

03d09a8

this is linked to the changes proposed and introduced in openfga/cli#506

SoulPancake mentioned this pull request Jun 12, 2025

feat: documentation changes for multiple tuple files support in tests openfga/openfga.dev#1050

Merged

4 tasks

Merge branch 'main' into ab/multiple-tuple-files-in-tests

a0e1dcf

feat: remove dedup logic

5c25af7

coderabbitai bot reviewed Jun 17, 2025

View reviewed changes

internal/storetest/storedata_test.go Show resolved Hide resolved

internal/storetest/storedata.go Show resolved Hide resolved

internal/storetest/storedata.go Outdated Show resolved Hide resolved

feat: apply suggestions from code review

5e6594f

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

coderabbitai bot reviewed Jun 17, 2025

View reviewed changes

internal/storetest/storedata.go Show resolved Hide resolved

fix: lint

8dc1ae6

SoulPancake added 2 commits July 1, 2025 11:33

Merge branch 'main' into ab/multiple-tuple-files-in-tests

1d7a9d2

feat: lint and update

66bc449

feat: lint

7112dc0

rhamzeh mentioned this pull request Jul 7, 2025

chore: fix lint #531

Merged

4 tasks

rhamzeh added 2 commits July 7, 2025 17:33

Merge branch 'main' into ab/multiple-tuple-files-in-tests

e66bb43

chore: fix lint

20f9021

rhamzeh approved these changes Jul 7, 2025

View reviewed changes

rhamzeh requested a review from jimmyjames July 7, 2025 22:03

jimmyjames approved these changes Jul 7, 2025

View reviewed changes

rhamzeh added this pull request to the merge queue Jul 7, 2025

Merged via the queue into openfga:main with commit 043d54e Jul 7, 2025
19 checks passed

SoulPancake mentioned this pull request Jul 8, 2025

Support multiple tuple files in tests #233

Closed

This was referenced Jul 11, 2025

Fix model test tuple file validation #540

Closed

fix: store tuples being required #541

Merged

This was referenced Jul 11, 2025

Support Multiple Tuple Files in the store file openfga/vscode-ext#380

Open

Support Multiple Tuple Files in the store file openfga/intellij-plugin#97

Open

Copilot AI mentioned this pull request Jul 16, 2025

Support Multiple Tuple Files in Store Configuration openfga/vscode-ext#382

Draft

feat: support multiple tuple files #506

feat: support multiple tuple files #506

Uh oh!

Conversation

SoulPancake commented Jun 5, 2025 • edited by rhamzeh Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

References

Review Checklist

Summary by CodeRabbit

Uh oh!

Uh oh!

SoulPancake commented Jun 9, 2025

Uh oh!

aaguiarz commented Jun 9, 2025

Uh oh!

SoulPancake commented Jun 10, 2025

Uh oh!

SoulPancake commented Jun 11, 2025

Uh oh!

Uh oh!

Uh oh!

aaguiarz commented Jun 12, 2025

Uh oh!

SoulPancake commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaguiarz commented Jun 12, 2025

Uh oh!

SoulPancake commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Sequence Diagram(s)

Suggested labels

Suggested reviewers

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

SoulPancake commented Jun 17, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SoulPancake commented Jun 27, 2025

Uh oh!

rhamzeh commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SoulPancake commented Jun 27, 2025

Uh oh!

SoulPancake commented Jul 1, 2025

Uh oh!

rhamzeh commented Jul 7, 2025

Uh oh!

SoulPancake commented Jul 7, 2025

Uh oh!

rhamzeh left a comment

Choose a reason for hiding this comment

Uh oh!

rhamzeh commented Jul 7, 2025

Uh oh!

Uh oh!

Uh oh!

SoulPancake commented Jun 5, 2025 •

edited by rhamzeh

Loading

SoulPancake commented Jun 12, 2025 •

edited

Loading

SoulPancake commented Jun 13, 2025 •

edited

Loading

coderabbitai bot commented Jun 17, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

rhamzeh commented Jun 27, 2025 •

edited

Loading