Skip to content

Commit

Permalink
Improve UX for cloud save (#7836)
Browse files Browse the repository at this point in the history 
* Showing more cloud state on the project cards

* showing last cloud edit time and state in UX

* Better date appearance; better alignment; todo lst

* dynamic homescreen update works

* Dim the cloud state message in the editor

* Notify virtual api of header change at the start of cloud save (so we can show "saving...")

* Keep cloudInProgressSyncStartTime in memory only

* cloud sync update without reload is working!

* update home page project list when num headers changes

* Fix how delete propegates

* Update virtual APIs & DataSubscriber to propegate specific path that changed from invalidate() call

* Revert virtual API changes. one-to-many subscriptions need more work before they'll funciton.

* Remove cloud sync message on home screen

* Don't show cloud state unless saving

* Keep a "saved!" indicator around right after we cloud save.

* Move CloudStateSummary out of pxt localtypings

* fix render bug

* remove debug logging

* Don't download deleted projects
  • Loading branch information
@darzu
darzu committed Feb 8, 2021
1 parent 2b62829 commit 28878de
Show file tree
Hide file tree
Showing 9 changed files with 189 additions and 96 deletions.
3 changes: 1 addition & 2 deletions localtypings/pxtpackage.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ declare namespace pxt {

type CodeCardType = "file" | "example" | "codeExample" | "tutorial" | "side" | "template" | "package" | "hw" | "forumUrl" | "forumExample" | "sharedExample" | "link";
type CodeCardEditorType = "blocks" | "js" | "py";
type CodeCardCloudState = "local" | "cloud";

interface Map<T> {
[index: string]: T;
Expand Down Expand Up @@ -169,8 +168,8 @@ declare namespace pxt {
cardType?: CodeCardType;
editor?: CodeCardEditorType;
otherActions?: CodeCardAction[];
cloudState?: CodeCardCloudState;
directOpen?: boolean; // skip the details view, directly do the card action
projectId?: string; // the project's header ID

header?: string;

Expand Down
4 changes: 4 additions & 0 deletions theme/common.less
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1077,6 +1077,10 @@ p.ui.font.small {
font-size: 0.95rem;
}

#projectNameArea .cloudState {
color: rgba(0, 0, 0, 0.5);
}

/*******************************
Media Adjustments
*******************************/
Expand Down
3 changes: 2 additions & 1 deletion webapp/src/app.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1329,7 +1329,8 @@ export class ProjectView
// We are too late; the editor has already been loaded.
// Call the onChanges handler to update the editor.
pxt.tickEvent(`identity.syncOnProjectOpen.timedout`, { 'elapsedSec': elapsed})
cloud.onChangesSynced(changes)
if (changes.length)
cloud.forceReloadForCloudSync()
} else {
// We're not too late, update the local var so that the
// first load has the new info.
Expand Down
187 changes: 117 additions & 70 deletions webapp/src/cloud.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@ type WorkspaceProvider = pxt.workspace.WorkspaceProvider;

import U = pxt.Util;

const state = {
uploadCount: 0,
downloadCount: 0
};

type CloudProject = {
id: string;
header: string;
Expand All @@ -37,6 +32,24 @@ export function excludeLocalOnlyMetadataFields(h: Header): Header {
return clone
}

export type CloudStateSummary = ""/*none*/ | "saved" | "justSaved" | "offline" | "syncing" | "conflict" | "localEdits";
export function getCloudSummary(h: pxt.workspace.Header, md: CloudTempMetadata): CloudStateSummary {
if (!h.cloudUserId || !md)
return "" // none
if (!auth.loggedInSync())
return "offline"
if (md.cloudInProgressSyncStartTime > 0)
return "syncing"
if (!h.cloudCurrent)
return "localEdits"
if (md.justSaved)
return "justSaved"
if (h.cloudLastSyncTime > 0)
return "saved"
pxt.reportError("cloudsave", `Invalid project cloud state for project ${h.name}(${h.id.substr(0, 4)}..): user: ${h.cloudUserId}, inProg: ${md.cloudInProgressSyncStartTime}, cloudCurr: ${h.cloudCurrent}, lastCloud: ${h.cloudLastSyncTime}`);
return ""
}

async function listAsync(): Promise<Header[]> {
return new Promise(async (resolve, reject) => {
// Note: Cosmos & our backend does not return e-tags each individual item in a list operation
Expand Down Expand Up @@ -85,19 +98,42 @@ function getAsync(h: Header): Promise<File> {
});
}

// temporary per-project cloud metadata is only kept in memory and shouldn't be persisted to storage.
export interface CloudTempMetadata {
cloudInProgressSyncStartTime?: number,
justSaved?: boolean,
}
const temporaryHeaderMetadata: { [key: string]: CloudTempMetadata } = {};
export function getCloudTempMetadata(headerId: string): CloudTempMetadata {
return temporaryHeaderMetadata[headerId] || {};
}
function updateCloudTempMetadata(headerId: string, props: Partial<CloudTempMetadata>) {
const oldMd = temporaryHeaderMetadata[headerId] || {};
const newMd = { ...oldMd, ...props }
temporaryHeaderMetadata[headerId] = newMd
data.invalidate(`${HEADER_CLOUDSTATE}:${headerId}`);
}

function setAsync(h: Header, prevVersion: Version, text?: ScriptText): Promise<Version> {
return new Promise(async (resolve, reject) => {
const userId = auth.user()?.id;
h.cloudUserId = userId;
h.cloudCurrent = false;
h.cloudVersion = prevVersion;
updateCloudTempMetadata(h.id, { cloudInProgressSyncStartTime: U.nowSeconds() })
const project: CloudProject = {
id: h.id,
header: JSON.stringify(excludeLocalOnlyMetadataFields(h)),
text: text ? JSON.stringify(text) : undefined,
version: prevVersion
}
const result = await auth.apiAsync<string>('/api/user/project', project);
updateCloudTempMetadata(h.id, { cloudInProgressSyncStartTime: 0, justSaved: true })
setTimeout(() => {
// slightly hacky, but we want to keep around a "saved!" message for a small time after
// a save succeeds so we notify metadata subscribers again afte a delay.
updateCloudTempMetadata(h.id, { justSaved: false })
}, 1000);
if (result.success) {
h.cloudCurrent = true;
h.cloudVersion = result.resp;
Expand All @@ -124,8 +160,8 @@ export async function saveAsync(h: Header, text?: ScriptText): Promise<CloudSave
if (!res) {
// wait to synchronize
pxt.debug('save to cloud failed; synchronizing...')
pxt.tickEvent(`identity.cloudSaveFailedTriggeringFullSync`);
await syncAsync()
pxt.tickEvent(`identity.cloudSaveFailedTriggeringPartialSync`);
await syncAsync([h])
return CloudSaveResult.SyncError;
} else {
return CloudSaveResult.Success;
Expand Down Expand Up @@ -216,31 +252,55 @@ async function syncAsyncInternal(hdrs?: Header[]): Promise<Header[]> {
})
const lastCloudChange = remoteHeaders.length ? Math.max(...remoteHeaders.map(h => h.modificationTime)) : syncStart
pxt.log(`Last cloud project change was ${agoStr(lastCloudChange)}`);
const remoteHeaderMap = U.toDictionary(remoteHeaders, h => h.id);
const remoteHeadersToProcess = U.toDictionary(remoteHeaders, h => h.id);
const localHeaderChanges: pxt.Map<Header> = {}
const toCloud = transferToCloud;
const fromCloud = async (loc: Header, rem: File) => {
const newLoc = await transferFromCloud(loc, rem)
localHeaderChanges[newLoc.id] = newLoc
return newLoc
}
let tasks = localCloudHeaders.map(async (local) => {
const remote = remoteHeaderMap[local.id];
delete remoteHeaderMap[local.id];
let didProjectCountChange = false;
let tasks: Promise<Header>[] = localCloudHeaders.map(async (local) => {
// track the fact that we're checking for updates on each project
updateCloudTempMetadata(local.id, { cloudInProgressSyncStartTime: U.nowSeconds() });

const remote = remoteHeadersToProcess[local.id];
delete remoteHeadersToProcess[local.id];
if (remote) {
local.cloudLastSyncTime = remote.cloudLastSyncTime
// Note that we use modification time to detect differences. If we had full (or partial) history, we could
// use version numbers. However we cannot currently use etags since the Cosmos list operations
// don't return etags per-version. And because of how etags work, the record itself can never
// have the latest etag version.
if (local.modificationTime !== remote.modificationTime) {
if (local.modificationTime !== remote.modificationTime || local.isDeleted !== remote.isDeleted) {
const projShorthand = `'${local.name}' (${local.id.substr(0, 5)}...)`;
const remoteFile = await getWithCacheAsync(local);
// delete always wins no matter what
if (local.isDeleted) {
// Mark remote copy as deleted.
pxt.debug(`Propegating ${projShorthand} delete to cloud.`)
const newHdr = await toCloud(local, remoteFile.version)
pxt.tickEvent(`identity.sync.localDeleteUpdatedCloud`)
return newHdr
}
if (remote.isDeleted) {
// Delete local copy.
pxt.debug(`Propegating ${projShorthand} delete from cloud.`)
const newHdr = await fromCloud(local, remoteFile);
didProjectCountChange = true;
pxt.tickEvent(`identity.sync.cloudDeleteUpdatedLocal`)
return newHdr
}
// if it's not a delete...
if (local.cloudCurrent) {
// No local changes, download latest.
await fromCloud(local, remoteFile);
const newHdr = await fromCloud(local, remoteFile);
pxt.tickEvent(`identity.sync.noConflict.localProjectUpdatedFromCloud`)
return newHdr
} else {
// Possible conflict.
const conflictStr = `conflict found for '${local.name}' (${local.id.substr(0, 5)}...). Last cloud change was ${agoStr(remoteFile.header.modificationTime)} and last local change was ${agoStr(local.modificationTime)}.`
const conflictStr = `conflict found for ${projShorthand}. Last cloud change was ${agoStr(remoteFile.header.modificationTime)} and last local change was ${agoStr(local.modificationTime)}.`
// last write wins.
if (local.modificationTime > remoteFile.header.modificationTime) {
if (local.cloudVersion === remoteFile.version) {
Expand All @@ -253,35 +313,17 @@ async function syncAsyncInternal(hdrs?: Header[]): Promise<Header[]> {
pxt.log(conflictStr + ' Local will overwrite cloud.')
pxt.tickEvent(`identity.sync.conflict.localOverwrittingCloud`)
}
await toCloud(local, remoteFile.version);
return await toCloud(local, remoteFile.version);
} else {
// conflict and remote wins
// TODO: Pop a dialog and/or show the user a diff. Better yet, handle merges.
pxt.log(conflictStr + ' Cloud will overwrite local.')
pxt.tickEvent(`identity.sync.conflict.cloudOverwritingLocal`)
await fromCloud(local, remoteFile);
return await fromCloud(local, remoteFile);
}
}
} else {
if (local.isDeleted) {
// Delete remote copy.
//return deleteAsync(local, local.cloudVersion);
// Mark remote copy as deleted.
remote.isDeleted = true;
await toCloud(local, remote.cloudVersion)
pxt.tickEvent(`identity.sync.localDeleteUpdatedCloud`)
}
if (remote.isDeleted) {
// Delete local copy.
local.isDeleted = true;
localHeaderChanges[local.id] = local
await workspace.forceSaveAsync(local, {}, true)
.then(() => { data.clearCache(); })
pxt.tickEvent(`identity.sync.cloudDeleteUpdatedLocal`)
}
// Nothing to do. We're up to date locally.
return Promise.resolve();
}
return local // no changes
} else if (!partialSync) {
if (local.cloudVersion) {
pxt.debug(`Project ${local.id} incorrectly thinks it is synced to the cloud (ver: ${local.cloudVersion})`)
Expand All @@ -292,18 +334,32 @@ async function syncAsyncInternal(hdrs?: Header[]): Promise<Header[]> {
}
// Local cloud synced project exists, but it didn't make it to the server,
// so let's push it now.
await toCloud(local, null)
return await toCloud(local, null)
}
else {
// no remote verison so nothing to do
return local
}
});
tasks = [...tasks, ...remoteHeaders.map(async (remote) => {
if (remoteHeaderMap[remote.id]) {
tasks = [...tasks, ...U.values(remoteHeadersToProcess)
.filter(h => !h.isDeleted) // don't bother downloading deleted projects
.map(async (remote) => {
// Project exists remotely and not locally, download it.
const remoteFile = await getWithCacheAsync(remote);
pxt.debug(`importing new cloud project '${remoteFile.header.name}' (${remoteFile.header.id})`)
await fromCloud(null, remoteFile)
const res = await fromCloud(null, remoteFile)
pxt.tickEvent(`identity.sync.importCloudProject`)
didProjectCountChange = true;
return res;
})]
tasks = tasks.map(async t => {
const newHdr = await t
// reset cloud state sync metadata if there is any
if (getCloudTempMetadata(newHdr.id).cloudInProgressSyncStartTime > 0) {
updateCloudTempMetadata(newHdr.id, { cloudInProgressSyncStartTime: 0 });
}
})]
return newHdr;
})
await Promise.all(tasks);

// sanity check: all cloud headers should have a new sync time
Expand All @@ -316,9 +372,11 @@ async function syncAsyncInternal(hdrs?: Header[]): Promise<Header[]> {
const elapsed = U.nowSeconds() - syncStart;
const localHeaderChangesList = U.values(localHeaderChanges)
pxt.log(`Cloud sync finished after ${elapsed} seconds with ${localHeaderChangesList.length} local changes.`);
pxt.tickEvent(`identity.sync.finished`, {elapsed})
if (!partialSync) {
onChangesSynced(localHeaderChangesList)
pxt.tickEvent(`identity.sync.finished`, { elapsed })
if (didProjectCountChange) {
// headers are individually invalidated as they are synced, but if new projects come along we also need to
// update the global headers list.
data.invalidate("headers:");
}

return localHeaderChangesList
Expand All @@ -329,17 +387,15 @@ async function syncAsyncInternal(hdrs?: Header[]): Promise<Header[]> {
return [];
}

export function onChangesSynced(changes: Header[]) {
if (changes.length) {
// TODO: This is too heavy handed. We can be more fine grain here with some work.
// preferably with just the virtual data APIs we can push updates to the whole editor.
core.infoNotification(lf("Cloud synchronization finished. Reloading... "));
setTimeout(() => {
pxt.log("Forcing reload.")
pxt.tickEvent(`identity.sync.forcingReload`)
location.reload();
}, 3000);
}
export function forceReloadForCloudSync() {
// TODO: This is too heavy handed. We can be more fine grain here with some work.
// preferably with just the virtual data APIs we can push updates to the whole editor.
core.infoNotification(lf("Cloud synchronization finished. Reloading... "));
setTimeout(() => {
pxt.log("Forcing reload.")
pxt.tickEvent(`identity.sync.forcingReload`)
location.reload();
}, 3000);
}

export async function convertCloudToLocal(userId: string) {
Expand All @@ -363,24 +419,15 @@ export async function convertCloudToLocal(userId: string) {
* Virtual API
*/

const MODULE = "cloud";
const FIELD_UPLOADING = "uploading";
const FIELD_DOWNLOADING = "downloading";
const FIELD_WORKING = "working";
export const UPLOADING = `${MODULE}:${FIELD_UPLOADING}`;
export const DOWNLOADING = `${MODULE}:${FIELD_DOWNLOADING}`;
export const WORKING = `${MODULE}:${FIELD_WORKING}`;
export const HEADER_CLOUDSTATE = "header-cloudstate"

function cloudApiHandler(p: string): any {
switch (data.stripProtocol(p)) {
case FIELD_UPLOADING: return state.uploadCount > 0;
case FIELD_DOWNLOADING: return state.downloadCount > 0;
case WORKING: return cloudApiHandler(UPLOADING) || cloudApiHandler(DOWNLOADING);
}
return null;
function cloudHeaderMetadataHandler(p: string): any {
p = data.stripProtocol(p)
if (p == "*") return workspace.getHeaders().map(h => getCloudTempMetadata(h.id))
return getCloudTempMetadata(p)
}

export function init() {
// 'cloudws' because 'cloud' protocol is already taken.
data.mountVirtualApi("cloudws", { getSync: cloudApiHandler });
data.mountVirtualApi(HEADER_CLOUDSTATE, { getSync: cloudHeaderMetadataHandler });
}

Loading

0 comments on commit 28878de

Please sign in to comment.