Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bugs related to the settings and remove dependency on electron-json-storage #662

Merged
merged 6 commits into from
Sep 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/components/base/editor/editor.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class EditorComponent implements ControlValueAccessor, AfterViewInit, OnC
this.updateValue(this.instance.getValue());

if (change.origin !== "complete" && change.origin !== "setValue") {
const hint = (CodeMirror as any).hint[this.instance.getDoc().getMode()];
const hint = (CodeMirror as any).hint[this.instance.getDoc().getMode().name];
if (hint) {
(this.instance as any).showHint({ hint: hint, completeSingle: false });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export class AutoscaleFormulaPickerComponent implements OnInit, OnDestroy, Contr
mode: "autoscale",
autoRefresh: true,
};

public customFormulaMode = true;
private _subs: Subscription[];
private _propagateChange: (value: string) => void;
Expand Down
2 changes: 1 addition & 1 deletion app/components/settings/settings.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class SettingsComponent implements OnDestroy {
private _subs: Subscription[] = [];

constructor(private settingsService: SettingsService) {
this._subs.push(this.settingsService.settingsObs.subscribe(() => {
this._subs.push(this.settingsService.settingsObs.subscribe((e) => {
this._updateOriginalValue();
}));
}
Expand Down
12 changes: 6 additions & 6 deletions app/components/shared/main-navigation.html
Original file line number Diff line number Diff line change
@@ -1,34 +1,34 @@
<ul class="top">
<li>
<a href="#" [routerLink]="['/accounts', selectedId]" routerLinkActive="active" title="Dashboard">
<a [routerLink]="['/accounts', selectedId]" routerLinkActive="active" title="Dashboard">
<i class="fa fa-th fa-lg"></i><div class="label">Dash</div>
</a>
</li>
<li>
<a href="#" routerLink="/jobs" routerLinkActive="active">
<a routerLink="/jobs" routerLinkActive="active">
<i class="fa fa-tasks fa-lg"></i><div class="label">Jobs</div>
</a>
</li>
<li>
<a href="#" routerLink="/pools" routerLinkActive="active">
<a routerLink="/pools" routerLinkActive="active">
<i class="fa fa-database fa-lg"></i><div class="label">Pools</div>
</a>
</li>
<li>
<a href="#" routerLink="/applications" routerLinkActive="active" title="Application packages">
<a routerLink="/applications" routerLinkActive="active" title="Application packages">
<i class="fa fa-file-archive-o fa-lg"></i><div class="label">Packages</div>
</a>
</li>
<li>
<a href="#" routerLink="/data" routerLinkActive="active" title="File groups">
<a routerLink="/data" routerLinkActive="active" title="File groups">
<i class="fa fa-cloud-upload fa-lg"></i><div class="label">Data</div>
</a>
</li>
</ul>
<div class="spacer"></div>
<ul class="bottom">
<li>
<a href="#" (click)="openSettingsContextMenu()" [title]="currentUserName" class="profile">
<a (click)="openSettingsContextMenu()" [title]="currentUserName" class="profile">
<i class="fa fa-user fa-lg"></i><div class="label">Profile</div>
</a>
</li>
Expand Down
35 changes: 7 additions & 28 deletions app/services/account.service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Injectable } from "@angular/core";
import { RequestOptions, URLSearchParams } from "@angular/http";
import * as storage from "electron-json-storage";
import { List } from "immutable";
import { AsyncSubject, BehaviorSubject, Observable } from "rxjs";

Expand All @@ -11,6 +10,7 @@ import { AzureHttpService } from "./azure-http.service";
import {
DataCache, DataCacheTracker, RxBasicEntityProxy, RxEntityProxy, getOnceProxy,
} from "./core";
import { LocalFileStorage } from "./local-file-storage.service";
import { SubscriptionService } from "./subscription.service";

export enum AccountStatus {
Expand Down Expand Up @@ -62,6 +62,7 @@ export class AccountService {
private _cache = new DataCache<AccountResource>();

constructor(
private storage: LocalFileStorage,
private azure: AzureHttpService,
private subscriptionService: SubscriptionService) {

Expand Down Expand Up @@ -267,40 +268,18 @@ export class AccountService {
}

private _loadFavoriteAccounts(): Observable<List<AccountResource>> {
let sub = new AsyncSubject<List<AccountResource>>();
storage.get(this._accountJsonFileName, (error, data) => {
if (error) {
log.error("Error retrieving accounts");
sub.error(error);
}

return this.storage.get(this._accountJsonFileName).map((data) => {
if (Array.isArray(data)) {
sub.next(List(data.map(x => new AccountResource(x))));
return List(data.map(x => new AccountResource(x)));
} else {
sub.next(List([]));
return List([]);
}

sub.complete();
});

return sub.asObservable();
}).share();
}

private _saveAccountFavorites(accounts: List<AccountResource> = null): Observable<any> {
let sub = new AsyncSubject();

accounts = accounts === null ? this._accountFavorites.getValue() : accounts;
storage.set(this._accountJsonFileName, accounts.toJS(), (error) => {
if (error) {
log.error("Error saving accounts", error);
sub.error(error);
}

sub.next(true);
sub.complete();
});

return sub;
return this.storage.set(this._accountJsonFileName, accounts.toJS());
}

private _getAccount(accountId: string): Observable<AccountResource> {
Expand Down
4 changes: 4 additions & 0 deletions app/services/fs.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { FileUtils } from "client/api";
export interface CommonFolders {
temp: string;
downloads: string;
appData: string;
userData: string;
}

/**
Expand All @@ -27,6 +29,8 @@ export class FileSystemService {
this.commonFolders = {
temp: path.join(app.getPath("temp"), "batch-labs"),
downloads: app.getPath("downloads"),
appData: app.getPath("appData"),
userData: app.getPath("userData"),
};
this._fileUtils = remote.getFileUtils();
}
Expand Down
60 changes: 36 additions & 24 deletions app/services/local-file-storage.service.ts
Original file line number Diff line number Diff line change
@@ -1,47 +1,59 @@
import { Injectable } from "@angular/core";
import * as storage from "electron-json-storage";
import { AsyncSubject, Observable } from "rxjs";
import * as path from "path";
import { Observable } from "rxjs";

import { log } from "app/utils";
import { FileSystemService } from "./fs.service";

/**
* This service is used to read/write files to the user data folder.
* Prefer this for writing big data over localStorage.
* Each key is a different file under userData.
*/
@Injectable()
export class LocalFileStorage {

constructor(private fs: FileSystemService) { }
/**
* @param filename Name of the file
* @param key Key where the data is store
* @returns Observable which will resolve the data contained in the file if successfull or reject if any error
*/
public get<T>(filename: string): Observable<T> {
const sub = new AsyncSubject<T>();
storage.get(filename, (error, data) => {
this._errorToSub(sub, error, data);
public get<T>(key: string): Observable<T> {
return this.read(key).map((content) => {
if (!content) {
return {};
}

try {
return JSON.parse(content);
} catch (e) {
log.error("Loading file from storage has invalid json", { key, content });
return {};
}
});
return sub.asObservable();
}

/**
* Store the given data into the given file.
* @param filename Filename to store the data
* @param filename Key to store the data(Corespond to a file under userdata)
* @param data Javascript object(JSON format) to store
* @returns observable that will resolve if saving is sucessfull or reject if any error
*/
public set<T>(filename: string, data: T): Observable<{}> {
const sub = new AsyncSubject();
storage.set(filename, data, (error) => {
this._errorToSub(sub, error);
});
return sub;
public set<T>(key: string, data: T): Observable<{}> {
const content = JSON.stringify(data);
return this.write(key, content);
}

private _errorToSub(subject: AsyncSubject<any>, error: any, data?: any) {
if (error) {
subject.error(error);
} else {
if (data) {
subject.next(data);
}
subject.complete();
}
public read(key: string): Observable<string> {
return Observable.fromPromise(this.fs.readFile(this._getFile(key)).catch(() => null));
}

public write(key: string, content: string): Observable<string> {
return Observable.fromPromise(this.fs.saveFile(this._getFile(key), content));
}

private _getFile(key: string) {
const filename = key.endsWith(".json") ? key : `${key}.json`;
return path.join(this.fs.commonFolders.userData, filename);
}
}
16 changes: 9 additions & 7 deletions app/services/settings.service.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Injectable, NgZone } from "@angular/core";
import * as storage from "electron-json-storage";
import { BehaviorSubject, Observable } from "rxjs";
// tslint:disable-next-line:no-var-requires
const stripJsonComments = require("strip-json-comments");
Expand Down Expand Up @@ -37,15 +36,17 @@ export class SettingsService {
this.loadSettings();
}

public saveUserSettings(userSettings: string) {
public saveUserSettings(userSettings: string): Observable<any> {
this.userSettingsStr = userSettings;
this.settings = { ...defaultSettings, ...this._parseUserSettings(userSettings) };
this._settingsSubject.next(this.settings);
return this.storage.set(this._filename, userSettings);
return this.storage.write(this._filename, userSettings);
}

private loadSettings() {
this.storage.get(this._filename).subscribe((userSettings: string) => {
this.storage.read(this._filename).catch(() => {
return null;
}).subscribe((userSettings: string) => {
this.userSettingsStr = userSettings;
this.settings = { ...defaultSettings, ...this._parseUserSettings(userSettings) };
this._hasSettingsLoaded.next(true);
Expand All @@ -56,9 +57,10 @@ export class SettingsService {
this.zone.run(() => {
// If the file has never been init create it
if (!Array.isArray(data)) {
storage.set(this._keybindingsFilename, [], () => {
log.error("Error saving the initial keybinding settings.");
});
this.storage.set(this._keybindingsFilename, []).catch((e) => {
log.error("Error saving the initial keybinding settings.", e);
return null;
}).subscribe();
}
this._keybindings.next(defaultKeybindings.concat(data));
});
Expand Down
40 changes: 10 additions & 30 deletions app/services/ssh-key.service.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { Injectable } from "@angular/core";
import { SSHPublicKey } from "app/models";
import { Constants, log } from "app/utils";
import * as storage from "electron-json-storage";
import { List } from "immutable";
import { AsyncSubject, BehaviorSubject, Observable } from "rxjs";

import { SSHPublicKey } from "app/models";
import { Constants } from "app/utils";
import { BehaviorSubject, Observable } from "rxjs";
import { LocalFileStorage } from "./local-file-storage.service";

const filename = Constants.SavedDataFilename.sshPublicKeys;

Expand All @@ -13,7 +14,7 @@ export class SSHKeyService {

private _keys = new BehaviorSubject<List<SSHPublicKey>>(List([]));

constructor() {
constructor(private storage: LocalFileStorage) {
this.keys = this._keys.asObservable();
}

Expand All @@ -37,38 +38,17 @@ export class SSHKeyService {
}

public loadInitialData(): Observable<List<SSHPublicKey>> {
const sub = new AsyncSubject<List<SSHPublicKey>>();
storage.get(filename, (error, data) => {
if (error) {
log.error("Error retrieving ssh public keys");
sub.error(error);
}

return this.storage.get(filename).map((data) => {
if (Array.isArray(data)) {
sub.next(List(data));
return List(data);
} else {
sub.next(List([]));
return List([]);
}

sub.complete();
});
return sub.asObservable();
}

private _saveSSHPublicKeys(keys: List<SSHPublicKey> = null): Observable<any> {
let sub = new AsyncSubject();

keys = keys === null ? this._keys.value : keys;
storage.set(filename, keys.toJS(), (error) => {
if (error) {
log.error("Error saving accounts", error);
sub.error(error);
}

sub.next(true);
sub.complete();
});

return sub;
return this.storage.set(filename, keys.toJS());
}
}
4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@
"@types/codemirror": "~0.0.40",
"@types/core-decorators": "^0.10.30",
"@types/d3": "^4.4.0",
"@types/electron-json-storage": "0.0.19",
"@types/extract-zip": "~1.6.2",
"@types/hammerjs": "^2.0.33",
"@types/inflection": "^1.5.28",
Expand All @@ -111,8 +110,8 @@
"@types/node": "~8.0.2",
"@types/strip-json-comments": "~0.0.28",
"add-asset-html-webpack-plugin": "^1.0.2",
"awesome-typescript-loader": "^3.2.1",
"angular2-template-loader": "~0.6.2",
"awesome-typescript-loader": "^3.2.1",
"codelyzer": "~3.1.1",
"concurrently": "^3.0.0",
"copy-webpack-plugin": "^4.0.1",
Expand Down Expand Up @@ -173,7 +172,6 @@
"core-decorators": "~0.19.0",
"d3": "~4.10.0",
"download": "~6.2.5",
"electron-json-storage": "~3.0.4",
"element-resize-detector": "^1.1.9",
"extract-text-webpack-plugin": "~2.1.0",
"extract-zip": "~1.6.5",
Expand Down
5 changes: 4 additions & 1 deletion test/app/services/account.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,16 @@ describe("AccountService", () => {
let account1 = new AccountResource({ id: "account-1" } as any);
let subscriptionServiceSpy;
let subs: Subscription[] = [];
let storageSpy;

beforeEach(() => {
subscriptionServiceSpy = {
cache: new DataCache<any>(),
};

accountService = new AccountService({} as any, subscriptionServiceSpy);
storageSpy = {};

accountService = new AccountService(storageSpy, {} as any, subscriptionServiceSpy);
accountService.getOnce = jasmine.createSpy("getOnce").and.returnValue(Observable.of(account1));
accountService.getAccountKeys = jasmine.createSpy("getAccountKeys").and.returnValue(Observable.of({}));
subs.push(accountService.currentAccountId.subscribe(x => currentAccountId = x));
Expand Down
Loading