Conversation
|
While I understand the fix itself or at least what qwen 3.5 claims, I don't understand why the error is not easily reproductible. Why do we have to follow a specific query execution (cf query long list of cypher queries) ? Why didn't we see this issue before ? |
Summary of ChangesHello @ExtReMLapin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical IllegalStateException that occurred during the update of security file access configurations. The fix ensures that the system correctly parses the JSON structure for type-specific permissions, preventing errors when new database elements, such as edge types, are created, by accessing the correct nested JSON object. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request fixes an IllegalStateException that occurs when updating security file access. The root cause was an incorrect lookup in a JSON structure, which is now corrected by accessing the nested 'types' object. The fix is correct and addresses the issue described. I have added one comment regarding a potential performance optimization by moving a repeated method call out of a loop.
|
|
||
| final JSONObject t; | ||
| if (defaultGroup.has(typeName)) { | ||
| final JSONObject defaultGroupTypes = defaultGroup.getJSONObject("types"); |
There was a problem hiding this comment.
This call to defaultGroup.getJSONObject("types") is inside a loop (the for loop at line 185) and could be executed multiple times. For better performance, consider extracting this call out of the loop. You could define defaultGroupTypes once before the loop, for example around line 183 where defaultType is also derived from defaultGroup.
🧪 CI InsightsHere's what we observed from your CI run for 6f03e3c. 🟢 All jobs passed!But CI Insights is watching 👀 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3521 +/- ##
==========================================
- Coverage 65.76% 65.70% -0.06%
==========================================
Files 1471 1471
Lines 100150 100151 +1
Branches 20874 20874
==========================================
- Hits 65860 65807 -53
- Misses 25305 25328 +23
- Partials 8985 9016 +31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
ref 6e10936 |
Qwen 3.5 27B wrote the fix, while running in claude code binary
fixes #3514
Error
java.lang.IllegalStateException: Not a JSON Object: []
at com.arcadedb.serializer.json.JSONObject.getJSONObject(JSONObject.java:326)
at com.arcadedb.server.security.ServerSecurityDatabaseUser.updateFileAccess(ServerSecurityDatabaseUser.java:227)
Trigger Query
Root Cause
In ServerSecurityDatabaseUser.java at line 225-227, there's a bug where the code looks for a type name at the wrong level of the JSON hierarchy:
The Problem
The variable defaultGroup is a group configuration object with this structure:
{ "*": { // <-- defaultGroup is THIS object "types": { // <-- typeName should be looked up HERE "causal_action": { "access": [...] } } } }The code incorrectly checks
defaultGroup.has(typeName)directly, but the type name is nested insidedefaultGroup.getJSONObject("types").**Qwen claims this but i'm not sure at all : **
When a new edge type is created via Cypher MERGE, the security system is updated. If the security configuration has any key at the group level that matches the type name (even accidentally), the code finds
it and tries to call getJSONObject() on it. If that value happens to be an array [], the error occurs.