RFR: 2167: Unify search methods in IssueProject

[Erik Duveblad]

On Tue, 30 Jan 2024 14:16:00 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> Hi all,
>> 
>> this patch unifies the various ways of searching that exist within `JiraProejct`. I also chose to expose the `search` method in the `IssueProject` since I will make use of it in later patches. This is the first patch of _many_ that will extract the JBS specific parts from `JiraProject` so that `JiraProject` will be about JIRA and JBS specific functionality will be moved elsewhere. This refactoring is needed to allow other parts of the Skara to use JBS functionality with only an `IssueProject` instance (without turning `JiraProject` into effectively `JbsProject`).
>> 
>> Thanks,
>> Erik
>
> issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraProject.java line 508:
> 
>> 506: 
>> 507:             if (count < issues.get("total").asInt()) {
>> 508:                 startAt += issues.get("issues").asArray().size();
> 
> Aren't `count` and `startAt` basically equivalent at this point? Seems a bit weird to have two values for basically the same thing but calculated differently.

Yes, I just tried to change the existing code as little as possible to try to keep the patch about refactoring. Since I will be touch this a bit more based on your other comment then I can clean this up as well.

> issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraProject.java line 513:
> 
>> 511:                              .param("startAt", String.valueOf(startAt));
>> 512:                 if (limit > 0) {
>> 513:                     req = req.param("maxResults", Integer.toString(limit));
> 
> Is the limit supposed to be the total limit or max size per page (would be great with javadoc specifying this btw)? The `maxResults` parameter is results returned per page. I would think a global limit to be more useful.

Yeah, it was meant to be a global limit (we only have one caller currently passing a limit other than -1, and that caller passes 1, so it would have worked, but I agree that the current code is wrong). Will update!

-------------

PR Review Comment: https://git.openjdk.org/skara/pull/1610#discussion_r1471408460
PR Review Comment: https://git.openjdk.org/skara/pull/1610#discussion_r1471407563