Skip to content

Commit

Permalink
Fix issues with temporary login
Browse files Browse the repository at this point in the history
The first issue is attempting to get last update time in
OfflineIndicatorViewModel. It was trying to wait for partial login but
unfortunately temporary login is also partial login.

We didn't want to just return null in this case as it might get
interpreted incorrectly and instead we returned a sum type with the
exact case and ignore uninitialized storage in this case.

The other issue was with LoginController and LoginListener not properly
resetting themselves on logout (e.g. after temporary login) and giving
incorrect results.

The third issue was with PostLoginActions initializing MailModel for
temporary login. If a real login into another account is attempted
afterwards it would lead to more errors.

To fully solve the problem we should rethink how temporary login affects
the rest of the app and perhaps skip most of the things that happen
during normal login.

fix #4559
  • Loading branch information
charlag committed Sep 7, 2022
1 parent 4b767be commit b8aa439
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 19 deletions.
6 changes: 5 additions & 1 deletion src/api/main/LoginController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export class LoginControllerImpl implements LoginController {
private fullyLoggedIn: boolean = false
private atLeastPartiallyLoggedIn: boolean = false

async init() {
init() {
this.waitForFullLogin().then(async () => {
this.fullyLoggedIn = true
await this.waitForPartialLogin()
Expand Down Expand Up @@ -256,6 +256,10 @@ export class LoginControllerImpl implements LoginController {
await this.userController.deleteSession(sync)
this.userController = null
this.partialLogin = defer()
this.fullyLoggedIn = false
const locator = await this.getMainLocator()
locator.loginListener.reset()
this.init()
} else {
console.log("No session to delete")
}
Expand Down
6 changes: 6 additions & 0 deletions src/api/main/LoginListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ export class LoginListener {
) {
}

/** e.g. after temp logout */
reset() {
this.loginPromise = defer()
this.fullLoginFailed = false
}

waitForFullLogin(): Promise<void> {
return this.loginPromise.promise
}
Expand Down
7 changes: 4 additions & 3 deletions src/api/worker/offline/OfflineStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
listIdPart,
timestampToGeneratedId
} from "../../common/utils/EntityUtils.js"
import {CacheStorage, expandId, ExposedCacheStorage} from "../rest/DefaultEntityRestCache.js"
import {CacheStorage, expandId, ExposedCacheStorage, LastUpdateTime} from "../rest/DefaultEntityRestCache.js"
import * as cborg from "cborg"
import {EncodeOptions, Token, Type} from "cborg"
import {assert, DAY_IN_MILLIS, getTypeId, groupByAndMap, mapNullable, TypeRef} from "@tutao/tutanota-utils"
Expand Down Expand Up @@ -217,8 +217,9 @@ AND NOT(${firstIdBigger("elementId", upper)})`
await this.sqlCipherFacade.run(query, params)
}

async getLastUpdateTime(): Promise<number | null> {
return this.getMetadata("lastUpdateTime")
async getLastUpdateTime(): Promise<LastUpdateTime> {
const time = await this.getMetadata("lastUpdateTime")
return time ? {type: "recorded", time} : {type: "never"}
}

async putLastUpdateTime(ms: number): Promise<void> {
Expand Down
9 changes: 6 additions & 3 deletions src/api/worker/rest/CacheStorageProxy.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {CacheStorage, Range} from "./DefaultEntityRestCache.js"
import {CacheStorage, LastUpdateTime, Range} from "./DefaultEntityRestCache.js"
import {ProgrammingError} from "../../common/error/ProgrammingError"
import {ListElementEntity, SomeEntity} from "../../common/EntityTypes"
import {TypeRef} from "@tutao/tutanota-utils"
Expand All @@ -22,6 +22,7 @@ interface CacheStorageInitReturn {

export interface CacheStorageLateInitializer {
initialize(args: OfflineStorageInitArgs | null): Promise<CacheStorageInitReturn>;

deInitialize(): Promise<void>;
}

Expand Down Expand Up @@ -112,8 +113,10 @@ export class LateInitializedCacheStorageImpl implements CacheStorageLateInitiali
return this.inner.getLastBatchIdForGroup(groupId)
}

getLastUpdateTime(): Promise<number | null> {
return this.inner.getLastUpdateTime()
async getLastUpdateTime(): Promise<LastUpdateTime> {
return this._inner
? this.inner.getLastUpdateTime()
: {type: "uninitialized"}
}

getRangeForList<T extends ListElementEntity>(typeRef: TypeRef<T>, listId: Id): Promise<Range | null> {
Expand Down
20 changes: 16 additions & 4 deletions src/api/worker/rest/DefaultEntityRestCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ export interface EntityRestCache extends EntityRestInterface {

export type Range = {lower: Id, upper: Id}

export type LastUpdateTime =
| {type: "recorded", time: number}
| {type: "never"}
| {type: "uninitialized"}

/**
* Part of the cache storage only with subset of CacheStorage functionality
*
Expand All @@ -106,7 +111,7 @@ export interface ExposedCacheStorage {
*/
getWholeList<T extends ListElementEntity>(typeRef: TypeRef<T>, listId: Id): Promise<Array<T>>

getLastUpdateTime(): Promise<number | null>
getLastUpdateTime(): Promise<LastUpdateTime>
}

export interface CacheStorage extends ExposedCacheStorage {
Expand Down Expand Up @@ -251,11 +256,18 @@ export class DefaultEntityRestCache implements EntityRestCache {

async timeSinceLastSyncMs(): Promise<number | null> {
const lastUpdate = await this.storage.getLastUpdateTime()
if (lastUpdate == null) {
return null
let lastUpdateTime: number
switch (lastUpdate.type) {
case "recorded":
lastUpdateTime = lastUpdate.time
break
case "never":
return null
case "uninitialized":
throw new ProgrammingError("Offline storage is not initialized")
}
const now = this.getServerTimestampMs()
return now - lastUpdate
return now - lastUpdateTime
}

private getServerTimestampMs(): number {
Expand Down
6 changes: 3 additions & 3 deletions src/api/worker/rest/EphemeralCacheStorage.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {ElementEntity, ListElementEntity, SomeEntity} from "../../common/EntityTypes.js"
import {EntityRestClient, typeRefToPath} from "./EntityRestClient.js"
import {firstBiggerThanSecond, getElementId, getListId, isElementEntity} from "../../common/utils/EntityUtils.js"
import {CacheStorage} from "./DefaultEntityRestCache.js"
import {CacheStorage, LastUpdateTime} from "./DefaultEntityRestCache.js"
import {clone, getFromMap, remove, TypeRef} from "@tutao/tutanota-utils"
import {CustomCacheHandlerMap} from "./CustomCacheHandler.js"

Expand Down Expand Up @@ -210,8 +210,8 @@ export class EphemeralCacheStorage implements CacheStorage {
}


async getLastUpdateTime(): Promise<number | null> {
return this.lastUpdateTime
async getLastUpdateTime(): Promise<LastUpdateTime> {
return this.lastUpdateTime ? {type: "recorded", time: this.lastUpdateTime} : {type: "never"}
}

async putLastUpdateTime(value: number): Promise<void> {
Expand Down
15 changes: 11 additions & 4 deletions src/gui/base/OfflineIndicatorViewModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,18 @@ export class OfflineIndicatorViewModel {
private async onWsStateChange(newState: WsConnectionState): Promise<void> {
this.lastWsState = newState
if (newState !== WsConnectionState.connected) {
await this.logins!.waitForPartialLogin()
const lastUpdate = await this.cacheStorage!.getLastUpdateTime()
this.lastUpdate = lastUpdate != null
? new Date(lastUpdate)
: null
switch (lastUpdate.type) {
case "recorded":
this.lastUpdate = new Date(lastUpdate.time)
break
case "never":
// We can get into uninitialized state after temporary login e.g. during signup
case "uninitialized":
this.lastUpdate = null
this.wsWasConnectedBefore = false
break
}

} else {
this.wsWasConnectedBefore = true
Expand Down
3 changes: 2 additions & 1 deletion src/login/PostLoginActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export class PostLoginActions implements IPostLoginAction {

// only show "Tutanota" after login if there is no custom title set
if (!logins.getUserController().isInternalUser()) {

if (document.title === LOGIN_TITLE) {
document.title = "Tutanota"
}
Expand All @@ -84,7 +85,7 @@ export class PostLoginActions implements IPostLoginAction {
hourCycle: getHourCycle(logins.getUserController().userSettingsGroupRoot),
})

if (!isAdminClient()) {
if (!isAdminClient() && loggedInEvent.sessionType !== SessionType.Temporary) {
await locator.mailModel.init()
}
if (isApp() || isDesktop()) {
Expand Down

0 comments on commit b8aa439

Please sign in to comment.