Skip to content

refactor AndroidEvent; introduce AndroidEvent2 with Entity as data object #40

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

Merged
merged 25 commits into from
Jul 21, 2025

Conversation

rfc2822
Copy link
Member

@rfc2822 rfc2822 commented Jul 9, 2025

To be merged before bitfireAT/davx5-ose#1579, for DAVx⁵ 4.5.3

  1. Provide new AndroidEvent2 that
    • uses the low-level Entity as data object,
    • is immutable (fields can only be accessed over getters),
    • should be used for new code.
  2. Provide new AndroidCalendar that has CRUD methods for AndroidEvent2.
  3. Refactor (and deprecate) AndroidEvent:
    • Move add() to the new LegacyAndroidCalendar so that AndroidEvent doesn't need to support unsaved states with id=null.
    • Move getEvent() to the new LegacyAndroidCalendar.
    • No mapping from/to Event anymore.

Other changes:

  • Update README
  • Add/adapt tests, especially for the new AndroidCalendar.

Outlook / steps after this PR:

  • Make AndroidEvent.eTag etc. immutable.
  • Move AndroidEvent.update() to LegacyAndroidCalendar, too.
  • Make AndroidEvent look/behave more and more like AndroidEvent2 so that it can be replaced by AndroidEvent2 at some point.

@rfc2822 rfc2822 self-assigned this Jul 9, 2025
@rfc2822 rfc2822 added the refactoring Quality improvement of existing functions label Jul 9, 2025
@rfc2822 rfc2822 force-pushed the androidevent-entity branch from 2db182e to ead97a8 Compare July 10, 2025 20:40
@rfc2822 rfc2822 force-pushed the androidevent-entity branch from d26e315 to 40f7df7 Compare July 10, 2025 22:20
@rfc2822 rfc2822 force-pushed the androidevent-entity branch from 4f2578e to b58a0f6 Compare July 10, 2025 22:34
@rfc2822 rfc2822 force-pushed the androidevent-entity branch from ea3e843 to 524a3f5 Compare July 10, 2025 22:44
Copy link

@Copilot 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 refactors the Android calendar integration by introducing a new immutable AndroidEvent2 backed by Entity, deprecating the legacy AndroidEvent, and providing a LegacyAndroidCalendar for backward mapping.

  • Introduce AndroidEvent2 and update AndroidCalendar to use Entity everywhere
  • Deprecate and simplify legacy AndroidEvent, move mapping into LegacyAndroidCalendar
  • Update tests and utilities to use the new API

Reviewed Changes

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

Show a summary per file
File Description
lib/src/main/kotlin/at/bitfire/synctools/test/InitCalendarProviderRule.kt Replace legacy AndroidEvent.add() calls with addEvent(Entity)
lib/src/main/kotlin/at/bitfire/synctools/test/AssertHelpers.kt Add assertContentValuesEqual helper for comparing ContentValues
lib/src/main/kotlin/at/bitfire/synctools/storage/calendar/AndroidEvent2.kt New immutable AndroidEvent2 wrapping an Entity
lib/src/main/kotlin/at/bitfire/synctools/storage/calendar/AndroidCalendar.kt Overhaul AndroidCalendar to CRUD Entity and add batch methods
lib/src/main/kotlin/at/bitfire/synctools/storage/BatchOperation.kt Extend batch builder with withValues(ContentValues)
lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/LegacyAndroidEventProcessor.kt Update processor to use AndroidEvent2 and LegacyAndroidCalendar
lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/LegacyAndroidEventBuilder.kt Reference AndroidEvent2 constants when building legacy rows
lib/src/main/kotlin/at/bitfire/ical4android/LegacyAndroidCalendar.kt New LegacyAndroidCalendar for mapping between Event and storage
lib/src/main/kotlin/at/bitfire/ical4android/Event.kt Deprecate legacy Event in favor of the standard ical4j model
lib/src/main/kotlin/at/bitfire/ical4android/AndroidEvent.kt Simplify and deprecate legacy AndroidEvent as a ContentValues wrapper
lib/src/androidTest/kotlin/at/bitfire/synctools/storage/calendar/AndroidCalendarTest.kt Convert tests to use Entity-based API and new calendar methods
lib/src/androidTest/kotlin/at/bitfire/synctools/storage/calendar/AndroidCalendarProviderTest.kt Update provider tests to use LegacyAndroidCalendar.add()
lib/src/androidTest/kotlin/at/bitfire/synctools/storage/calendar/AndroidCalendarProviderBehaviorTest.kt Add behavior test for updateEventRow null-status edge case
lib/src/androidTest/kotlin/at/bitfire/synctools/mapping/calendar/LegacyAndroidEventProcessorTest.kt Rename and adapt processor tests to LegacyAndroidEventProcessor
lib/src/androidTest/kotlin/at/bitfire/synctools/mapping/calendar/LegacyAndroidEventBuilderTest.kt Rename builder tests to use LegacyAndroidCalendar
lib/src/androidTest/kotlin/at/bitfire/ical4android/AndroidEventTest.kt Update AndroidEvent tests to use LegacyAndroidCalendar
README.md Document new package layout and deprecation notes
Comments suppressed due to low confidence (3)

lib/src/main/kotlin/at/bitfire/synctools/storage/calendar/AndroidCalendar.kt:75

  • [nitpick] Public method addEvent lacks KDoc. Add @param entity and @return comments to describe that it returns the newly inserted event ID and possible exceptions.
    fun addEvent(entity: Entity): Long {

lib/src/main/kotlin/at/bitfire/synctools/storage/calendar/AndroidCalendar.kt:1

  • The class KDoc lists @param client but the constructor parameter is provider: AndroidCalendarProvider. Update the @param tag to match the actual parameter name.
/*

README.md:25

  • [nitpick] The markdown sub-list under at.bitfire.synctools is inconsistently indented. Consider using two spaces before each sub-item for proper list nesting.
- `at.bitfire.synctools`: new package where everything shall be refactored into

@rfc2822 rfc2822 changed the title AndroidEvent: use Entity as data object refactor AndroidEvent; introduce AndroidEvent2 with Entity as data object Jul 11, 2025
@rfc2822 rfc2822 force-pushed the androidevent-entity branch from d2dc900 to ee47481 Compare July 12, 2025 09:55
@rfc2822 rfc2822 force-pushed the androidevent-entity branch from ee47481 to 8da9161 Compare July 12, 2025 10:08
@rfc2822 rfc2822 marked this pull request as ready for review July 12, 2025 10:35
@rfc2822 rfc2822 force-pushed the androidevent-entity branch from 9feaf32 to 3eab4eb Compare July 12, 2025 10:35
@rfc2822 rfc2822 force-pushed the androidevent-entity branch from 6e4d2a5 to 5012782 Compare July 12, 2025 11:04
Copy link
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

I think it should be all good. I mainly looked at the tests to assume the new code will behave correctly.

Maybe @ArnyminerZ can review once as well? Just to make sure :)

@rfc2822 rfc2822 merged commit b59ceec into main Jul 21, 2025
9 checks passed
@rfc2822 rfc2822 deleted the androidevent-entity branch July 21, 2025 09:57
@github-project-automation github-project-automation bot moved this from Todo to Done in DAVx⁵ Releases Jul 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Quality improvement of existing functions
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants