Previously working APIs now return invalid JSON bodies

[marchof]

For example:

• https://api.foojay.io/disco/v1.0/packages?version=6&distro=oracle
• https://api.foojay.io/disco/v1.0/packages?version=10&distro=sap_machine

[geertjanw]

Probably wait a little bit, a new version has just been released.

[HanSolo]

We had to make some changes. Could you try to add &latest=overall to the url?

[marchof]

@geertjanw Are you sure time will heal invalid JSON syntax? 🤔

[geertjanw]

Can you try Gerrit's suggestion above?

[marchof]

@HanSolo the extra parameter &latest=overall fixes some results. But in case there are no results the returned JSON has a syntax error (misplaces colon):

https://api.foojay.io/disco/v1.0/packages?version=8&distro=aojup&latest=overall

{
  "info":"No package found",

}

[HanSolo]

Ok, will take a look at it, thx for the heads up 👍🏻

[marchof]

Another remark: If my query does not result in a hit, I would expect an empty result set from the API. Not an object with a test message. The new behavior would force me to implement if/else for the API calls to handle the no-result case separately.

Any ETA when this case can be fixed? Currently my update jobs and the download page on javaalmanac.io is broken.

[HanSolo]

That was discussed in another round where more than one person decided it would be better to get at least a message that nothing was found instead of returning an empty array. I think we need to discuss that internally once more.

[marchof]

Please look at this from a client side. It really forces every client to introduce ugly type checks.

Alternatively you can have a message along the empty result (introducing a result wrapper object consisting of result an meta data).

[marchof]

And in any case it should be parseable Json...

[HanSolo]

It was suggested by clients, so no internal discussion but with clients

[marchof]

Ok...

[HanSolo]

And yes it should be parseable, that's right :)

[marchof]

@HanSolo Looks like the syntax is fixed.

So now what is the official contract to correctly process the empty result case? Should clients test for the string value of the info attribute?

{
    "info":"No package found"
}

Should clients test for the data type (array vs. object)? Maybe someone who was involved in the discussion about the new API can enlighten us here :)

[geertjanw]

Would you like to be involved, @marchof? We could set up a meeting where you and others who want to discuss the Disco API and provide input into it could get together to discuss?

[marchof]

Sure, at least from my javaalmanac.io perspective I could give feedback. Not sure whether meetings are the right format in the long run. Typically API changes are best tracked via issues and pull-requests.

[geertjanw]

You are welcome to provide issues and pull requests any time and they'll be warmly welcomed. :-)

[marchof]

Well, I meant the other way round: If there are API changes there should be a public discussion here on the tracker and a PR so interested parties can review and test before it is deployed.

[geertjanw]

Agreed. Also on the Foojay Slack channel, there's now a dedicated channel #disco for these discussions too.

[HanSolo]

Ok so what should we do now? I personal have no preference but I agree we need a working version. So the easiest way would be the one we had in the beginning: If nothing was found the API will return an empty json array "[]". I've just changed it because more than one person asked for having a meaningful response in case nothing was found. If you would ask me personally...an empty result is an empty result...means nothing was found, but if anyone has a different opinion, please let me know.

[marchof]

I would prefer a solution where the schema of the result is fixed and (technical) clients are not forced to create code to distinguish cases. If meta information is required a envelope object would be the best option and also allows addition of additional meta data in future. For example:

{
    "result": []
    "message": "No package found"
}

This should probably then consistently for all APIs (v2.0).

[HanSolo]

Yep that looks like a good approach and should serve all needs that came up so far...

[HanSolo]

I've implemented a first version and deployed it to my own server. I've sent you the coordinates in a direct msg on foojay slack.

[marchof]

@HanSolo Thanks! Looks like foojay slack has been limited to a few domains and I'm not able any more to access it any more. You might send it via Twitter to @marcandsweep

[geertjanw]

https://join.slack.com/t/foojay/shared_invite/zt-ni5afow2-FcF7sMZYBnkng~TXQ781Nw

[marchof]

I had a quick look at the test deplyoment:

• The packages endpoint looks good ✅
• The packages/${packageid} now returns the result object as an list element. It was a single object before. I would somehow expect the same also for the new result envelope. ❓

[HanSolo]

Right it was a single object before but I first implemented it in that way but then thought why not having the same schema for all endpoints. So it will always return a json array for the result and a string for the message. In the case of the packageId endpoint the array only contains just one entry. I've made some tests by converting 3 other projects that use disco to the new response format and found that it's much easier if all responses are in the same format...so what do you think?

[marchof]

Both is ok for me. What is the behavior if the is no package with the given id? Empty array or 404?

[HanSolo]

It will return an empty array and the message will contain something like "pkg not found"

[HanSolo]

Should be fixed now

[marchof]

Confirmed, syntax is correct now. Thanks!