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

feat: add remnant events migration plugin #40

Merged
merged 33 commits into from
May 26, 2023

Conversation

liuyang1520
Copy link
Collaborator

@liuyang1520 liuyang1520 commented Jun 24, 2022

Summary

This PR adds a remnant events migration plugin to send the remnant events in Sqlite generated by old Android Java SDK, including:

  • DB utils for retrieving the entries in sqlite database
  • Covert the event to new format
  • Send the event with this new Kotlin SDK

Todos:

Checklist

  • Does your PR title have the correct title format?
    yes
  • Does your PR have a breaking change?:
    no

@secure-code-warrior-for-github

Micro-Learning Topic: SQL injection (Detected by phrase)

Matched on "Sqli"

What is this? (2min video)

This is probably one of the two most exploited vulnerabilities in web applications and has led to a number of high profile company breaches. It occurs when an application fails to sanitize or validate input before using it to dynamically construct a statement. An attacker that exploits this vulnerability will be able to gain access to the underlying database and view or modify data without permission.

Try this challenge in Secure Code Warrior

@liuyang1520 liuyang1520 changed the title [WIP] feat: add remnant events migration plugin feat: add remnant events migration plugin Jun 24, 2022
@liuyang1520 liuyang1520 marked this pull request as ready for review June 27, 2022 08:32
@falconandy
Copy link
Contributor

falconandy commented May 18, 2023

  1. Migration logic is changed to write legacy data directly to new storages.
  2. Migration logic is moved from Plugin to let run it before any plugins including standard plugin (AndroidContextPlugin, etc). I called it Initializer but maybe simple configuration flag like runLegacyDataMigration is enough. Reason: device/user id logic is pretty complex (classes State, IdentityContainer, IdentityListener, etc) and asynchronous - AndroidContextPlugin concurrently sets random device id on first run if device id is currently empty - sometimes the random value wins, sometimes - value from legacy data wins (if migration logic is implemented in a plugin executed after AndroidContextPlugin).
  3. Supported: on first run since upgrade (recommended) - device/user id, session id, last event date/id, events, identifies, intercepted identifies. If migration was enabled later after upgrade - only events are migrated.
  4. Supported sqlite db versions - v3 (2015 year) and current v4 (2/9/23, added table identify_interceptor)
  5. Added support for instance names.
  6. Some legacy fields are converted to new names/format: library -> from object to string, timestamp -> time, adid, android_app_set_id.
  7. Added methods getUserId()/ getDeviceId to android Amplitude
  8. After migration legacy sqlite data are cleaned except deviceId/userId - maybe they will be required later to link legacy/new ids, for example.
  9. Added tests for v3/v4 sqlite databases - currently cover on first run since upgrade case

@justin-fiedler

Can you please add so extra tests around deviceId to verify that deviceId generated once and is persisted across multiple sessions correctly.

Not added yet.

@falconandy falconandy requested a review from qingzhuozhen May 18, 2023 18:06
@falconandy falconandy requested a review from justin-fiedler May 22, 2023 18:33
@falconandy falconandy requested a review from justin-fiedler May 23, 2023 11:34
@falconandy falconandy requested a review from yuhao900914 May 24, 2023 08:13
@falconandy falconandy requested a review from justin-fiedler May 24, 2023 18:20
@falconandy falconandy requested a review from justin-fiedler May 26, 2023 07:22
Copy link
Contributor

@justin-fiedler justin-fiedler left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks @falconandy!

@justin-fiedler justin-fiedler merged commit 6225f5d into main May 26, 2023
@justin-fiedler justin-fiedler deleted the add-remnant-events-migration-plugin branch May 26, 2023 17:04
github-actions bot pushed a commit that referenced this pull request May 26, 2023
# [1.9.0](v1.8.2...v1.9.0) (2023-05-26)

### Bug Fixes

* event options should override event properties, refresh last event time on exit foreground ([#122](#122)) ([652ea42](652ea42))

### Features

* add remnant events migration plugin ([#40](#40)) ([6225f5d](6225f5d))
@github-actions
Copy link

🎉 This PR is included in version 1.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

5 participants