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

Many REST APIs return different response *types* based on result count #107

Open
palenshus opened this issue Nov 24, 2021 · 2 comments
Open
Labels
Issue-Bug Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work.

Comments

@palenshus
Copy link
Contributor

palenshus commented Nov 24, 2021

Brief description of your issue

I just hit upon something in the REST api that I believe we should consider changing at the schema/interface level. Some of the APIs return different result types depending on the count of items returned, for example:

        return packages.Items.Count switch
        {
            0 => new NoContentResult(),
            1 => new ApiObjectResult(new ApiResponse<Package>(packages.Items.First(), packages.ContinuationToken)),
            _ => new ApiObjectResult(new ApiResponse<List<Package>>(packages.Items.ToList(), packages.ContinuationToken)),
        };

Either No Content (null), a single object, or an array. For example:

This returns an array: .../api/packages/Microsoft.PowerToys/versions

{"Data":[{...},{...},,],"ContinuationToken":""}

This returns a single object: .../api/packages/Zwift.Zwift/versions

{"Data":{...}}

This returns 204 (null): .../api/packages/Foo.Bar/versions

If the query returns more than one result, I get an object where Data is an array (and there’s a continuationToken property), but if there’s only one result, then Data is an object. This is true for the packages, packageManifests, locales, installers and versions APIs.

IMO this is a bad practice and requires clients to try to parse the result object twice in a catch block to successfully deserialize it. Instead we should always consistently return the same object type, potentially with only one item in the array, and a null continuationToken.

Steps to reproduce

Call above APIs

Expected behavior

Response type should be consistent regardless of # of results

Actual behavior

Response type varies based on number of results

Environment

N/A
@RDMacLachlan
Copy link
Member

Thank you @palenshus for the feedback, I will bring this topic up in a future status meeting to ensure we address your concern of returning a consistent object type.

@palenshus
Copy link
Contributor Author

Thanks Roy! I'll mention a couple other related bugs here, I think it's fine if the one Issue tracks them too:

This .../api/packages/Microsoft.PowerToys/versions/0.33.1/locales returns 404 and this:

{"ErrorCode": 13,"ErrorMessage": "The selected locale contains no locale information."}

It should return a 200 with an empty data array.

And when there are no matching results, for example:

.../api/packages/Microsoft.PowerToysa

We return 204 (No Content) instead of 404, in violation of this guidance: https://docs.microsoft.com/en-us/azure/architecture/best-practices/api-design#get-methods

A successful GET method typically returns HTTP status code 200 (OK). If the resource cannot be found, the method should return 404 (Not Found).

@denelon denelon added Issue-Bug Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Nov 30, 2021
@denelon denelon added this to the v1.2-REST milestone Dec 3, 2021
@denelon denelon removed this from the v1.2-REST milestone Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work.
Projects
None yet
Development

No branches or pull requests

3 participants