Vendor JSON #1554

denrase · 2021-06-23T16:25:24Z

📜 Description

This PR will be the baseline where further PRs will be merged into. Therefore it will stay in draft for a longer period of time.

Vendor

Add gson JSONReader and JSONWriter with needed files in vendor/gson folder
- Document tag, commit hast and changes of added files
- Add license in vendor/gson
- Exclude vendor files from usual checks so we can keep the source/tests the same.
  - Easier to see updates when importing new files and also no risk of different behaviour.

Serialization

Adds JsonSerializable that models can implement to write json to a stream.
Adds JSONReader subclass JsonObjectWriter
- Exposes method to call JsonObjectSerializer
  - Writes primitives, objects and serializable objects to the json stream.
  - Supports: null, boolean, int, long, double, String, Map, Collection, Array, JsonSerializable
  - Unknown objects are serialized with a string placeholder until we support reflection.
The JsonSerializable interface provides access to a unknown map which is serialized using JsonObjectSerializer
Exposes JsonSerializable as public API.

Deserialization

Adds JsonDeserializer that models can implement to read json to a stream.
Adds JSONWriter subclass JsonObjectReader.
- Exposes method to call JsonObjectDeserializer.
  - Reads from the json stream and converts it to Java primitives & objects.
  - Supports: null, boolean, int, long, double, String, Map, List
The JsonDeserializer will take read all unknown fields and set then on the the deserialized object.

Example

UserFeedback implements serialization/deserialization using the new classes to illustrate the usage.

💡 Motivation and Context

Relates to #1537
Resolves #1733
Resolves #1497

Evaluate approach and plan further steps.

💚 How did you test it?

📝 Checklist

I reviewed the submitted code
I added tests to verify the changes
I updated the docs if needed
No breaking changes

🔮 Next steps

This PR will stay open until everything is implemented.

codecov-commenter · 2021-06-24T12:52:20Z

Codecov Report

❗ No coverage uploaded for pull request base (6.x.x@c2a4506). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 6f0e0c5 differs from pull request most recent head 4ff9606. Consider uploading reports for the commit 4ff9606 to get more accurate results

@@           Coverage Diff            @@
##             6.x.x    #1554   +/-   ##
========================================
  Coverage         ?   80.91%           
  Complexity       ?     2815           
========================================
  Files            ?      204           
  Lines            ?    10340           
  Branches         ?     1367           
========================================
  Hits             ?     8367           
  Misses           ?     1491           
  Partials         ?      482

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2a4506...4ff9606. Read the comment docs.

bruno-garcia

Would be nice to add a README.md to vendor/gson with a link to where we got it (their github I assume? but more importantly, the version (and commit sha), so we can easily diff/check if we need to update things.

~~Alternatively just add a comment to the top of the class:~~


https://github.com/google/gson/blob/f319c1b8e5ac1135ab253513f91d5ece6719cdf7/gson/src/main/java/com/google/gson/stream/JsonWriter.java

I see you've added the info to the file already under the header.

sentry/src/main/java/io/sentry/vendor/gson/stream/JsonScope.java

# Conflicts: # sentry/api/sentry.api # sentry/src/main/java/io/sentry/SentryEnvelopeHeader.java # sentry/src/main/java/io/sentry/protocol/DebugImage.java # sentry/src/main/java/io/sentry/protocol/MeasurementValue.java # sentry/src/main/java/io/sentry/protocol/SentryTransaction.java

denrase · 2021-10-08T13:10:19Z

@marandaneto @maciejwalkowiak Think we are ready for an alpha release, i updated the changelog with the largest changes that this PR introduces. I did not merge in the main branch yet, as @marandaneto mentioned that we might point this against the 6.x.x branch.

marandaneto · 2021-10-11T13:48:54Z

@marandaneto @maciejwalkowiak Think we are ready for an alpha release, i updated the changelog with the largest changes that this PR introduces. I did not merge in the main branch yet, as @marandaneto mentioned that we might point this against the 6.x.x branch.

@denrase I've targeted to the new branch, mind resolving the conflicts? thanks

# Conflicts: # CHANGELOG.md # sentry/api/sentry.api # sentry/src/test/java/io/sentry/SentryOptionsTest.kt

marandaneto · 2021-10-19T12:05:32Z

Keep track of items for migration docs and changelog

looks like this is done or is there something missing?

denrase · 2021-10-27T09:44:54Z

@marandaneto Don't remember what the original intent was. I did add some sentences in the changelog to describe the overall impact of the PR.

marandaneto · 2021-10-27T13:14:11Z

@marandaneto Don't remember what the original intent was. I did add some sentences in the changelog to describe the overall impact of the PR.

just be sure that the changelog is up to date with all breaking changes.

See:

Serialization of objects with refection is currently not supported.

which is not the case anymore right.
Also, there are conflicting files now.

# Conflicts: # buildSrc/src/main/java/Config.kt # sentry/api/sentry.api # sentry/src/test/java/io/sentry/JsonSerializerTest.kt # sentry/src/test/java/io/sentry/SentryClientTest.kt

denrase · 2021-10-27T14:18:04Z

@marandaneto Merged in the current 6.x.x state and also updated the readme to reflect the reflection (🥁) state.

marandaneto

LGTM
thanks @denrase

bruno-garcia · 2021-11-02T16:09:06Z

I'll do the honours then :D

denrase added 5 commits June 23, 2021 16:37

add json writer from gson

7b419cd

ignore nullaway warnings

4dd911b

add JsonSerializable and implement serialization for UserFeedback

ca1d203

play around with JsonDeserializable stati method protocol

10a5182

Add gsjon JsonReader and implement UserFeedback deserialization

325447d

denrase self-assigned this Jun 23, 2021

denrase added 20 commits June 24, 2021 10:33

change deserializer to inner static class

a1e98ec

change naming, change input of deserializer to jsonwriter

80cb717

add extension to access value or null

5a7cd26

change kotlin extension to java util

603f3ad

reoerder

d9bfd27

fix error message

f82d03e

introduce json keys, mark classes as @ApiStatus.Internal

b2e01ad

fix whitespace

c45f7d0

move gson files to vendor folder

8b77592

exclude vendor files from spotless check

eeefa4d

run spotless apply

17a3581

add comments with source, tag, commit hash and changes

a2a83ca

updste comment position

9b0319d

nmark vendor classes as api internal

88e5035

run api dump

5a1e3ac

add missing tests for json reader utils

ba9c278

add gson license

85330df

use json key

c48ac24

move gson apache license to correct folder

5cd5729

runn spotlessApply

b86673c

bruno-garcia reviewed Jun 25, 2021

View reviewed changes

sentry/src/main/java/io/sentry/vendor/gson/stream/JsonScope.java Show resolved Hide resolved

inject logger into serializable/deserializer

086c6b5

denrase added 5 commits August 30, 2021 15:31

Vendor JSON: DebugMeta (#1685)

35923ff

Vendor JSON: SentryThread (#1686)

9472dfb

Vendor Json: SentryBaseEvent Models (#1688)

69b05da

Vendor JSON: TraceState (#1736)

ac2f814

denrase changed the title ~~Vendor JSON Writer/Reader~~ Vendor JSON Oct 4, 2021

denrase added 3 commits October 8, 2021 14:15

Vendor JSON: Json Serializer (#1744)

923d79f

Remove IUnknownPropertiesConsumer (#1749)

4bcb126

Update changelog

dfda0e4

marandaneto changed the base branch from main to 6.x.x October 11, 2021 13:48

Merge branch '6.x.x' into feat/vendor-json-writer-reader

8c33c98

# Conflicts: # CHANGELOG.md # sentry/api/sentry.api # sentry/src/test/java/io/sentry/SentryOptionsTest.kt

Vendor JSON: Serialize unknown objects using reflection (#1764)

fa0127d

denrase marked this pull request as ready for review October 27, 2021 09:45

Merge branch '6.x.x' into feat/vendor-json-writer-reader

0cbbc8f

# Conflicts: # buildSrc/src/main/java/Config.kt # sentry/api/sentry.api # sentry/src/test/java/io/sentry/JsonSerializerTest.kt # sentry/src/test/java/io/sentry/SentryClientTest.kt

denrase requested a review from romtsn as a code owner October 27, 2021 13:41

denrase added 2 commits October 27, 2021 15:56

expect int number in unit test

6f0e0c5

update changelog

4ff9606

marandaneto approved these changes Oct 29, 2021

View reviewed changes

bruno-garcia approved these changes Nov 2, 2021

View reviewed changes

bruno-garcia merged commit ad944b4 into 6.x.x Nov 2, 2021

bruno-garcia deleted the feat/vendor-json-writer-reader branch November 2, 2021 18:00

This was referenced Nov 8, 2021

Remove or fix IUnknownPropertiesConsumer implementation #1497

Closed

Vendor JsonWriter/Reader and drop Gson dependency #1537

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vendor JSON #1554

Vendor JSON #1554

denrase commented Jun 23, 2021 •

edited

Loading

codecov-commenter commented Jun 24, 2021 •

edited

Loading

bruno-garcia left a comment •

edited

Loading

denrase commented Oct 8, 2021 •

edited

Loading

marandaneto commented Oct 11, 2021

marandaneto commented Oct 19, 2021

denrase commented Oct 27, 2021

marandaneto commented Oct 27, 2021

denrase commented Oct 27, 2021

marandaneto left a comment

bruno-garcia commented Nov 2, 2021

Vendor JSON #1554

Vendor JSON #1554

Conversation

denrase commented Jun 23, 2021 • edited Loading

📜 Description

Vendor

Serialization

Deserialization

Example

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

codecov-commenter commented Jun 24, 2021 • edited Loading

Codecov Report

bruno-garcia left a comment • edited Loading

Choose a reason for hiding this comment

denrase commented Oct 8, 2021 • edited Loading

marandaneto commented Oct 11, 2021

marandaneto commented Oct 19, 2021

denrase commented Oct 27, 2021

marandaneto commented Oct 27, 2021

denrase commented Oct 27, 2021

marandaneto left a comment

Choose a reason for hiding this comment

bruno-garcia commented Nov 2, 2021

denrase commented Jun 23, 2021 •

edited

Loading

codecov-commenter commented Jun 24, 2021 •

edited

Loading

bruno-garcia left a comment •

edited

Loading

denrase commented Oct 8, 2021 •

edited

Loading