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

Add Json.decodeOneElement(bufferedSource: BufferedSource) #2854

Open
martinbonnin opened this issue Nov 9, 2024 · 5 comments
Open

Add Json.decodeOneElement(bufferedSource: BufferedSource) #2854

martinbonnin opened this issue Nov 9, 2024 · 5 comments
Labels

Comments

@martinbonnin
Copy link
Contributor

Hi 👋

This is a follow up from #1789.

There is now decodeBufferedSourceToSequence() but it doesn't work for arbitrary data:

{ "foo": "bar" }

some important non-json data

Can there be decodeOneElement() that leaves the bufferedSource open just after the parsed JsonElement?

@sandwwraith
Copy link
Member

It looks strange to me that there are two different types of data in the same stream. Do you have any real-world examples of such format?

@JakeWharton
Copy link
Contributor

Wait shouldn't the normal decodeFromBufferedSource do this?

Us three discussed this when the Okio integration was added: #1901 (comment)

@martinbonnin
Copy link
Contributor Author

martinbonnin commented Dec 5, 2024

@JakeWharton that would work for me. I was kind of put towards decodeOneElement() because there is already decodeBufferedSourceToSequence() but I'd be happy if the regular decodeFromBufferedSource() just decoded a single element and the rest would be up to the caller (reading other elements or reading other, non-JSON data, or asserting EOF).

@sandwwraith the specific case that made me open this issue is some old personal blog post format that I used 10 years ago where for some reason I used a JSON object instead of a front matter header 😄 . I wouldn't say it qualifies as very widespread but still a use case?

@sandwwraith
Copy link
Member

@JakeWharton We discussed the usage of buffered sources there, and decodeFromBufferedSource indeed uses BufferedSource as an input. Yet, for uniformity with decodeFromString/decodeFromString, it still expects input to be fully consumed and ending with EOF — although there are no technical limitations to leave something in the source.

@JakeWharton
Copy link
Contributor

Okay. I guess I never really followed up on the actual behavior on that original PR.

If the library is going to consume the entire source, there's really no point in accepting a BufferedSource vs. a regular Source. The reason you accept a BufferedSource is to move that control to the caller to allow prefix and suffix data. That's really the only reason that I advocated for it in that original PR.

Moshi and Wire accept a BufferedSource because they don't know whether you are consuming part of a stream or all of a stream. You can do line-delimited JSON or length-delimited proto with those libraries using the same entrypoint as when reading a single JSON value or single proto message in a "normal" HTTP response, file, etc. This is in contrast to things like OkHttp and Retrofit that can use Moshi and Wire, but which are fundamentally document-based and apply enforcement of the start and end of things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants