Skip to content

Conversation

@Thespica
Copy link
Contributor

Purpose of the PR

Main Changes

Verifying these changes

  • Trivial rework / code cleanup without any test coverage. (No Need)
  • Already covered by existing tests, such as (please modify tests here).
  • Need tests and can be verified as follows:
    • xxx

Does this PR potentially affect the following parts?

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. api Changes of API feature New feature labels Apr 20, 2025
@codecov
Copy link

codecov bot commented Apr 20, 2025

Codecov Report

Attention: Patch coverage is 90.90909% with 4 lines in your changes missing coverage. Please review.

Project coverage is 34.10%. Comparing base (1badd93) to head (1829aad).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
.../org/apache/hugegraph/auth/HugeGraphAuthProxy.java 92.50% 2 Missing and 1 partial ⚠️
...org/apache/hugegraph/auth/StandardAuthManager.java 50.00% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (1badd93) and HEAD (1829aad). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (1badd93) HEAD (1829aad)
3 1
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2754      +/-   ##
============================================
- Coverage     41.18%   34.10%   -7.09%     
+ Complexity      333      264      -69     
============================================
  Files           747      747              
  Lines         60119    60123       +4     
  Branches       7676     7676              
============================================
- Hits          24760    20504    -4256     
- Misses        32772    37316    +4544     
+ Partials       2587     2303     -284     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@imbajin imbajin requested a review from Copilot April 22, 2025 04:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for a token_expire field to the LoginAPI by updating the loginUser method signatures and their usage across the API, core, and test modules. Key changes include:

  • Updating the AuthManager and StandardAuthManager loginUser methods to accept an expiration value.
  • Modifying the LoginAPI endpoint to extract and pass the token_expire parameter.
  • Updating test cases in AuthTest to use the new loginUser method and replacing less idiomatic assertions.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/AuthTest.java Updated tests to call loginUser with an expire parameter and improved assertions (e.g., using Assert.assertNull).
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManager.java Modified loginUser signature and repositioned the isLocal method with additional comments.
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/AuthManager.java Updated the loginUser method signature to include the expire parameter.
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java Adjusted loginUser invocations and refactored context handling.
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/LoginAPI.java Updated the API to support the token_expire field during login.
Comments suppressed due to low confidence (1)

hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/AuthTest.java:1381

  • [nitpick] Consider extracting the literal 10080L into a named constant (e.g., TOKEN_EXPIRE) to improve clarity and maintainability in test cases.
authManager.loginUser("test", "pass", 10080L);


@Override
public String loginUser(String username, String password)
public String loginUser(String username, String password, long expire)
Copy link
Member

@imbajin imbajin Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not used? At least we need add a TODO/comment for it

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label May 2, 2025
@VGalaxies VGalaxies merged commit 8c1ee71 into apache:master May 5, 2025
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Changes of API feature New feature lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] LoginAPI support token_expire field

3 participants