Add foreignKeys experimental setting support#980
Open
Monish-K0 wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds an experimental foreignKeys settings feature to the Java SDK: a new ForeignKey model, a foreignKeys field on Settings, get/update/reset methods on SettingsHandler and Index, integration tests, and documentation code samples. Also updates Gradle integrationTest task configuration and normalizes gradlew.bat formatting. ChangesForeign Keys Settings Feature
Build Tooling Formatting
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Index
participant SettingsHandler
participant MeilisearchAPI
Index->>SettingsHandler: updateForeignKeysSettings(uid, foreignKeys)
SettingsHandler->>MeilisearchAPI: PUT /indexes/{uid}/settings/foreign-keys
MeilisearchAPI-->>SettingsHandler: TaskInfo
SettingsHandler-->>Index: TaskInfo
Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/java/com/meilisearch/integration/SettingsTest.java (1)
1662-1703: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test coverage for resetting
foreignKeysviaupdateForeignKeysSettings(null).Every other setting in this file (synonyms, stop-words, ranking rules, displayed/searchable/filterable attributes, distinct attribute, localized attributes) has a companion
...UsingNulltest verifying the update-to-null reset path (SettingsHandler.updateForeignKeysSettingsexplicitly encodesnullfor this case). That branch is currently untested forforeignKeys.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/meilisearch/integration/SettingsTest.java` around lines 1662 - 1703, Add a null-reset coverage test for foreign keys in SettingsTest by extending the existing test around Index.updateForeignKeysSettings and getForeignKeysSettings. Create a companion test that calls updateForeignKeysSettings(null), wait for the task via waitForTask, then assert getForeignKeysSettings returns the reset/empty state and differs from the previously configured ForeignKey[]; this should mirror the existing “UsingNull” patterns used for other settings and exercise the null branch in SettingsHandler.updateForeignKeysSettings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/test/java/com/meilisearch/integration/SettingsTest.java`:
- Around line 1644-1740: The foreignKeys integration tests are exercising
experimental endpoints without enabling the feature first, so they will fail on
a default server. Update the test setup used by SettingsTest/AbstractIT to opt
into experimental features before calling Index.getForeignKeysSettings,
Index.updateForeignKeysSettings, and Index.resetForeignKeysSettings, so the
foreign-key assertions run against an enabled server.
---
Nitpick comments:
In `@src/test/java/com/meilisearch/integration/SettingsTest.java`:
- Around line 1662-1703: Add a null-reset coverage test for foreign keys in
SettingsTest by extending the existing test around
Index.updateForeignKeysSettings and getForeignKeysSettings. Create a companion
test that calls updateForeignKeysSettings(null), wait for the task via
waitForTask, then assert getForeignKeysSettings returns the reset/empty state
and differs from the previously configured ForeignKey[]; this should mirror the
existing “UsingNull” patterns used for other settings and exercise the null
branch in SettingsHandler.updateForeignKeysSettings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6abda010-f5ae-4f89-9c6d-269cc9a95725
📒 Files selected for processing (8)
.code-samples.meilisearch.yamlbuild.gradlegradlew.batsrc/main/java/com/meilisearch/sdk/Index.javasrc/main/java/com/meilisearch/sdk/SettingsHandler.javasrc/main/java/com/meilisearch/sdk/model/ForeignKey.javasrc/main/java/com/meilisearch/sdk/model/Settings.javasrc/test/java/com/meilisearch/integration/SettingsTest.java
Comment on lines
+1644
to
+1740
| /** Tests of the foreignKeys setting methods (experimental feature, Meilisearch v1.39+) */ | ||
| @Test | ||
| @DisplayName("Test get foreign keys settings by uid") | ||
| public void testGetForeignKeysSettings() throws Exception { | ||
| Index index = createIndex("testGetForeignKeysSettings"); | ||
|
|
||
| // Fetch global settings and the dedicated endpoint — they must agree. | ||
| Settings initialSettings = index.getSettings(); | ||
| ForeignKey[] initialForeignKeys = index.getForeignKeysSettings(); | ||
|
|
||
| int dedicatedSize = initialForeignKeys == null ? 0 : initialForeignKeys.length; | ||
| int globalSize = | ||
| initialSettings.getForeignKeys() == null | ||
| ? 0 | ||
| : initialSettings.getForeignKeys().length; | ||
| assertThat(dedicatedSize, is(equalTo(globalSize))); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Test update foreign keys settings") | ||
| public void testUpdateForeignKeysSettings() throws Exception { | ||
| Index index = createIndex("testUpdateForeignKeysSettings"); | ||
|
|
||
| // Record what is there before we make any changes. | ||
| ForeignKey[] initialForeignKeys = index.getForeignKeysSettings(); | ||
|
|
||
| // Build two ForeignKey entries using setters (mirrors the LocalizedAttribute pattern). | ||
| ForeignKey firstKey = new ForeignKey(); | ||
| firstKey.setForeignIndexUid("authors"); // the related index uid | ||
| firstKey.setFieldName("author_id"); // the field in THIS index that holds the link | ||
|
|
||
| ForeignKey secondKey = new ForeignKey(); | ||
| secondKey.setForeignIndexUid("genres"); | ||
| secondKey.setFieldName("genre_id"); | ||
|
|
||
| ForeignKey[] newForeignKeys = new ForeignKey[] {firstKey, secondKey}; | ||
|
|
||
| // Push the update and wait for the async task to finish. | ||
| index.waitForTask(index.updateForeignKeysSettings(newForeignKeys).getTaskUid()); | ||
|
|
||
| // Fetch the setting back and verify every field was stored correctly. | ||
| ForeignKey[] updatedForeignKeys = index.getForeignKeysSettings(); | ||
|
|
||
| assertThat(updatedForeignKeys, is(arrayWithSize(newForeignKeys.length))); | ||
| assertThat( | ||
| updatedForeignKeys[0].getForeignIndexUid(), | ||
| is(equalTo(newForeignKeys[0].getForeignIndexUid()))); | ||
| assertThat( | ||
| updatedForeignKeys[0].getFieldName(), | ||
| is(equalTo(newForeignKeys[0].getFieldName()))); | ||
| assertThat( | ||
| updatedForeignKeys[1].getForeignIndexUid(), | ||
| is(equalTo(newForeignKeys[1].getForeignIndexUid()))); | ||
| assertThat( | ||
| updatedForeignKeys[1].getFieldName(), | ||
| is(equalTo(newForeignKeys[1].getFieldName()))); | ||
|
|
||
| // The updated setting must differ from what was there initially. | ||
| assertThat(updatedForeignKeys, is(not(equalTo(initialForeignKeys)))); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Test reset foreign keys settings") | ||
| public void testResetForeignKeysSettings() throws Exception { | ||
| Index index = createIndex("testResetForeignKeysSettings"); | ||
|
|
||
| ForeignKey[] initialForeignKeys = index.getForeignKeysSettings(); | ||
|
|
||
| // Apply some foreign keys first. | ||
| ForeignKey firstKey = new ForeignKey(); | ||
| firstKey.setForeignIndexUid("authors"); | ||
| firstKey.setFieldName("author_id"); | ||
|
|
||
| ForeignKey secondKey = new ForeignKey(); | ||
| secondKey.setForeignIndexUid("genres"); | ||
| secondKey.setFieldName("genre_id"); | ||
|
|
||
| ForeignKey[] newForeignKeys = new ForeignKey[] {firstKey, secondKey}; | ||
|
|
||
| index.waitForTask(index.updateForeignKeysSettings(newForeignKeys).getTaskUid()); | ||
| ForeignKey[] updatedForeignKeys = index.getForeignKeysSettings(); | ||
|
|
||
| // Now reset and verify the setting returned to its initial state. | ||
| index.waitForTask(index.resetForeignKeysSettings().getTaskUid()); | ||
| ForeignKey[] foreignKeysAfterReset = index.getForeignKeysSettings(); | ||
|
|
||
| assertThat(updatedForeignKeys, is(arrayWithSize(newForeignKeys.length))); | ||
| assertThat( | ||
| updatedForeignKeys[0].getForeignIndexUid(), | ||
| is(equalTo(newForeignKeys[0].getForeignIndexUid()))); | ||
| assertThat( | ||
| updatedForeignKeys[0].getFieldName(), | ||
| is(equalTo(newForeignKeys[0].getFieldName()))); | ||
|
|
||
| // After reset the setting must differ from the updated state. | ||
| assertThat(foreignKeysAfterReset, is(not(equalTo(updatedForeignKeys)))); | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for experimental-features enablement in test setup / CI config
rg -n -i "experimental" src/test/java/com/meilisearch/integration/AbstractIT.java 2>/dev/null
fd -i "AbstractIT.java"
rg -n -i "experimental-feature|experimentalFeatures" -g '*.java' -g '*.yml' -g '*.yaml'Repository: meilisearch/meilisearch-java
Length of output: 232
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== AbstractIT outline ==\n'
ast-grep outline src/test/java/com/meilisearch/integration/classes/AbstractIT.java --view expanded || true
printf '\n== AbstractIT excerpt ==\n'
sed -n '1,260p' src/test/java/com/meilisearch/integration/classes/AbstractIT.java
printf '\n== CI/test config mentions of experimental features ==\n'
rg -n -i "experimental-feature|experimentalFeatures|PATCH /experimental-features|foreignKeys|experimental" .github src/test/java src/test/resources . 2>/dev/null || trueRepository: meilisearch/meilisearch-java
Length of output: 22060
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files in integration test area ==\n'
git ls-files 'src/test/java/com/meilisearch/integration/**' | sed -n '1,120p'
printf '\n== Search for foreignKeys feature enablement ==\n'
rg -n -i "foreignKeys|experimental-features|experimentalFeatures|PATCH /experimental-features" src/test/java .github . 2>/dev/null || trueRepository: meilisearch/meilisearch-java
Length of output: 14480
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Integration test base class ==\n'
sed -n '1,260p' src/test/java/com/meilisearch/integration/classes/AbstractIT.java
printf '\n== Test/CI config files ==\n'
git ls-files '.github/**' 'pom.xml' 'build.gradle*' 'gradle.properties' 'docker-compose*' 'src/test/resources/**' | sed -n '1,200p'
printf '\n== Experimental feature env/config search ==\n'
rg -n -i "MEILI_EXPERIMENTAL|experimental-features|experimentalFeatures|foreignKeys|meilisearch.*test|docker.*meilisearch|AbstractIT" .github pom.xml build.gradle* gradle.properties docker-compose* src/test/resources src/test/java 2>/dev/null || trueRepository: meilisearch/meilisearch-java
Length of output: 17192
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== AbstractIT =="
sed -n '1,260p' src/test/java/com/meilisearch/integration/classes/AbstractIT.java
echo
echo "== Workflow / CI files referencing Meilisearch =="
git ls-files '.github/workflows/**' '.github/**' 'pom.xml' 'docker-compose*' 'src/test/resources/**' | sed -n '1,200p'
echo
echo "== Grep for feature enablement / test launcher =="
rg -n -i "experimental-features|experimentalFeatures|MEILI_EXPERIMENTAL|foreignKeys|meilisearch" \
.github/workflows .github pom.xml docker-compose* src/test/resources src/test/java 2>/dev/null || trueRepository: meilisearch/meilisearch-java
Length of output: 45028
Enable foreignKeys in the integration-test setup
AbstractIT and the CI workflows start Meilisearch without any /experimental-features opt-in, so these tests will hit feature_not_enabled on a default server. Add the enablement before exercising the foreign-key endpoints.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/java/com/meilisearch/integration/SettingsTest.java` around lines
1644 - 1740, The foreignKeys integration tests are exercising experimental
endpoints without enabling the feature first, so they will fail on a default
server. Update the test setup used by SettingsTest/AbstractIT to opt into
experimental features before calling Index.getForeignKeysSettings,
Index.updateForeignKeysSettings, and Index.resetForeignKeysSettings, so the
foreign-key assertions run against an enabled server.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Related issue
Fixes #978
What does this PR do?
Changes
Testing
PR checklist
Please check if your PR fulfills the following requirements:
AI disclosure: I used ChatGPT to help understand the repository structure, follow existing patterns, and generate the initial implementation. I reviewed all the code, verified field names against the official Meilisearch API docs, and confirmed everything works by running tests against a live Meilisearch instance.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores