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

Fix for - Internal Server Error When Using Offset >= Total Organizations and Limit = 0 in Organization Discovery GET API #680

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kumardeepak5
Copy link
Contributor

@kumardeepak5 kumardeepak5 commented Oct 4, 2024

Purpose

Resolves wso2/product-is#21025

Goals

This PR removes the cyclic recursive call to the calculateOffsetForPreviousLink method as mentioned at wso2/product-is#21025 (comment).

Behavior After Fix:
Sample Request:

curl --location 'https://localhost:9443/api/server/v1/organizations/discovery?offset=10&limit=0' \
--header 'Accept: application/json' \
--header 'Authorization: Basic YWRtaW46YWRtaW4='

Sample Response

{
    "totalResults": 10,
    "startIndex": 11,
    "count": 0,
    "links": [
        {
            "href": "/api/server/v1/organizations/discovery?offset=10&limit=0",
            "rel": "previous"
        }
    ]
}

Related PRs

wso2/product-is#21263

@CLAassistant
Copy link

CLAassistant commented Oct 4, 2024

CLA assistant check
All committers have signed the CLA.

@AnuradhaSK
Copy link
Contributor

Hi @kumardeepak5

Thank you for your contribution!

Here, in this case the expected behavior is not to have any previous or next links when the limit = 0. There is no point of giving any previous/next result set accessing URL with offset is 0.

We want to make this API behavior more or less similar to GET /scim2/Users?startIndex={index}&count={count}

Therefore, for this case expected behavior should be:

{
    "totalResults": 0,
    "startIndex": 11,
    "count": 0,
    "links": [
     
    ]
}

Would you be able to fix in that way?

@kumardeepak5
Copy link
Contributor Author

Hi @AnuradhaSK

Therefore, for this case expected behavior should be:

{
    "totalResults": 0,
    "startIndex": 11,
    "count": 0,
    "links": [
     
    ]
}

Would you be able to fix in that way?

I have a question about the sample response above: Should the expected value for "totalResults" always be 0 when limit=0, or should it reflect the actual total results? I believe it should reflect the actual total results; could you please confirm?

@kumardeepak5
Copy link
Contributor Author

Hi @AnuradhaSK

Therefore, for this case expected behavior should be:

{
    "totalResults": 0,
    "startIndex": 11,
    "count": 0,
    "links": [
     
    ]
}

Would you be able to fix in that way?

The requested changes implemented and below are sample request and response examples after the modifications.

curl --location 'https://localhost:9443/api/server/v1/organizations/discovery?offset=5&limit=0' \
--header 'Authorization: Basic <username:password>'

{
    "totalResults": 5,
    "startIndex": 6,
    "count": 0,
    "links": []
}

Please Review.

@kumardeepak5
Copy link
Contributor Author

Hi @AnuradhaSK just a gentle reminder to review the changes.

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.

Internal Server Error When Using Offset >= Total Organizations and Limit = 0 in Organization Discovery GET API
3 participants