Multi threading download from API is this optimal or u see potential issue?

[grizzled skiff]

    public List<ClientResponse> findAllRepositoriesByUsername(String username) {
        long start = System.currentTimeMillis();
        List<RepositoriesResponseAPI> userRepositories = makeRequestForUserRepositories(username);

        List<ClientResponse> clientData = new LinkedList<>();
        ExecutorService executorService = Executors.newFixedThreadPool(checkBestThreadNumber(userRepositories));

        userRepositories.forEach(repository -> { executorService.execute(() -> {
                    List<BranchesResponseAPI> repositoryBranches = makeRequestForRepositoryBranches(repository.owner().login(), repository.name());
                    ClientResponse clientResponse = MapperResponseAPI.mapToClientResponse(repository, repositoryBranches);
                    synchronized (clientData) {
                        clientData.add(clientResponse);
                    }
                });
        });

        executorService.shutdown();
        try {
            executorService.awaitTermination(THREADS_WAITING_TIME, TimeUnit.SECONDS);
        } catch (InterruptedException e) {
            throw new RuntimeException(e);
        }
        System.out.println(System.currentTimeMillis()-start);
        return clientData;
    }

You think its good or there is something dangerous? And what I can do by alternative? Is clientData as an key good option?

[nova laurel]

⌛ This post has been reserved for your question.

Hey @grizzled skiff! Please use /close or the Close Post button above when your problem is solved. Please remember to follow the help guidelines. This post will be automatically closed after 300 minutes of inactivity.

TIP: Narrow down your issue to simple and precise questions to maximize the chance that others will reply in here.

[wintry ferry]

arent you making it single threaded by adding synchronized in the chain?

userRepositories.stream
  .map(repo -> supplyAsync(your call here, executor))
  .toList()
  .stream()
  .map(CF::join)
  .toList

this is what i would do

[grizzled skiff]

dont think so this is so fast and synchronsed works only when thread trying to access list so downloading from api and mapping is not sync

But i will test that and check results

if it is faster

unfortunely I have problem because github API has 100requests per hour and I have to reset router all time XD

[deep oyster]

i wouldnt necessarily exceed that if possible

or just sign in to increase that limit

[grizzled skiff]

singing in is not part of my project, im making dumb web client which will simulate response for development

But I have a little problem with downloading data

can you tell me what should I do with webClient?

    @Override
    public List<RepositoriesResponseAPI> fetchRepositories(String login) {
        try {
           ResponseEntity<List<RepositoriesResponseAPI>> response = webClient
                .get()
                .uri(urlBuilder.buildRepositoryUrl(login))
                .retrieve()
                .toEntityList(RepositoriesResponseAPI.class)
                .block();

           List<RepositoriesResponseAPI> repositories = response.getBody();

           if(repositories == null) {
               //
           }

           return repositories;

        } catch (Exception e) {
            return null;
        }
    }

ohh

nvm its not about web client

only mapper

    static ClientResponse mapToClientResponse(RepositoriesResponseAPI repository, List<BranchesResponseAPI> branches) {

        if(branches == null) {
            System.out.println(repository);
        }

        if(branches.isEmpty()) {
            throw new NullPointerException("Each repository must have at least one branch");
        }

        return new ClientResponse(
            repository.name(),
            repository.owner().login(),
            branches.stream()
                .map(branch -> new ClientResponse.Branch(
                    branch.name(),
                    branch.commit().sha()))
                .toList()
        );
    }

API cannot always download branches

and branches are null somehow

idk why

tbh

sometimes while im downloading branches it returns null idk why haha

[deep oyster]

it would help if you would show the branch downloading code

[grizzled skiff]

you mean url

or what

[deep oyster]

the code that downloads and provides the branches list

since you said that is null sometimes

[grizzled skiff]

actually second fetcher is broken too

idk

    @Override
    public List<RepositoriesResponseAPI> fetchRepositories(String login) {
        try {
           return webClient
                .get()
                .uri(urlBuilder.buildRepositoryUrl(login))
                .retrieve()
                .toEntityList(RepositoriesResponseAPI.class)
                .block()
                .getBody();

        } catch (Exception e) {
            return null;
        }
    }

    @Override
    public List<BranchesResponseAPI> fetchBranches(String login, String repositoryName) {
        try {
            return webClient
                .get()
                .uri(urlBuilder.buildBranchesUrl(login, repositoryName))
                .retrieve()
                .toEntityList(BranchesResponseAPI.class)
                .block()
                .getBody();

        } catch (Exception e) {
            return null;
        }
    }

right now fetchRepositories returns null

while im googling this url and it returns array

urlBuilder.buildRepositoryUrl(login) returns https://api.github.com/users/kalqa/repos

but my code now returns null

[deep oyster]

well

do you see the problem

do you see where the method might return null

[grizzled skiff]

it throws NullPointerException while im trying get size() of null (expected List)

the only way is during this request

and its weird for me because it worked some time ago xD

    @Bean
    public WebClient createWebClient() {
        return WebClient.builder()
            .defaultHeader(CONTENT_TYPE, APPLICATION_JSON_VALUE)
            .build();
    }

maybe config is bad?

but it worked so it does not makes sense for me xD

Okay

repositories worked since I reset router

and after return I dont download any null branch

so this api is very restricted

[deep oyster]

do you see why the fetchBranches method here might return null

or why any web request doing method in your program might return null for that matter

[grizzled skiff]

getBody?

[deep oyster]

do you comprehend the try catch around the entire thing

all of your web methods have a try catch around them that just blanket catches Exception, does nothing and returns null

i'd at least print the stack trace

[grizzled skiff]

yes I will do that I have restrictions what I should return but thats later

now im planing to do DumbClient to stop have problems with API

[nova laurel]

💤 Post marked as dormant

This post has been inactive for over 300 minutes, thus, it has been archived.
If your question was not answered yet, feel free to re-open this post or create a new one.
In case your post is not getting any attention, you can try to use /help ping.
Warning: abusing this will result in moderative actions taken against you.

[grizzled skiff]

After some consideration I agree with you it could be better way

But dont understand why u did stream().toList().stream().map() there

could you explain?

@wintry ferry

[wintry ferry]

gotta collect futures

[grizzled skiff]

ah so join works wth List of futures

[wintry ferry]

if you dont join will block the chain

[grizzled skiff]

and will wait for all

    public List<ClientResponse> findAllRepositoriesByUsername(String username) {
        long start = System.currentTimeMillis();
        List<RepositoriesResponseAPI> userRepositories = makeRequestForUserRepositories(username);
        ExecutorService executorService = Executors.newFixedThreadPool(checkBestThreadNumber(userRepositories));
        List<ClientResponse> clientResponses = userRepositories
            .stream()
            .map(repo -> CompletableFuture.supplyAsync(() -> {
                List<BranchesResponseAPI> repositoryBranches = makeRequestForRepositoryBranches(repo.owner().login(), repo.name());
                return MapperResponseAPI.mapToClientResponse(repo, repositoryBranches);
            }, executorService))
            .toList()
            .stream()
            .map(CompletableFuture::join)
            .toList();

        System.out.println(System.currentTimeMillis()-start);
        return clientResponses;
    }

I have something like that

but not sure I should use Mapper there

[wintry ferry]

seems fine

[grizzled skiff]

okay and whats about request

it could be Mono?

already I have sync request

    @Override
    public List<BranchesResponseAPI> fetchBranches(String login, String repositoryName) {
            return githubWebClient
                .get()
                .uri(uriBuilder.buildBranchesUrl(login, repositoryName))
                .retrieve()
                .onStatus(HttpStatusCode::isError, this::handleError)
                .toEntityList(BranchesResponseAPI.class)
                .block()
                .getBody();
    }

I think it can be even faster If I do async requests

[wintry ferry]

supplyAsync already does that

[grizzled skiff]

even with such request?

[wintry ferry]

yeah

i dont get it, just add a logger and see for yourself

you really should not be using .block()

[grizzled skiff]

I think so too

but have problem to handle this with Mono

[wintry ferry]

just return mono

[grizzled skiff]

and Dont get what Flux is but such option exists in webClient

[wintry ferry]

Flux is for infinite data

just return Mono<List<whaterver>>

[grizzled skiff]

I dont get how to handle Mono

   @Override
    public Mono<List<BranchesResponseAPI>> fetchBranches(String login, String repositoryName) {
            return githubWebClient
                .get()
                .uri(uriBuilder.buildBranchesUrl(login, repositoryName))
                .retrieve()
                .onStatus(HttpStatusCode::isError, this::handleError)
                .bodyToMono(new ParameterizedTypeReference<>() {});
    }

im returning Mono List but what next

Before Mapping I need wait for download data

so maybe I should try do Maper later

okay

add bodyToMono and next will propably work the best