-
Notifications
You must be signed in to change notification settings - Fork 14
chore: Migrate CI to sonatype central portal #171
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.
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes involve downgrading versions of several dependencies and GitHub Actions in build scripts and workflow files, updating Sonatype Nexus repository URLs, and making documentation clarifications. In the Java client, telemetry attribute collection and batch check request customization were enhanced, and a new test was added to verify batch check behavior with options. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OpenFgaClient
participant OpenFgaApi
Client->>OpenFgaClient: batchCheck(checks, options)
OpenFgaClient->>OpenFgaClient: Build BatchCheckRequest
OpenFgaClient->>OpenFgaClient: If options.consistency, set on request
OpenFgaClient->>OpenFgaClient: If options.authorizationModelId or default, set on request
OpenFgaClient->>OpenFgaApi: batchCheck(storeId, request, configOverride)
OpenFgaApi-->>OpenFgaClient: BatchCheckResponse
OpenFgaClient-->>Client: BatchCheckResponse
sequenceDiagram
participant OpenFgaApi
participant Telemetry
OpenFgaApi->>OpenFgaApi: buildTelemetryAttributes(request)
alt request is BatchCheckRequest and authorizationModelId present
OpenFgaApi->>Telemetry: Add FGA_CLIENT_REQUEST_MODEL_ID attribute
end
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.38.1)src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java✨ Finishing Touches
🪧 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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #171 +/- ##
============================================
+ Coverage 33.42% 33.66% +0.23%
- Complexity 994 1003 +9
============================================
Files 182 182
Lines 6878 6889 +11
Branches 772 776 +4
============================================
+ Hits 2299 2319 +20
+ Misses 4476 4466 -10
- Partials 103 104 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
README.md (1)
613-642
: Code snippet does not compile – multiple typos and syntax errorsThe example introduced for
batchCheck
contains several issues that will confuse users copying it verbatim:
var reequst
– typo in variable name.- Two of the list elements use
new ClientCheckRequest()
instead ofnew ClientBatchCheckItem()
.- Superfluous commas terminate the method-chaining lines (Java disallows this).
- Missing closing quote/parenthesis in
.correlationId("cor-3)
.- The final two builder calls (
.maxParallelRequests(5)
/.maxBatchSize(20)
) are split by a semicolon, breaking the chain.A minimal correction:
-var reequst = new ClientBatchCheckRequest().checks( +var request = new ClientBatchCheckRequest().checks( List.of( new ClientBatchCheckItem() .user("user:81684243-9356-4421-8fbf-a4f8d36aa31b") .relation("viewer") ._object("document:0192ab2a-d83f-756d-9397-c5ed9f3cb69a") .correlationId("cor-1") // optional, one will be generated for you if not provided .contextualTuples(List.of( new ClientTupleKey() .user("user:81684243-9356-4421-8fbf-a4f8d36aa31b") .relation("editor") ._object("document:0192ab2a-d83f-756d-9397-c5ed9f3cb69a") )), - new ClientCheckRequest() + new ClientBatchCheckItem() .user("user:81684243-9356-4421-8fbf-a4f8d36aa31b") .relation("admin") ._object("document:0192ab2a-d83f-756d-9397-c5ed9f3cb69a") - .correlationId("cor-2") // optional, one will be generated for you if not provided + .correlationId("cor-2") // optional, one will be generated for you if not provided .contextualTuples(List.of( new ClientTupleKey() .user("user:81684243-9356-4421-8fbf-a4f8d36aa31b") .relation("editor") ._object("document:0192ab2a-d83f-756d-9397-c5ed9f3cb69a") )), - new ClientCheckRequest() + new ClientBatchCheckItem() .user("user:81684243-9356-4421-8fbf-a4f8d36aa31b") .relation("creator") ._object("document:0192ab2a-d83f-756d-9397-c5ed9f3cb69a") - .correlationId("cor-3), // optional, one will be generated for you if not provided + .correlationId("cor-3"), // optional, one will be generated for you if not provided new ClientBatchCheckItem() .user("user:81684243-9356-4421-8fbf-a4f8d36aa31b") .relation("deleter") ._object("document:0192ab2a-d83f-756d-9397-c5ed9f3cb69a") .correlationId("cor-4") // optional, one will be generated for you if not provided ) ); var options = new ClientBatchCheckOptions() .additionalHeaders(Map.of("Some-Http-Header", "Some value")) .authorizationModelId("01GXSA8YR785C4FYS3C0RTG7B1") - .maxParallelRequests(5); // Max number of requests to issue in parallel, defaults to 10 - .maxBatchSize(20); // Max number of batches to split the list of checks into, defaults to 50 + .maxParallelRequests(5) // Max number of requests to issue in parallel, defaults to 10 + .maxBatchSize(20); // Max number of batches to split the list of checks into, defaults to 50Consider running the README snippets through a quick compile/lint check to prevent this type of drift in future PRs.
🧹 Nitpick comments (7)
build.gradle (1)
78-80
: JUnit/Mockito roll-back — any failing tests on 5.13.0/5.18.0?No code in the repository shows incompatibility with the newer versions. Please document the rationale; otherwise keep the latest patch releases to benefit from bug & CVE fixes.
example/example1/build.gradle (1)
3-5
: Keep plugin versions aligned with root buildIf the root project remains on Kotlin 2.1.21 in the future, diverging here will eventually break
gradle :example:check
because of mismatched stdlib versions. Prefer inheriting the version viaplugins { id "org.jetbrains.kotlin.jvm" version rootProject.kotlinVersion }
..github/workflows/main.yaml (1)
37-38
: Downgrading Codecov action drops SHA-validationv5.4.3 started verifying upload integrity; v5.4.2 does not. Consider staying on the latest patch release.
src/main/java/dev/openfga/sdk/api/model/AbstractOpenApiSchema.java (1)
137-142
: Return a primitiveboolean
instead ofBoolean
for clearer intent
isNullable()
never returnsnull
after the latest change (it always returns eitherBoolean.TRUE
orBoolean.FALSE
).
Switching the return type to the primitiveboolean
would:
- eliminate unnecessary boxing/unboxing,
- guarantee a non-null response by type, and
- slightly simplify call-sites (
if (schema.isNullable()) …
instead of unboxing).- public Boolean isNullable() { - if (Boolean.TRUE.equals(isNullable)) { - return Boolean.TRUE; - } - return Boolean.FALSE; + public boolean isNullable() { + return Boolean.TRUE.equals(isNullable); }src/main/java/dev/openfga/sdk/api/OpenFgaApi.java (1)
1154-1161
: Nice – telemetry now capturesauthorizationModelId
forBatchCheckRequest
The extra attribute closes a gap in observability. 👍
Longer-term, the same “copy-paste” block appears for 6 request types (CheckRequest
,ExpandRequest
, …). Consider extracting a small helper (e.g.,putModelIdIfPresent(Object request, Map<Attribute,String> attrs)
) to reduce duplication and future drift.README.md (1)
608-609
: Eliminate blank line inside the blockquote
markdownlint
(MD028) flags the empty line that follows the> **Note** …
text.-> **Note**: The order of `batchCheck` results is not guaranteed to match the order of the checks provided. Use `correlationId` to pair responses with requests. - -> Passing `ClientBatchCheckOptions` is optional. All fields of `ClientBatchCheckOptions` are optional. +> **Note**: The order of `batchCheck` results is not guaranteed to match the order of the checks provided. Use `correlationId` to pair responses with requests. +> Passing `ClientBatchCheckOptions` is optional. All fields of `ClientBatchCheckOptions` are optional.src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java (1)
2043-2074
: LGTM with minor consistency suggestion.The test correctly verifies that the
batchCheck
method includes authorization model ID and consistency options in the request body when provided. The test structure, mocking, and assertions are all appropriate.Minor observation: This test uses
.join()
on line 2067, while most other tests in this file use.get()
for CompletableFuture operations. Consider using.get()
for consistency with the existing test patterns.- ClientBatchCheckResponse response = fga.batchCheck(request, options).join(); + ClientBatchCheckResponse response = fga.batchCheck(request, options).get();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/fossa.yaml
(1 hunks).github/workflows/main.yaml
(3 hunks)README.md
(2 hunks)build.gradle
(4 hunks)example/example1/build.gradle
(1 hunks)publish.gradle
(1 hunks)src/main/java/dev/openfga/sdk/api/OpenFgaApi.java
(1 hunks)src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java
(1 hunks)src/main/java/dev/openfga/sdk/api/model/AbstractOpenApiSchema.java
(1 hunks)src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/dev/openfga/sdk/api/OpenFgaApi.java (1)
src/main/java/dev/openfga/sdk/telemetry/Attributes.java (1)
Attributes
(31-216)
🪛 markdownlint-cli2 (0.17.2)
README.md
609-609: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (7)
.github/workflows/fossa.yaml (1)
25-33
: Why downgradefossas/fossa-action
?
v1.7.0
contains the Gradle-project detection and a bug-fix for large dependency graphs. Rolling back tov1.6.0
may silently skip those improvements and will not receive further security updates.Please confirm that the downgrade is intentional and still compatible with the latest FOSSA SaaS backend.
build.gradle (2)
68-70
: Downgrading OpenTelemetry BOM loses critical fixesOTel 1.50.0 fixed context-propagation NPEs that affect async SDK calls (§ otel-java#6996). Verify that reverting to 1.49.0 will not break
BatchCheck
telemetry recently added.
109-111
: Testcontainers 1.21.0 lacks the fix for Docker 26 on macOS1.21.1 added a workaround for the new cgroup layout. Consider staying on 1.21.1 unless the downgrade solves a specific Central-Portal issue.
.github/workflows/main.yaml (2)
28-38
:gradle/actions/setup-gradle@v4.3.1
missed the fix for corrupted build-cache keysv4.4.0 introduced deterministic cache-key generation for Gradle 8.8+. Reverting may increase CI time due to more cache misses.
61-66
: Wrapper-validation downgradeGradle 8.8 changed the distribution checksum format. v4.3.1 cannot validate wrappers generated by 8.8+. Ensure your wrapper remains ≤ 8.7 or keep v4.4.0.
README.md (1)
177-177
: Heading rename looks goodDropping the “Auth0” qualifier makes the section title more generic and consistent with the other credential-type headings.
src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java (1)
714-730
: LGTM! Enhanced BatchCheckRequest construction with proper conditional parameter handling.The changes correctly implement conditional inclusion of consistency and authorization model ID parameters. The logic properly prioritizes options over client defaults and includes appropriate null/whitespace checks before setting the authorization model ID.
This enhancement aligns well with similar patterns used in other methods like
check
,expand
, andlistObjects
in the same class.
id 'com.diffplug.spotless' version '7.0.3' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spotless downgrade re-introduces CVE-2024-34329 transitively
Spotless 7.0.4 shaded the vulnerable commons-text
range; 7.0.3 did not.
Unless the downgrade is required for the Central-Portal migration, keep 7.0.4 or explicitly override commons-text
.
- id 'com.diffplug.spotless' version '7.0.3'
+ id 'com.diffplug.spotless' version '7.0.4'
📝 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.
id 'com.diffplug.spotless' version '7.0.3' | |
id 'com.diffplug.spotless' version '7.0.4' |
🤖 Prompt for AI Agents
In build.gradle around lines 7 to 8, the Spotless plugin version is downgraded
to 7.0.3, which reintroduces the CVE-2024-34329 vulnerability due to an unshaded
vulnerable commons-text dependency. To fix this, either revert the Spotless
plugin version back to 7.0.4 or, if the downgrade is necessary, explicitly add a
dependency override for commons-text to a safe version that mitigates the
vulnerability.
nexusUrl.set(uri('https://ossrh-staging-api.central.sonatype.com/service/local/')) | ||
snapshotRepositoryUrl.set(uri('https://central.sonatype.com/repository/maven-snapshots/')) | ||
username.set(System.getenv('MAVEN_USERNAME')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify new Central-Portal endpoints
According to Sonatype’s migration guide the staging API is
https://central.sonatype.com/api/v1/staging
and snapshots stay on s01.oss.sonatype.org
.
The URLs used here (ossrh-staging-api.central.sonatype.com
and /repository/maven-snapshots/
) are not documented and may 404.
- nexusUrl.set(uri('https://ossrh-staging-api.central.sonatype.com/service/local/'))
- snapshotRepositoryUrl.set(uri('https://central.sonatype.com/repository/maven-snapshots/'))
+ nexusUrl.set(uri('https://central.sonatype.com/api/v1/staging'))
+ snapshotRepositoryUrl.set(uri('https://s01.oss.sonatype.org/content/repositories/snapshots/'))
Also, the workflows still use OSSRH_USERNAME/OSSRH_TOKEN
; Central Portal requires a generated ‘token’ credential instead.
📝 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.
nexusUrl.set(uri('https://ossrh-staging-api.central.sonatype.com/service/local/')) | |
snapshotRepositoryUrl.set(uri('https://central.sonatype.com/repository/maven-snapshots/')) | |
username.set(System.getenv('MAVEN_USERNAME')) | |
nexusUrl.set(uri('https://central.sonatype.com/api/v1/staging')) | |
snapshotRepositoryUrl.set(uri('https://s01.oss.sonatype.org/content/repositories/snapshots/')) | |
username.set(System.getenv('MAVEN_USERNAME')) |
🤖 Prompt for AI Agents
In publish.gradle around lines 54 to 56, update the nexusUrl to
'https://central.sonatype.com/api/v1/staging' as per Sonatype's migration guide,
and change the snapshotRepositoryUrl to use the 's01.oss.sonatype.org' domain
for snapshots. Also, replace the environment variables for authentication from
'OSSRH_USERNAME' and 'OSSRH_TOKEN' to use the new Central Portal generated token
credentials accordingly.
Description
Migration of java sdk publishing from OSSRH to Sonatype Central Portal.
Sonatype has announced that the legacy OSSRH service (oss.sonatype.org) will reach end-of-life in June 2025. The openfga/java-sdk currently publishes to Maven Central via OSSRH. To avoid disruptions, we need to migrate publishing to the Central Portal (central.sonatype.com).
References
openfga/sdk-generator#562
Review Checklist
main
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores