Skip to content
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

feat: allow Connector's addresses to be relative URLs #241

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

fdionisi
Copy link
Member

Description

Up until now, Addresses were required to include the origin (e.g., https://my.edc.example.com/api/check/health), but it become impractical when we want to implement a UI with proxy on the same origin (e.g., /api/check/health).

This ability allow the URL to be relative, enabling the use case mentioned above.

How to test it

Added a test to ensure failure is coming from the HTTP request, and not from the URL construction. Since relative URLs work only in the browser, I wouldn't know how to test this without bringing in a new e2e setup (e.g., using playwright or cypress)


Approach

Rather then constructing the URL via new URL(...) and then adding query parameters to the instance, URL is simply a concatenation of two strings – the "baseUrl" (or relative path) and the resource path), appending at the end the query params added to a newly instance of URLSearchParams:

    const searchParams = new URLSearchParams();
    if (innerRequest.query) {
      Object.entries(innerRequest.query).forEach(([key, value]) => {
        searchParams.append(key, value);
      });
    }

    const url = `${baseUrl}${innerRequest.path}?${searchParams.toString()}`;

@fdionisi
Copy link
Member Author

@ndr-brt not ideal, but this PR also solves an issue with main where new controllers were added which uses JS private fields #inner and #context instead of TS protected ones - my bad in preparing the previous PR.

If you think we should split the work, I'm happy to do so. Otherwise we can get this in, and taking two birds with one stone.

Copy link
Contributor

@ndr-brt ndr-brt left a comment

Choose a reason for hiding this comment

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

good for me! 🚀

@fdionisi fdionisi merged commit 50f1442 into main Jun 11, 2024
1 check passed
@fdionisi fdionisi deleted the feat/accept-relative-url branch June 11, 2024 08:12
@fdionisi fdionisi added the enhancement New feature or request label Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants