Skip to content

Conversation

binayshaw7777
Copy link

This PR replaces the Gson library with kotlinx.serialization for JSON handling throughout the application.
This change affects network data sources, preference data storage, RSS provider APIs, and security key encoding/decoding.

This also resolves Issue: 994

@@ -15,7 +15,7 @@ class GoogleReaderSecurityKey private constructor() : SecurityKey() {
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the requested changes. I followed the 1st approach, i.e. going with sealded classes instead of abstract classes. Also marking Serializable to it's subclasses. Now one step more, I also added custom serial name to their subclasses.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure if @Serializable is needed on the base class, and I don't think a sealed class is necessary. I propose creating some fake objects and the corresponding JSON produced by previous Gson implementation to compare against in unit tests.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand. I actually followed this rule: https://github.com/Kotlin/kotlinx.serialization/blob/master/docs/polymorphism.md#concrete-properties-in-a-base-class

Moreover, I can you please help me with how do I reproduce this to test it out? I mean the JSON response before and after the implementation. Do we need to write testcases for it? If yes can you help me with the dummy data which is valid for GSON?

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.

2 participants