Conversation
📝 WalkthroughWalkthroughAPI key resolution now prefers Changes
Sequence Diagram(s)sequenceDiagram
participant Scanner
participant ScanApi
participant Env as "Environment"
participant HTTP as "Remote API"
Scanner->>ScanApi: new ScanApi(builder) with apiKey (may be blank)
ScanApi->>Env: read SCANOSS_API_KEY if input blank
Env-->>ScanApi: returns env value or null
ScanApi-->>Scanner: stores resolved apiKey
Scanner->>ScanApi: perform scan request (uses x-api-key header if present)
ScanApi->>HTTP: send request with x-api-key header
HTTP-->>ScanApi: response
ScanApi-->>Scanner: scan result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/com/scanoss/TestCli.java (1)
136-136: Clarify why flag values differ between test methods.Flag 256 and 2048 both preserve the JSON response structure; they differ only in content (flag 256 reports all matches, flag 2048 enables path-hint logic). If both tests verify the same behavior, consider standardizing the
-Fflag value for consistency—either update line 200 to use 2048 or line 136 to use 256, depending on which behavior each test intends to validate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/scanoss/TestCli.java` at line 136, Two tests use different -F flag values (256 vs 2048) but appear to assert the same JSON-preservation behavior; pick one and make them consistent: locate the test that builds args with "--skip-snippets", "--all-extensions", "-F", "2048" and the other test that uses "-F", "256" and change the flag to the correct value consistently across both tests—use 2048 if the test expectations rely on path-hint logic, or use 256 if they only expect "report all matches" behavior—then run the tests to ensure assertions still match the chosen flag semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/test/java/com/scanoss/TestCli.java`:
- Line 136: Two tests use different -F flag values (256 vs 2048) but appear to
assert the same JSON-preservation behavior; pick one and make them consistent:
locate the test that builds args with "--skip-snippets", "--all-extensions",
"-F", "2048" and the other test that uses "-F", "256" and change the flag to the
correct value consistently across both tests—use 2048 if the test expectations
rely on path-hint logic, or use 256 if they only expect "report all matches"
behavior—then run the tests to ensure assertions still match the chosen flag
semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8c8d6c09-7500-4340-a20f-c886628f5db1
📒 Files selected for processing (1)
src/test/java/com/scanoss/TestCli.java
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/publish.yml (1)
35-40: API key added to existing env block.The
SCANOSS_API_KEYis correctly appended to the existing environment variables for this job. Minor note: there appears to be a trailing blank line (line 40) within theenvblock which is valid YAML but slightly unconventional formatting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish.yml around lines 35 - 40, The env block adds SCANOSS_API_KEY correctly but contains an unnecessary trailing blank line; edit the workflow's env mapping to remove the extra blank line so the YAML block ends immediately after SCANOSS_API_KEY: ${{ secrets.SC_API_KEY }}, keeping the existing keys (MAVEN_USERNAME, MAVEN_PASSWORD, MAVEN_GPG_PASSPHRASE, SCANOSS_API_KEY) intact and properly aligned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/publish.yml:
- Around line 35-40: The env block adds SCANOSS_API_KEY correctly but contains
an unnecessary trailing blank line; edit the workflow's env mapping to remove
the extra blank line so the YAML block ends immediately after SCANOSS_API_KEY:
${{ secrets.SC_API_KEY }}, keeping the existing keys (MAVEN_USERNAME,
MAVEN_PASSWORD, MAVEN_GPG_PASSPHRASE, SCANOSS_API_KEY) intact and properly
aligned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e71e4ce9-691e-4a27-be82-6b9dd3b9e0f3
📒 Files selected for processing (3)
.github/workflows/java-ci.yml.github/workflows/java-native-build.yml.github/workflows/publish.yml
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/java-ci.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 156-157: Update the duplicate markdown link reference so the
second reference currently labeled "[0.13.0]" is renamed to "[0.13.1]" to match
the URL comparing v0.13.0...v0.13.1; ensure the reference keys in CHANGELOG.md
are unique and that the link text [0.13.1] corresponds to the URL
https://github.com/scanoss/scanoss.java/compare/v0.13.0...v0.13.1.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9fd3e83a-187d-4629-9f1e-2372ae67c29f
📒 Files selected for processing (3)
CHANGELOG.mdpom.xmlsrc/main/java/com/scanoss/Scanner.java
✅ Files skipped from review due to trivial changes (1)
- pom.xml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/com/scanoss/rest/ScanApi.java (1)
87-87: API key resolution logic looks good overall.The new
resolveApiKeymethod correctly prioritizes user-provided keys over environment variables, and theRuntimeExceptioncatch appropriately handlesSecurityExceptionfromSystem.getenv().Minor inconsistency: The new method uses
isBlank()to check the API key, but subsequent checks at lines 95 and 128 useisEmpty(). If a whitespace-only key is provided and no env var is set, the whitespace key would be returned and later treated as valid byisEmpty().Consider aligning the checks for consistency:
♻️ Optional: Use isBlank() consistently
- } else if (this.apiKey != null && !this.apiKey.isEmpty() && (this.url == null || this.url.isEmpty())) { + } else if (this.apiKey != null && !this.apiKey.isBlank() && (this.url == null || this.url.isEmpty())) {- if (!this.headers.containsKey("x-api-key") && this.apiKey != null && !this.apiKey.isEmpty()) { + if (!this.headers.containsKey("x-api-key") && this.apiKey != null && !this.apiKey.isBlank()) {Also applies to: 133-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/scanoss/rest/ScanApi.java` at line 87, The API key checks are inconsistent: resolveApiKey uses isBlank() but later validation uses isEmpty(), so whitespace-only keys slip through; update the validation in ScanApi (the places currently using isEmpty() around the checks after resolveApiKey and the blocks you noted) to use isBlank() as well (or explicitly trim() then isEmpty()) so that whitespace-only keys are treated as invalid consistently with resolveApiKey; ensure this applies to the checks around the constructor/initialization and the later validation logic (the same code paths you referenced).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/java/com/scanoss/rest/ScanApi.java`:
- Line 87: The API key checks are inconsistent: resolveApiKey uses isBlank() but
later validation uses isEmpty(), so whitespace-only keys slip through; update
the validation in ScanApi (the places currently using isEmpty() around the
checks after resolveApiKey and the blocks you noted) to use isBlank() as well
(or explicitly trim() then isEmpty()) so that whitespace-only keys are treated
as invalid consistently with resolveApiKey; ensure this applies to the checks
around the constructor/initialization and the later validation logic (the same
code paths you referenced).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 02c44506-3a3f-4bab-9df7-37a47b1779f8
📒 Files selected for processing (3)
CHANGELOG.mdsrc/main/java/com/scanoss/Scanner.javasrc/main/java/com/scanoss/rest/ScanApi.java
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/scanoss/Scanner.java
eeisegn
deleted the
fix/change-flag-value-to-avoid-unwanted-format-change
branch
2 participants
Update unit test to use flag
-Fvalue that does not alter the response json structure.Summary by CodeRabbit
New Features
Tests
Chores
Documentation