Skip to content

Commit

Permalink
Merge pull request #95 from GetToSet/ethanwong/database-batch-operations
Browse files Browse the repository at this point in the history 
Performance improvements for batch (un)marking played or favourite.
  • Loading branch information
@dyerc
dyerc authored Oct 11, 2022
2 parents 58eea28 + 5010e39 commit 6d648f7
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 75 deletions.
68 changes: 61 additions & 7 deletions Doughnut/Library/Library.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ import GRDB
protocol LibraryDelegate {
func librarySubscribedToPodcast(subscribed: Podcast)
func libraryUnsubscribedFromPodcast(unsubscribed: Podcast)
func libraryUpdatingPodcast(podcast: Podcast)
func libraryUpdatedPodcast(podcast: Podcast)
func libraryUpdatedEpisode(episode: Episode)
func libraryUpdatingPodcasts(podcasts: [Podcast])
func libraryUpdatedPodcasts(podcasts: [Podcast])
func libraryUpdatedEpisodes(episodes: [Episode])
func libraryReloaded()
}

Expand Down Expand Up @@ -390,7 +390,7 @@ class Library: NSObject {
// Mark as loading
podcast.loading = true
DispatchQueue.main.async {
self.delegate?.libraryUpdatingPodcast(podcast: podcast)
self.delegate?.libraryUpdatingPodcasts(podcasts: [podcast])
}

let newEpisodes = podcast.fetch()
Expand All @@ -407,6 +407,60 @@ class Library: NSObject {
}
}

func batchUpdateEpisodes(favourite: Bool, episodes: [Episode], completion: ((Result<[Episode], LibraryError>) -> Void)? = nil) {
dbQueue?.asyncWrite({ db in
let keys = episodes.compactMap { $0.id }
try Episode.filter(keys: keys)
.updateAll(db, Column("favourite").set(to: favourite))
}, completion: { _, result in
episodes.forEach {
$0.favourite = favourite
}

switch result {
case .success:
DispatchQueue.main.async {
self.delegate?.libraryUpdatedEpisodes(episodes: episodes)
}
completion?(.success(episodes))
case let .failure(error):
if let error = error as? DatabaseError {
Library.handleDatabaseError(error)
completion?(.failure(.databaseError(error)))
} else {
completion?(.failure(.unknown(error)))
}
}
})
}

func batchUpdateEpisodes(played: Bool, episodes: [Episode], completion: ((Result<[Episode], LibraryError>) -> Void)? = nil) {
dbQueue?.asyncWrite({ db in
let keys = episodes.compactMap { $0.id }
try Episode.filter(keys: keys)
.updateAll(db, Column("played").set(to: played))
}, completion: { _, result in
episodes.forEach {
$0.played = played
}

switch result {
case .success:
DispatchQueue.main.async {
self.delegate?.libraryUpdatedEpisodes(episodes: episodes)
}
completion?(.success(episodes))
case let .failure(error):
if let error = error as? DatabaseError {
Library.handleDatabaseError(error)
completion?(.failure(.databaseError(error)))
} else {
completion?(.failure(.unknown(error)))
}
}
})
}

// Async episode save and event emission
func save(episode: Episode, completion: ((Result<Episode, LibraryError>) -> Void)? = nil) {
dbQueue?.asyncWrite({ db in
Expand All @@ -420,7 +474,7 @@ class Library: NSObject {
case .success:
completion?(.success(episode))
DispatchQueue.main.async {
self.delegate?.libraryUpdatedEpisode(episode: episode)
self.delegate?.libraryUpdatedEpisodes(episodes: [episode])
}
case let .failure(error):
if let error = error as? DatabaseError {
Expand All @@ -443,7 +497,7 @@ class Library: NSObject {
case .success:
completion?(.success(episode))
DispatchQueue.main.async {
self.delegate?.libraryUpdatedPodcast(podcast: podcast)
self.delegate?.libraryUpdatedPodcasts(podcasts: [podcast])
}
case let .failure(error):
if let error = error as? DatabaseError {
Expand Down Expand Up @@ -500,7 +554,7 @@ class Library: NSObject {
case .success:
completion?(.success(podcast))
DispatchQueue.main.async {
self.delegate?.libraryUpdatedPodcast(podcast: podcast)
self.delegate?.libraryUpdatedPodcasts(podcasts: [podcast])
}
case let .failure(error):
if let error = error as? DatabaseError {
Expand Down
14 changes: 2 additions & 12 deletions Doughnut/View Controllers/EpisodeViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,6 @@ final class EpisodeViewController: NSViewController, NSTableViewDelegate, NSTabl
tableView.scrollRowToVisible(tableView.selectedRow)
}

func reload(forEpisode episode: Episode) {
reload(forChangedEpisodes: [episode])
}

func reload(forChangedEpisodes changedEpisodes: [Episode]?) {
let availableRowIndicesRange = tableView.availableRowIndicesRange

Expand Down Expand Up @@ -453,10 +449,7 @@ final class EpisodeViewController: NSViewController, NSTableViewDelegate, NSTabl

let shouldMarkPlayed = !allPlayed

for episode in episodes {
episode.played = shouldMarkPlayed
Library.global.save(episode: episode)
}
Library.global.batchUpdateEpisodes(played: shouldMarkPlayed, episodes: episodes)
}

@IBAction func toggleFavourite(_ sender: Any) {
Expand All @@ -466,10 +459,7 @@ final class EpisodeViewController: NSViewController, NSTableViewDelegate, NSTabl

let shouldMarkAsFavourite = !allMarkedAsFavourite

for episode in activeEpisodesForAction() {
episode.favourite = shouldMarkAsFavourite
Library.global.save(episode: episode)
}
Library.global.batchUpdateEpisodes(favourite: shouldMarkAsFavourite, episodes: episodes)
}

@IBAction func downloadEpisode(_ sender: Any) {
Expand Down
34 changes: 5 additions & 29 deletions Doughnut/View Controllers/PodcastViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,7 @@ final class PodcastViewController: NSViewController, NSTableViewDelegate, NSTabl
tableView.scrollRowToVisible(tableView.selectedRow)
}

func reload(forPodcast podcast: Podcast) {
reload(forChangedPodcasts: [podcast])
}

private func reload(forChangedPodcasts changedPodcasts: [Podcast]?) {
func reload(forChangedPodcasts changedPodcasts: [Podcast]?) {
let availableRowIndicesRange = tableView.availableRowIndicesRange

let podcastIdsBeforeReload = podcasts.map { $0.id }
Expand Down Expand Up @@ -337,34 +333,14 @@ final class PodcastViewController: NSViewController, NSTableViewDelegate, NSTabl

@IBAction func markAllAsPlayed(_ sender: Any) {
let podcasts = activePodcastsForAction()

for podcast in podcasts {
for episode in podcast.episodes {
episode.played = true
}

// Manually trigger a view reload to make update seem instant
viewController.libraryUpdatedPodcast(podcast: podcast)

// Commit changes to library
Library.global.update(podcast: podcast)
}
let episodes = podcasts.flatMap { $0.episodes }
Library.global.batchUpdateEpisodes(played: true, episodes: episodes)
}

@IBAction func markAllAsUnplayed(_ sender: Any) {
let podcasts = activePodcastsForAction()

for podcast in podcasts {
for episode in podcast.episodes {
episode.played = false
}

// Manually trigger a view reload to make update seem instant
viewController.libraryUpdatedPodcast(podcast: podcast)

// Commit changes to library
Library.global.update(podcast: podcast)
}
let episodes = podcasts.flatMap { $0.episodes }
Library.global.batchUpdateEpisodes(played: false, episodes: episodes)
}

@IBAction func copyPodcastURL(_ sender: Any) {
Expand Down
28 changes: 18 additions & 10 deletions Doughnut/View Controllers/ViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,30 +108,38 @@ final class ViewController: NSSplitViewController, LibraryDelegate {
updateWindowTitleAndDockIcon()
}

func libraryUpdatingPodcast(podcast: Podcast) {
podcastViewController.reload(forPodcast: podcast)
func libraryUpdatingPodcasts(podcasts: [Podcast]) {
podcastViewController.reload(forChangedPodcasts: podcasts)
updateWindowTitleAndDockIcon()
}

func libraryUpdatedPodcast(podcast: Podcast) {
podcastViewController.reload(forPodcast: podcast)
func libraryUpdatedPodcasts(podcasts: [Podcast]) {
podcastViewController.reload(forChangedPodcasts: podcasts)

if episodeViewController.podcast?.id == podcast.id {
if podcasts.contains(where: { episodeViewController.podcast?.id == $0.id }) {
episodeViewController.reloadEpisodes()
}

updateWindowTitleAndDockIcon()
}

func libraryUpdatedEpisode(episode: Episode) {
if episodeViewController.podcast?.id == episode.podcastId {
episodeViewController.reload(forEpisode: episode)
func libraryUpdatedEpisodes(episodes: [Episode]) {
let currentEpisodes = episodes.filter {
episodeViewController.podcast?.id == $0.podcastId
}

if let podcast = episode.podcast {
podcastViewController.reload(forPodcast: podcast)
episodeViewController.reload(forChangedEpisodes: currentEpisodes)

var podcasts = [Podcast]()

for episode in episodes {
if let podcast = episode.podcast, !podcasts.contains(where: { $0 === podcast }) {
podcasts.append(podcast)
}
}

podcastViewController.reload(forChangedPodcasts: podcasts)

updateWindowTitleAndDockIcon()
}

Expand Down
14 changes: 7 additions & 7 deletions DoughnutTests/LibraryTests/LibrarySpyDelegate.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,18 @@ class LibrarySpyDelegate: LibraryDelegate {
}

var updatedPodcastExpectation: XCTestExpectation?
var updatedPodcastResult: Podcast?
func libraryUpdatedPodcast(podcast: Podcast) {
var updatedPodcastResults = [Podcast]()
func libraryUpdatedPodcasts(podcasts: [Podcast]) {
guard let expectation = updatedPodcastExpectation else { return }
updatedPodcastResult = podcast
updatedPodcastResults = podcasts
expectation.fulfill()
}

var updatedEpisodeExpectation: XCTestExpectation?
var updatedEpisodeResult: Episode?
func libraryUpdatedEpisode(episode: Episode) {
var updatedEpisodeResults = [Episode]()
func libraryUpdatedEpisodes(episodes: [Episode]) {
guard let expectation = updatedEpisodeExpectation else { return }
updatedEpisodeResult = episode
updatedEpisodeResults = episodes
expectation.fulfill()
}

Expand All @@ -57,7 +57,7 @@ class LibrarySpyDelegate: LibraryDelegate {
expectation.fulfill()
}

func libraryUpdatingPodcast(podcast: Podcast) {
func libraryUpdatingPodcasts(podcasts: [Podcast]) {

}
}
18 changes: 8 additions & 10 deletions DoughnutTests/LibraryTests/LibraryTestsWithSubscription.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,9 @@ class LibraryTestsWithSubscription: LibraryTestCase {
Library.global.reload(podcast: sub!)

self.waitForExpectations(timeout: 10) { _ in
guard let podcast = spy.updatedPodcastResult else {
XCTFail("Expected delegate to be called")
return
}

XCTAssertEqual(podcast.episodes.count, 2)
let podcasts = spy.updatedPodcastResults
XCTAssertEqual(podcasts.count, 1)
XCTAssertEqual(podcasts.first!.episodes.count, 2)
}
}

Expand All @@ -76,10 +73,11 @@ class LibraryTestsWithSubscription: LibraryTestCase {
Library.global.reload(podcast: sub!)

self.waitForExpectations(timeout: 10) { _ in
guard let podcast = spy.updatedPodcastResult else {
XCTFail("Expected delegate to be called")
return
}
let podcasts = spy.updatedPodcastResults

XCTAssertEqual(podcasts.count, 1)

let podcast = podcasts.first!

XCTAssertEqual(podcast.episodes.count, 3)

Expand Down

0 comments on commit 6d648f7

Please sign in to comment.