Skip to content

chore: updates UserInfo#16

Closed
lub0s wants to merge 2 commits intoonelogin:developfrom
lub0s:chore/annotate-userinfo
Closed

chore: updates UserInfo#16
lub0s wants to merge 2 commits intoonelogin:developfrom
lub0s:chore/annotate-userinfo

Conversation

@lub0s
Copy link
Copy Markdown

@lub0s lub0s commented Dec 14, 2022

chore: adds gson annotation to UserInfo properties
chore: changes updatedAt type to Long

@Subterrane Subterrane requested a review from Copilot January 7, 2026 21:29
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the UserInfo data class to improve JSON serialization handling and correct the data type for the updatedAt field. The changes add explicit Gson serialization annotations to all properties and change updatedAt from String? to Long? to properly represent Unix timestamps.

Key changes:

  • Added @SerializedName annotations to all UserInfo properties for explicit JSON field mapping
  • Changed updatedAt type from String? to Long? to store Unix timestamps
  • Updated UI code in both Java and Kotlin demo apps to format the timestamp for display

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
oneloginoidc/src/main/java/com/onelogin/oidc/userInfo/UserInfo.kt Added Gson annotations and changed updatedAt to Long?
appjava/src/main/java/com/onelogin/oidc/appjava/UserInfoFragment.java Updated to format updatedAt timestamp using SimpleDateFormat
app/src/main/java/com/onelogin/oidc/demo/UserInfoFragment.kt Updated to format updatedAt timestamp using SimpleDateFormat

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

private TextView preferredName;
private TextView updatedAt;
private ViewAnimator animator;
private final SimpleDateFormat format = new SimpleDateFormat("EEE, MMM d, ''yy", Locale.getDefault());
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

SimpleDateFormat is not thread-safe and should not be stored as an instance field. If this fragment is accessed concurrently or reused, this could cause issues. Consider using ThreadLocal or DateTimeFormatter (API 26+) instead.

Copilot uses AI. Check for mistakes.
private val preferredName: TextView by lazy { requireView().findViewById<TextView>(R.id.username) }
private val updatedAt: TextView by lazy { requireView().findViewById<TextView>(R.id.updated_at) }
private val animator: ViewAnimator by lazy { requireView() as ViewAnimator }
private val format = SimpleDateFormat("EEE, MMM d, ''yy", Locale.getDefault())
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

SimpleDateFormat is not thread-safe. While Kotlin's lazy initialization helps, if this fragment is accessed by multiple threads, thread-safety issues could occur. Consider using DateTimeFormatter or create a new SimpleDateFormat instance when needed.

Copilot uses AI. Check for mistakes.
Subterrane added a commit that referenced this pull request Mar 17, 2026
UserInfo fields with snake_case JSON keys (preferred_username, given_name,
family_name, updated_at) were silently deserializing as null because Gson
performs exact name matching against the camelCase Kotlin property names.

Also changes updatedAt from String? to Long? to match the OIDC spec
(updated_at is a Unix epoch timestamp), adds ProGuard consumer rules to
prevent R8 from stripping Gson model fields, and adds deserialization tests.

Supersedes #6, #13, #16. [no-ado]
@Subterrane
Copy link
Copy Markdown
Collaborator

Thanks — this is the most complete of the three UserInfo PRs. The @SerializedName annotations and Long? type for updatedAt are both correct.

Minor issues in the demo fragment updates: SimpleDateFormat is stored as a non-thread-safe instance field, and @SuppressLint("SetTextI18n") suppresses rather than fixes the lint warning.

I've opened PR #23 which takes the core library fixes from this PR (annotations + type change) and adds ProGuard consumer rules and deserialization tests. The demo fragment timestamp formatting could be a follow-up.

@Subterrane
Copy link
Copy Markdown
Collaborator

#23

@Subterrane Subterrane closed this Mar 17, 2026
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.

5 participants