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.

Sign up for GitHub

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

Jump to bottom

Migrate from id to guid #319

Open
11 tasks done
johnjohndoe opened this issue Sep 19, 2020 · 10 comments · May be fixed by #337 or #350
Open
11 tasks done

Migrate from id to guid #319

johnjohndoe opened this issue Sep 19, 2020 · 10 comments · May be fixed by #337 or #350
Assignees
@johnjohndoe
Labels
Feature request A request for a new feature. Help wanted You are invited to help. Please annouce it as a comment on the issue.

Comments

@johnjohndoe
Copy link
Member

johnjohndoe commented Sep 19, 2020 •
edited
Loading

Current state

  • The id XML attribute is parsed from the schedule XML and used with the app.
    • Used to compose PendingIntent in AlarmServices.
    • Used to compose PendingIntent in AlarmReceiver.
    • Rendered in session details screen.
    • Used to persist and query data in sessions, alerts and highlights database tables.
  • id values are not guaranteed to be unique - id values used for different sessions cause errors. See Duplicate event ids (36C3, GPN22) voc/schedule#63.
  • New session data is compared and modified in the ScheduleChanges class.
  • New session data is persisted by first deleting all old session data and then inserting all new session data in one transaction. See SessionsDatabaseRepository#insert.
  • The sessions table does mark any of its columns as primary key nor use an auto-increment key.

Target state

  • The guid XML attribute is parsed from the schedule XML and used with the app.
    • Used to persist and query data in sessions, alerts and highlights database tables.

Preparation

  • Stop parsing sessionId as an Integer in AlarmServices to compose a PendingIntent.
  • Resolve sessionId being used as an Integer in AlarmReceiver#onReceive to compose a PendingIntent.
  • Migrate Highlight#sessionId from Int to String. See HighlightDBOpenHelper.

TODOs

  • Parse guid XML attribute.
  • Use guid within the app.
  • Check if id XML attribute can be removed. -> Removed.
  • Revise SessionUrlComposer and related tests.
  • Update sample data in tests - use typical guid sample data.
  • Display guid value? -> No value for the user.
  • Clarify if data can be kept on application update. -> Data will be wiped on application update.
  • Clarify if Chaosflix export will be broken when moving from id to guid.

Related
@johnjohndoe johnjohndoe added the Feature request A request for a new feature. label Sep 19, 2020
@johnjohndoe johnjohndoe mentioned this issue Sep 19, 2020
Open
johnjohndoe referenced this issue Oct 28, 2020
show alarms as bell icon, delete alarm from event context menu
7e4c3f6
johnjohndoe referenced this issue Oct 28, 2020
Initial version of Camp Fahrplan
e4104ab
@johnjohndoe johnjohndoe self-assigned this Oct 30, 2020
johnjohndoe added a commit that referenced this issue Oct 31, 2020
@johnjohndoe
Use unique session IDs - DROP and recreate all database tables.
57e70f6 
!!! ATTENTION !!! This commit wipes all database data!

+ This change is necessary because IDs will no longer be based on numeric
  characters in the future. Instead a UUID, containing alpha-numeric
  characters will be used.
+ This replaces the Session#sessionId value with the value of the "guid"
  attribute parsed from "event" node. The formerly used "id" attribute
  turned out to hold identical values for different sessions.
  Resolves #319.
+ Fix column type of "session_id" in alarms and highlight tables from
  INTEGER to TEXT to allow storing "guid" values.
+ Increase database versions.
+ Update sample data in tests.

Additional changes - The following changes are committed here to subsume
them as an atomic database schema change.

+ Rename sessions database and table to reflect current project wording
  (lectures -> sessions). See issue #250.
+ Rename database column in the alarms, highlight and sessions tables to
  reflect the current project naming (event -> session).
+ A database index is created for the sessions table to improve query
  performance.
+ Database column types in the alarms and session tables are corrected.
  This resolves issues where leading zeros are lost on this columns.
  Resolves #5.
@johnjohndoe johnjohndoe linked a pull request Oct 31, 2020 that will close this issue
Draft
6 tasks
johnjohndoe added a commit that referenced this issue Nov 3, 2020
@johnjohndoe
Use unique session IDs - DROP and recreate all database tables.
7323e7e 
!!! ATTENTION !!! This commit wipes all database data!

+ This change is necessary because IDs will no longer be based on numeric
  characters in the future. Instead a UUID, containing alpha-numeric
  characters will be used.
+ This replaces the Session#sessionId value with the value of the "guid"
  attribute parsed from "event" node. The formerly used "id" attribute
  turned out to hold identical values for different sessions.
  Resolves #319.
+ Fix column type of "session_id" in alarms and highlight tables from
  INTEGER to TEXT to allow storing "guid" values.
+ Increase database versions.
+ Update sample data in tests.

Additional changes - The following changes are committed here to subsume
them as an atomic database schema change.

+ Rename sessions database and table to reflect current project wording
  (lectures -> sessions). See issue #250.
+ Rename database column in the alarms, highlight and sessions tables to
  reflect the current project naming (event -> session).
+ A database index is created for the sessions table to improve query
  performance.
+ Database column types in the alarms and session tables are corrected.
  This resolves issues where leading zeros are lost on this columns.
  Resolves #5.
@johnjohndoe
Copy link
Member Author

@NiciDieNase Can you tell if the JSON import in the Chaosflix app will be broken when I move from id to guid here?

@cketti
Copy link
Collaborator

cketti commented Nov 4, 2020

I think a better approach would be to use an auto-incrementing INTEGER column to uniquely identify a session in the database. This will keep the app simple in situations where stable numeric identifiers are required or at least very helpful (e.g. in a RecyclerView.Adapter).

This way only the code that maps between network model and app model has to be aware of GUIDs. If the identifier in the network format changes in the future only the mapping code needs to be updated.

Using that approach also allows splitting this change into two parts:

  1. Change app to use its own unique session ID.
  2. Change mapping code to use guid attribute instead of id attribute for matching items.

johnjohndoe added a commit that referenced this issue Nov 8, 2020
@johnjohndoe
Use unique session IDs - DROP and recreate all database tables.
be9c1bf 
!!! ATTENTION !!! This commit wipes all database data!

+ This change is necessary because IDs will no longer be based on numeric
  characters in the future. Instead a UUID, containing alpha-numeric
  characters will be used.
+ This replaces the Session#sessionId value with the value of the "guid"
  attribute parsed from "event" node. The formerly used "id" attribute
  turned out to hold identical values for different sessions.
  Resolves #319.
+ Fix column type of "session_id" in alarms and highlight tables from
  INTEGER to TEXT to allow storing "guid" values.
+ Increase database versions.
+ Update sample data in tests.

Additional changes - The following changes are committed here to subsume
them as an atomic database schema change.

+ Rename sessions database and table to reflect current project wording
  (lectures -> sessions). See issue #250.
+ Rename database column in the alarms, highlight and sessions tables to
  reflect the current project naming (event -> session).
+ A database index is created for the sessions table to improve query
  performance.
+ Database column types in the alarms and session tables are corrected.
  This resolves issues where leading zeros are lost on this columns.
  Resolves #5.
johnjohndoe added a commit that referenced this issue Nov 8, 2020
@johnjohndoe
Use unique session IDs - DROP and recreate all database tables.
3070395 
!!! ATTENTION !!! This commit wipes all database data!

+ This change is necessary because IDs will no longer be based on numeric
  characters in the future. Instead a UUID, containing alpha-numeric
  characters will be used.
+ This replaces the Session#sessionId value with the value of the "guid"
  attribute parsed from "event" node. The formerly used "id" attribute
  turned out to hold identical values for different sessions.
  Resolves #319.
+ Fix column type of "session_id" in alarms and highlight tables from
  INTEGER to TEXT to allow storing "guid" values.
+ Increase database versions.
+ Update sample data in tests.

Additional changes - The following changes are committed here to subsume
them as an atomic database schema change.

+ Rename sessions database and table to reflect current project wording
  (lectures -> sessions). See issue #250.
+ Rename database column in the alarms, highlight and sessions tables to
  reflect the current project naming (event -> session).
+ A database index is created for the sessions table to improve query
  performance.
+ Database column types in the alarms and session tables are corrected.
  This resolves issues where leading zeros are lost on this columns.
  Resolves #5.
@johnjohndoe johnjohndoe pinned this issue Nov 16, 2020
johnjohndoe added a commit that referenced this issue Nov 20, 2020
@johnjohndoe
Use unique session IDs - DROP and recreate all database tables.
97be3db 
!!! ATTENTION !!! This commit wipes all database data!

+ This change is necessary because IDs will no longer be based on numeric
  characters in the future. Instead a UUID, containing alpha-numeric
  characters will be used.
+ This replaces the Session#sessionId value with the value of the "guid"
  attribute parsed from "event" node. The formerly used "id" attribute
  turned out to hold identical values for different sessions.
  Resolves #319.
+ Fix column type of "session_id" in alarms and highlight tables from
  INTEGER to TEXT to allow storing "guid" values.
+ Increase database versions.
+ Update sample data in tests.

Additional changes - The following changes are committed here to subsume
them as an atomic database schema change.

+ Rename sessions database and table to reflect current project wording
  (lectures -> sessions). See issue #250.
+ Rename database column in the alarms, highlight and sessions tables to
  reflect the current project naming (event -> session).
+ A database index is created for the sessions table to improve query
  performance.
+ Database column types in the alarms and session tables are corrected.
  This resolves issues where leading zeros are lost on this columns.
  Resolves #5.
johnjohndoe added a commit that referenced this issue Nov 21, 2020
@johnjohndoe
Use unique session IDs - DROP and recreate all database tables.
05acd3d 
!!! ATTENTION !!! This commit wipes all database data!

+ This change is necessary because IDs will no longer be based on numeric
  characters in the future. Instead a UUID, containing alpha-numeric
  characters will be used.
+ This replaces the Session#sessionId value with the value of the "guid"
  attribute parsed from "event" node. The formerly used "id" attribute
  turned out to hold identical values for different sessions.
  Resolves #319.
+ Fix column type of "session_id" in alarms and highlight tables from
  INTEGER to TEXT to allow storing "guid" values.
+ Increase database versions.
+ Update sample data in tests.

Additional changes - The following changes are committed here to subsume
them as an atomic database schema change.

+ Rename sessions database and table to reflect current project wording
  (lectures -> sessions). See issue #250.
+ Rename database column in the alarms, highlight and sessions tables to
  reflect the current project naming (event -> session).
+ A database index is created for the sessions table to improve query
  performance.
+ Database column types in the alarms and session tables are corrected.
  This resolves issues where leading zeros are lost on this columns.
  Resolves #5.
johnjohndoe added a commit that referenced this issue Nov 22, 2020
@johnjohndoe
Use unique session IDs - DROP and recreate all database tables.
c4dd2b3 
!!! ATTENTION !!! This commit wipes all database data!

+ This change is necessary because IDs will no longer be based on numeric
  characters in the future. Instead a UUID, containing alpha-numeric
  characters will be used.
+ This replaces the Session#sessionId value with the value of the "guid"
  attribute parsed from "event" node. The formerly used "id" attribute
  turned out to hold identical values for different sessions.
  Resolves #319.
+ Fix column type of "session_id" in alarms and highlight tables from
  INTEGER to TEXT to allow storing "guid" values.
+ Increase database versions.
+ Update sample data in tests.

Additional changes - The following changes are committed here to subsume
them as an atomic database schema change.

+ Rename sessions database and table to reflect current project wording
  (lectures -> sessions). See issue #250.
+ Rename database column in the alarms, highlight and sessions tables to
  reflect the current project naming (event -> session).
+ A database index is created for the sessions table to improve query
  performance.
+ Database column types in the alarms and session tables are corrected.
  This resolves issues where leading zeros are lost on this columns.
  Resolves #5.
johnjohndoe added a commit that referenced this issue Dec 10, 2020
@johnjohndoe
Use unique session IDs - DROP and recreate all database tables.
266e220 
!!! ATTENTION !!! This commit wipes all database data!

+ This change is necessary because IDs will no longer be based on numeric
  characters in the future. Instead a UUID, containing alpha-numeric
  characters will be used.
+ This replaces the Session#sessionId value with the value of the "guid"
  attribute parsed from "event" node. The formerly used "id" attribute
  turned out to hold identical values for different sessions.
  Resolves #319.
+ Fix column type of "session_id" in alarms and highlight tables from
  INTEGER to TEXT to allow storing "guid" values.
+ Increase database versions.
+ Update sample data in tests.

Additional changes - The following changes are committed here to subsume
them as an atomic database schema change.

+ Rename sessions database and table to reflect current project wording
  (lectures -> sessions). See issue #250.
+ Rename database column in the alarms, highlight and sessions tables to
  reflect the current project naming (event -> session).
+ A database index is created for the sessions table to improve query
  performance.
+ Database column types in the alarms and session tables are corrected.
  This resolves issues where leading zeros are lost on this columns.
  Resolves #5.
@johnjohndoe
Copy link
Member Author

🧑‍🚒 This has become top priority because the merged schedule data for the upcoming rc3 will not contain id attributes for all XML event nodes - but guid attributes!

@johnjohndoe johnjohndoe added the Help wanted You are invited to help. Please annouce it as a comment on the issue. label Dec 11, 2020
@johnjohndoe
Copy link
Member Author

johnjohndoe commented Dec 12, 2020 •
edited
Loading

I extended the information in the "current state" section 👆 to clarify the currently used mechanism.

@NiciDieNase
Copy link
Contributor

@NiciDieNase Can you tell if the JSON import in the Chaosflix app will be broken when I move from id to guid here?

No, should not be affected, chaoflix uses guid for importing.

@johnjohndoe
Copy link
Member Author

@cketti The work-in-progress of a new implementation can be found on the id-guid-migration-v2 branch.

This was referenced Dec 17, 2020
Merged
Draft
@johnjohndoe
Copy link
Member Author

johnjohndoe commented Dec 24, 2020 •
edited
Loading

🔈 Update 24.12.2020

  • The aggregated schedule for the rc3 event has been adapted.
  • id values are derived from guid values to support the usage of id in client applications.
  • The id is generated from the first six numbers found in the guid string.
  • I will test the schedule data and probably postpone the migration because there are still unresolved issues in PR Migrate from "id" to "guid" #350.

@johnjohndoe johnjohndoe unpinned this issue Dec 26, 2020
@johnjohndoe johnjohndoe pinned this issue Apr 17, 2021
@johnjohndoe johnjohndoe mentioned this issue Jul 31, 2021
Merged
@saerdnaer
Copy link

@johnjohndoe Whats the current state of this issue?

@johnjohndoe
Copy link
Member Author

@saerdnaer Paused. I will pick up work on the topic again in October 2021.

@johnjohndoe
Copy link
Member Author

@saerdnaer I haven't been able to continue here. I likely have to stick to IDs for rC3.rev2.

@johnjohndoe johnjohndoe unpinned this issue Dec 23, 2021
@NerdyProjects NerdyProjects mentioned this issue Aug 14, 2022
Open
@johnjohndoe johnjohndoe pinned this issue Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@johnjohndoe johnjohndoe

Labels
Feature request A request for a new feature. Help wanted You are invited to help. Please annouce it as a comment on the issue.
Milestone
No milestone
4 participants
@saerdnaer @johnjohndoe @cketti @NiciDieNase