Skip to content

miduo/224_perf: Optimize login latency and reduce redundant I/O in so…#36

Open
Miduo666 wants to merge 6 commits intoanusii:devfrom
Miduo666:miduo/224_optimize_login_latecy
Open

miduo/224_perf: Optimize login latency and reduce redundant I/O in so…#36
Miduo666 wants to merge 6 commits intoanusii:devfrom
Miduo666:miduo/224_optimize_login_latecy

Conversation

@Miduo666
Copy link

@Miduo666 Miduo666 commented Mar 2, 2026

Pull Request Details

What does this PR address

This PR optimizes the OIDC issuer discovery process by eliminating redundant network requests. Previously, we fetched the WebID profile twice—once to extract the issuer URI and again during the authentication step—which added unnecessary latency. I've introduced a simple caching mechanism using two fields: _issuerCache to store the mapping between the user-input URL and the resolved issuer, and _profileBodyCache to hold the Turtle text from the initial fetch. By reusing the data we already downloaded in the first step, we cut the network overhead in half and ensure a much snappier login experience for the user.

Associated Issue

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist

Complete the check-list below to ensure your branch is ready for PR.

  • Screenshots included in linked issue #
  • Changes adhere to the style and coding guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • Any dependent changes have been merged and published in downstream modules
  • The update contains no confidential information
  • The update has no duplicated content
  • No lint check errors are related to these changes (make prep or flutter analyze lib)
  • Integration test dart test output or screenshot included in issue #
  • I tested the PR on these devices:
    • Android
    • iOS
    • Linux
    • MacOS
    • Windows
    • Web
  • I have identified reviewers
  • The PR has been approved by reviewers

Finalising

Once PR discussion is complete and reviewers have approved:

  • Merge dev into the this branch
  • Resolve any conflicts
  • Add a one line summary into the CHANGELOG.md
  • Push to the git repository and review
  • Merge the PR into dev

@gjwgit
Copy link
Contributor

gjwgit commented Mar 9, 2026

This requires a DESCRIPTION in the PR to explain what has been done and why. Thanks.

@Miduo666
Copy link
Author

Miduo666 commented Mar 9, 2026

This requires a DESCRIPTION in the PR to explain what has been done and why. Thanks.

Hi @gjwgit ,
This PR optimizes the OIDC issuer discovery process by eliminating redundant network requests. Previously, we fetched the WebID profile twice—once to extract the issuer URI and again during the authentication step—which added unnecessary latency. I've introduced a simple caching mechanism using two fields: _issuerCache to store the mapping between the user-input URL and the resolved issuer, and _profileBodyCache to hold the Turtle text from the initial fetch. By reusing the data we already downloaded in the first step, we cut the network overhead in half and ensure a much snappier login experience for the user.

@gjwgit gjwgit requested review from anushkavidanage and cdawei March 9, 2026 22:43
Copilot AI review requested due to automatic review settings March 9, 2026 22:47
Copy link

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 aims to reduce login latency by avoiding repeated network work when resolving a Solid server’s OIDC issuer (and by caching the fetched WebID profile body for reuse).

Changes:

  • Added an in-memory issuer-resolution cache to short-circuit repeated calls to getIssuer.
  • Added an in-memory cache for fetched WebID profile document bodies plus a getter to retrieve the cached body.
  • Minor doc/comment cleanup around issuer/profile helpers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copy link

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +83 to +86
// Normalize the key so that trivially different representations of the same
// URL (e.g. percent-encoding variants) share a single cache entry.
final String cacheKey = Uri.parse(textUrl).toString();

Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

Uri.parse(textUrl) can throw a FormatException for common user-input cases (leading/trailing whitespace, embedded spaces, etc.). Because this happens before the regex fallback, it can turn previously recoverable inputs into hard failures. Consider trimming first and using Uri.tryParse(...) (or catching FormatException) and falling back to the raw/trimmed string for cacheKey when parsing fails.

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

it already implemented in solidui\lib\src\widgets\solid_login.dart, line 348, final podServer = webIdController.text.trim().isNotEmpty

Copy link

Choose a reason for hiding this comment

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

Note that solid_auth may be used by another library or app other than solidpod or solidui. Therefore, it's better to ensure any leading/trailing blank-spaces in textUrl are trimmed here in solid_auth.

Copy link

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

lib/solid_auth_issuer.dart:134

  • fetchProfileData sets Content-Type: text/turtle on a GET request, but Content-Type describes the request body (which GET doesn’t have). If the intent is to request Turtle, this should be an Accept: text/turtle header (optionally with a fallback like text/turtle,application/n-triples;q=0.9,*/*;q=0.1) to improve compatibility with content negotiation on Solid servers.
Future<String> fetchProfileData(String profUrl) async {
  final response = await http.get(
    Uri.parse(profUrl),
    headers: <String, String>{
      'Content-Type': 'text/turtle',
    },

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

/// Returns the profile card body that was fetched during [getIssuer], or `null`
/// if the profile has not been fetched yet (e.g. the server URL is a plain
/// issuer URI rather than a WebID URL).

Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The doc comment for getCachedIssuerProfileBody is split by blank (non-///) lines, so the first part becomes a dangling library doc comment and won’t be associated with the function (and may trigger dangling_library_doc_comments). Keep the comment block contiguous by using /// on blank lines or removing the blank lines between the /// lines.

Suggested change
///

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants