Skip to content

Commit

Permalink
Fixing #9 and #8
Browse files Browse the repository at this point in the history
  • Loading branch information
crspybits committed Nov 28, 2020
1 parent f0d95de commit cbf1635
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 7 deletions.
Binary file modified Docs/Procedures.docx
Binary file not shown.
17 changes: 15 additions & 2 deletions Sources/Server/Controllers/UserController/UserController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -222,14 +222,27 @@ class UserController : ControllerProtocol {
return
}

// When deleting a user, should set to NULL any sharing users that have that userId (being deleted) as their owningUserId.
// TODO: When deleting a user, should set to NULL any sharing users that have that userId (being deleted) as their owningUserId.

// This is somewhat aggressive: I'm going to remove both `Upload` and `DeferredUpload` records. It's somewhat aggressive because this gets rid of "work" the user has already done. However both of these record types have a UserId field, and this userId is being removed below (when we remove the `UserRepository` record). Plus, if the user was the cloud storage owner on these files, these files are getting marked as deleted as part of this user deletion anyways.
let uploadRepoKey = UploadRepository.LookupKey.userId(params.currentSignedInUser!.userId)
let removeResult2 = params.repos.upload.retry {
return params.repos.upload.remove(key: uploadRepoKey)
}
guard case .removed = removeResult2 else {
let message = "Could not remove upload files for user!"
let message = "Could not remove Upload records for user!"
Log.error(message)
params.completion(.failure(.message(message)))
return
}

// 11/28/20; Didn't have this DeferredUpload deletion until today. I think this was the source of https://github.com/SyncServerII/ServerMain/issues/9 and https://github.com/SyncServerII/ServerMain/issues/8.
let deferrredUploadRepoKey = DeferredUploadRepository.LookupKey.userId(params.currentSignedInUser!.userId)
let deferrredUploadRemoveResult = params.repos.deferredUpload.retry {
return params.repos.deferredUpload.remove(key: deferrredUploadRepoKey)
}
guard case .removed = deferrredUploadRemoveResult else {
let message = "Could not remove DeferredUpload records for user!"
Log.error(message)
params.completion(.failure(.message(message)))
return
Expand Down
6 changes: 6 additions & 0 deletions Sources/Server/Database/Files/DeferredUploadRepository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ class DeferredUploadRepository : Repository, RepositoryLookup, ModelIndexId {
case deferredUploadId(Int64)
case fileGroupUUIDWithStatus(fileGroupUUID: String, status: DeferredUploadStatus)
case resultsUUID(String)
case userId(UserId)

var description : String {
switch self {
Expand All @@ -150,6 +151,8 @@ class DeferredUploadRepository : Repository, RepositoryLookup, ModelIndexId {
return "fileGroupUUID(\(fileGroupUUID); status: \(status.rawValue))"
case .resultsUUID(let resultsUUID):
return "resultsUUID(\(resultsUUID))"
case .userId(let userId):
return "userId(\(userId))"
}
}
}
Expand All @@ -162,6 +165,8 @@ class DeferredUploadRepository : Repository, RepositoryLookup, ModelIndexId {
return "fileGroupUUID = '\(fileGroupUUID)' and status = '\(status.rawValue)'"
case .resultsUUID(let resultsUUID):
return "resultsUUID = '\(resultsUUID)'"
case .userId(let userId):
return "userId = \(userId)"
}
}

Expand Down Expand Up @@ -244,6 +249,7 @@ class DeferredUploadRepository : Repository, RepositoryLookup, ModelIndexId {
}

// A nil result indicates an error. No rows in the query is returned as an empty array.
// This `select` is not constrained by `UserId` because it is used from the `Uploader`, and the intent there is that the Uploader works *across* users.
func select(rowsWithStatus status: [DeferredUploadStatus]) -> [DeferredUpload]? {
let quotedStatusString = status.map {$0.rawValue}.map {"'\($0)'"}.joined(separator: ",")

Expand Down
5 changes: 4 additions & 1 deletion Sources/Server/ServerUploader/Uploader+UploadDeletion.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ extension Uploader {
throw Errors.mismatchWithDeferredUploadIdsCount
}

// Case 2) continued
if deferredUploadIds.count > 0 {
guard let theUploads = uploadRepo.select(forDeferredUploadIds: deferredUploadIds) else {
throw Errors.failedToGetUploads
Expand All @@ -104,12 +105,14 @@ extension Uploader {
}
}

// End: specific code for Case 1) and Case 2); moving on to more general code.

// Not going to worry about any resulting errors because: (a) we might be trying the deletion a 2nd (or more) time and the file might just not be there, (b) the consequences of leaving a file in cloud storage are not dire-- just some garbage that could possibly be cleaned up later.
if let errors = FileDeletion.apply(deletions: fileDeletions) {
Log.warning("Some error(s) occurred while deleting files: \(errors)")
}

// Now, delete the database records.
// Now, delete the Upload database records, and change the status of the DeferredUpload records to `completed`.

for upload in uploads {
let key = UploadRepository.LookupKey.uploadId(upload.uploadId)
Expand Down
10 changes: 6 additions & 4 deletions Sources/Server/ServerUploader/Uploader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ class Uploader: UploaderProtocol {
}

Log.debug("Got lock!")


// Get `pendingDeletions` across all users. The `Uploader` works across all users-- and this is OK because of the mutex lock.
guard let deferredFileDeletions = deferredUploadRepo.select(rowsWithStatus: [.pendingDeletion]) else {
Log.error("Failed setting up select to get deferred upload deletions")
try releaseLock()
Expand All @@ -152,6 +153,7 @@ class Uploader: UploaderProtocol {
// Must do pruning based on deletions before fetching the deferred file changes. Because the pruning may remove some of the deferred file changes.
guard pruneFileUploads(deferredFileDeletions: deferredFileDeletions) else {
Log.error("Failed pruning file uploads.")
// TODO(https://github.com/SyncServerII/ServerMain/issues/9)
try releaseLock()
throw Errors.failedToGetDeferredUploads
}
Expand Down Expand Up @@ -351,6 +353,7 @@ class Uploader: UploaderProtocol {
}

// Remove file uploads (DeferredUpload, Upload's) corresponding to these deletions. They are not relevant any more given that we're doing deletions.
// If `deferredFileDeletions` is empty, this returns immediately.
func pruneFileUploads(deferredFileDeletions: [DeferredUpload]) -> Bool {
guard deferredFileDeletions.count > 0 else {
return true
Expand Down Expand Up @@ -396,8 +399,7 @@ class Uploader: UploaderProtocol {
var deferredIdsForFileChangeUploads = [Int64]()

for deferredDeletion in deferredDeletionsWithoutFileGroups {
#warning("9/20/20: This lookup just failed. And the server is now stuck in terms of deferred operations. Need some means of removing these if they fail more than a set number of times. This looks like an example where I could record this on the deferred upload record as an error-- so the client can know of the failure.")
#warning("If we get this again, one fix would be to remove the DeferredUpload that caused this to fail.")
// TODO(https://github.com/SyncServerII/ServerMain/issues/8)
// Get the deletion Upload associated with this `deferredDeletion`
let key1 = UploadRepository.LookupKey.deferredUploadId(deferredDeletion.deferredUploadId)
guard case .found(let model) = uploadRepo.lookup(key: key1, modelInit: Upload.init), let uploadDeletion = model as? Upload else {
Expand Down Expand Up @@ -427,7 +429,7 @@ class Uploader: UploaderProtocol {
return false
}
}
} // end for
} // end for: deferredDeletionsWithoutFileGroups

// Get rid of any non-distinct deferredUploadId's for file DeferredUpload's
deferredIdsForFileChangeUploads = Array(Set<Int64>(deferredIdsForFileChangeUploads))
Expand Down

0 comments on commit cbf1635

Please sign in to comment.