Skip to content

Conversation

cmark
Copy link
Member

@cmark cmark commented Nov 25, 2024

When working with async streaming contexts the system was unable to propagate authorization information to the sub-request which was resulted in an error. This PR fixes that.

Jira: https://snowowl.atlassian.net/browse/SO-6339

... SearchPageableCollectionResourceRequestBuilder

Prefer using the context variant when there is a context available
otherwise ensure that callers always supply the necessary request
headers so that they are not failing with Missing authorization token
exception when the initiating request was already successfully
authenticated.

Fixes https://snowowl.atlassian.net/browse/SO-6339
@cmark cmark added the bug label Nov 25, 2024
@cmark cmark requested review from apeteri and AAAlinaaa November 25, 2024 11:18
@cmark cmark self-assigned this Nov 25, 2024
Copy link

codecov bot commented Nov 25, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 63.67%. Comparing base (820f68d) to head (b82ac5a).
Report is 15 commits behind head on 9.x.

Files with missing lines Patch % Lines
...earchPageableCollectionResourceRequestBuilder.java 54.54% 5 Missing ⚠️
...wowl/core/conceptmap/ConceptMapCompareRequest.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                9.x    #1346      +/-   ##
============================================
- Coverage     63.67%   63.67%   -0.01%     
- Complexity    12523    12524       +1     
============================================
  Files          1612     1612              
  Lines         57120    57126       +6     
  Branches       5622     5622              
============================================
+ Hits          36373    36376       +3     
- Misses        18397    18401       +4     
+ Partials       2350     2349       -1     

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

Copy link
Member

@apeteri apeteri left a comment

Choose a reason for hiding this comment

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

RSLGTM!

}

public final Stream<R> streamAsync(IEventBus bus, Map<String, String> headers, Function<B, AsyncRequest<R>> req) {
return Streams.stream(new SearchResourceRequestIterator<B, R>(getSelf(), (builder) -> req.apply(builder).withHeaders(headers).execute(bus).getSync(3, TimeUnit.MINUTES)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we document these timeouts?

Copy link
Member Author

Choose a reason for hiding this comment

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

Somewhere yes, but that requires a bit more effort and there are a couple of these lying around in the code. Most of them are around 3 minutes. Probably we need to extract them into a static/dynamic config value. Will create a Jira for it.

@cmark cmark merged commit 8acc09a into 9.x Nov 25, 2024
4 of 5 checks passed
@cmark cmark deleted the issue/SO-6339-upgrade-missing-authorization branch November 25, 2024 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants