Skip to content

Create protocol layers for EventHistory and EventHistoryDatabase#1136

Merged
timkimadobe merged 6 commits intoadobe:feature/historical-consequencefrom
timkimadobe:event-history-protocol
Apr 29, 2025
Merged

Create protocol layers for EventHistory and EventHistoryDatabase#1136
timkimadobe merged 6 commits intoadobe:feature/historical-consequencefrom
timkimadobe:event-history-protocol

Conversation

@timkimadobe
Copy link
Copy Markdown
Contributor

Description

This PR creates protocol layers for EventHistory and EventHistoryDatabase for a cleaner testing setup.

The new protocols, EventHistoryService and EventHistoryDatabaseService, simply define the current internally available methods from their respective classes.

The existing EventHistory and EventHistoryDatabase classes have been updated to conform to their respective protocols.

EventHub has been updated to use the EventHistoryService protocol for eventHistory and use dependency injection to set this value, with the default being the real EventHistory

  • By providing the real EventHistory as the default value, it doesn't change the signature of any previous calls to EventHub()

Note

My understanding (please correct me if I'm wrong!) is that this also shouldn't change any real, meaningful runtime behavior of EventHub, since by moving the instantiation of EventHistory from the default property value to the initializer, EventHistory creation and set is being moved from phase 1 to phase 2 of class initialization as described here: https://docs.swift.org/swift-book/documentation/the-swift-programming-language/initialization/#Two-Phase-Initialization

  • Which means that the initialization and setting of the real EventHistory still happens completely within the initialization process of EventHub

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

… EventHub to use dependency injection for EventHistory

Update MockEventHistoryDatabase to use new EventHistoryDatabaseService protocol
Update EventHistory and EventHistoryDatabase to conform to their respective protocols
Update MockEventHistory to conform to EventHistoryService protocol and fix implementation
Fix EventHubEventHistoryTests to use the instantiated eventhub instance not the shared one

# Conflicts:
#	AEPCore.xcodeproj/project.pbxproj
#	AEPCore/Mocks/PublicTestUtils/MockEventHistoryDatabase.swift
#	AEPCore/Tests/EventHubTests/EventHubEventHistoryTests.swift
#	AEPCore/Tests/TestHelpers/MockEventHistory.swift
Copy link
Copy Markdown
Contributor

@cdhoffmann cdhoffmann 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! I really want @sbenedicadb to take a look as well though :)

private var preprocessors = ThreadSafeArray<EventPreprocessor>(identifier: "com.adobe.eventHub.preprocessors.queue")
private var started = false // true if the `EventHub` is started, false otherwise. Should only be accessed from within the `eventHubQueue`
private var eventHistory = EventHistory()
private var eventHistory: EventHistoryService?
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can this be made to a Kotlin final/val equivalent

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't see any usages either in production or testing where setting this directly is used, so I've updated this from var to let

@timkimadobe timkimadobe merged commit 4f4240d into adobe:feature/historical-consequence Apr 29, 2025
17 checks passed
@timkimadobe timkimadobe deleted the event-history-protocol branch April 29, 2025 18:18
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.

3 participants