Skip to content

Commit

Permalink
Merge pull request #8 from FirebaseExtended/storage_fixes
Browse files Browse the repository at this point in the history
* Don't cancel the upload if the task is unsubscribed from, this is unexpected behavior. `fromTask` is actually a hot-observable and the task controls it (having the cancel, pause, and resume functions) thus we should lean on the task to control itself to not surprise the developer
* As such `fromTask` should be idempotent and immediately emit the latest snapshot, as the task is effective a Subject
* Before completion `fromTask` was not emitting a `success` state, move to the promise form of the task to get that, as such `progress` was never reaching 100%
* `fromTask` should emit an `error` or `canceled` snapshot, before the observable errors out; making for easier handling of errors. `takeWhile(snap => snap.state !== 'canceled')` for instance.
* `put` and `putString` should be cold, replay, and cancel on unsubscribe however
  • Loading branch information
jamesdaniels authored May 13, 2021
2 parents e8f850b + eb0927d commit 985f4de
Show file tree
Hide file tree
Showing 22 changed files with 6,022 additions and 278 deletions.
5 changes: 5 additions & 0 deletions .firebaserc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"projects": {
"default": "rxfire-test-c497c"
}
}
16 changes: 15 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,16 @@ jobs:
restore-keys: |
${{ runner.os }}-${{ matrix.node }}-${{ matrix.firebase }}-${{ matrix.rxjs }}-node_modules-
${{ runner.os }}-${{ matrix.node }}-node_modules-
- name: test/functions node_modules cache
id: functions_node_modules_cache
uses: actions/cache@v2
with:
path: ./test/functions/node_modules
key: ${{ runner.os }}-${{ matrix.node }}-functions_node_modules-${{ hashFiles('test/functions/yarn.lock') }}
restore-keys: |
${{ runner.os }}-${{ matrix.node }}-functions_node_modules-
- name: Yarn offline cache
if: steps.node_modules_cache.outputs.cache-hit != 'true'
if: steps.node_modules_cache.outputs.cache-hit != 'true' || steps.functions_node_modules_cache.outputs.cache-hit != 'true'
uses: actions/cache@v2
with:
path: ~/.npm-packages-offline-cache
Expand All @@ -93,6 +101,12 @@ jobs:
yarn config set yarn-offline-mirror ~/.npm-packages-offline-cache
yarn install --frozen-lockfile --prefer-offline
yarn add --dev firebase@${{ matrix.firebase }} rxjs@${{ matrix.rxjs }} --prefer-offline
- name: Install test/functions deps
if: steps.functions_node_modules_cache.outputs.cache-hit != 'true'
run: |
yarn config set yarn-offline-mirror ~/.npm-packages-offline-cache
cd test/functions
yarn install --frozen-lockfile --prefer-offline
- name: Firebase emulator cache
uses: actions/cache@v2
with:
Expand Down
35 changes: 35 additions & 0 deletions firebase.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"database": {
"rules": "test/database.rules.json"
},
"firestore": {
"rules": "test/firestore.rules",
"indexes": "test/firestore.indexes.json"
},
"storage": {
"rules": "test/storage.rules"
},
"functions": {
"source": "test/functions"
},
"emulators": {
"auth": {
"port": 9099
},
"functions": {
"port": 5001
},
"firestore": {
"port": 8080
},
"database": {
"port": 9000
},
"storage": {
"port": 9199
},
"ui": {
"enabled": true
}
}
}
25 changes: 0 additions & 25 deletions karma.conf.js

This file was deleted.

12 changes: 8 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"build:rollup": "rollup -c",
"build:docs": "cp README.md ./dist/ && cp -r ./docs ./dist/",
"dev": "rollup -c -w",
"test": "jest"
"test": "FIREBASE_CLI_PREVIEWS=storageemulator firebase emulators:exec --project=rxfire-test-c497c \"jest\""
},
"dependencies": {
"tslib": "^1.9.0 || ~2.1.0"
Expand All @@ -62,18 +62,22 @@
"@typescript-eslint/eslint-plugin": "^4.13.0",
"@typescript-eslint/parser": "^4.13.0",
"babel-jest": "^26.6.3",
"cross-fetch": "^3.1.4",
"eslint": "^7.17.0",
"eslint-config-google": "^0.14.0",
"firebase": "^8.2.2",
"firebase": "^8.6.1",
"firebase-tools": "^9.10.2",
"glob": "^7.1.6",
"jest": "^26.6.3",
"md5": "^2.3.0",
"npm-run-all": "^4.1.5",
"rollup": "^2.33.2",
"rollup-plugin-generate-package-json": "^3.2.0",
"rollup-plugin-uglify": "^6.0.4",
"rxjs": "^7.0.0",
"tslib": "~2.1.0",
"typescript": "^4.2.4"
"tslib": "^1.9.0 || ~2.1.0",
"typescript": "^4.2.4",
"xhr2": "^0.2.1"
},
"files": [
"dist/**/*",
Expand Down
2 changes: 1 addition & 1 deletion rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { sync as globSync } from 'glob';
import { readFileSync } from 'fs';
import generatePackageJson from 'rollup-plugin-generate-package-json';

const packageJsonPaths = globSync('**/package.json', { ignore: ['node_modules/**', 'dist/**'] });
const packageJsonPaths = globSync('**/package.json', { ignore: ['node_modules/**', 'dist/**', 'test/**'] });
const packages = packageJsonPaths.reduce((acc, path) => {
const pkg = JSON.parse(readFileSync(path, { encoding: 'utf-8'} ));
const component = dirname(path);
Expand Down
73 changes: 51 additions & 22 deletions storage/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
*/

import firebase from 'firebase/app';
import {Observable, from} from 'rxjs';
import {map} from 'rxjs/operators';
import { Observable, from } from 'rxjs';
import { debounceTime, map, shareReplay } from 'rxjs/operators';

type UploadTaskSnapshot = firebase.storage.UploadTaskSnapshot;
type Reference = firebase.storage.Reference;
Expand All @@ -27,15 +27,38 @@ type UploadTask = firebase.storage.UploadTask;
type Data = Blob | Uint8Array | ArrayBuffer;

export function fromTask(
task: firebase.storage.UploadTask,
task: UploadTask
): Observable<UploadTaskSnapshot> {
return new Observable<UploadTaskSnapshot>((subscriber) => {
return new Observable<UploadTaskSnapshot>(subscriber => {
const progress = (snap: UploadTaskSnapshot): void => subscriber.next(snap);
const error = (e: Error): void => subscriber.error(e);
const complete = (): void => subscriber.complete();
task.on('state_changed', progress, error, complete);
return () => task.cancel();
});
// emit the current state of the task
progress(task.snapshot);
// emit progression of the task
const unsubscribeFromOnStateChanged = task.on('state_changed', progress);
// use the promise form of task, to get the last success snapshot
task.then(
snapshot => {
progress(snapshot);
setTimeout(() => complete(), 0);
},
e => {
progress(task.snapshot);
setTimeout(() => error(e), 0);
}
);
// the unsubscribe method returns by storage isn't typed in the
// way rxjs expects, Function vs () => void, so wrap it
return function unsubscribe() {
unsubscribeFromOnStateChanged();
};
}).pipe(
// since we're emitting first the current snapshot and then progression
// it's possible that we could double fire synchronously; namely when in
// a terminal state (success, error, canceled). Debounce to address.
debounceTime(0)
);
}

export function getDownloadURL(ref: Reference): Observable<string> {
Expand All @@ -49,32 +72,38 @@ export function getMetadata(ref: Reference): Observable<any> {
}

export function put(
ref: Reference,
data: Data,
metadata?: UploadMetadata,
ref: Reference,
data: Data,
metadata?: UploadMetadata,
): Observable<UploadTaskSnapshot> {
return fromTask(ref.put(data, metadata));
return new Observable<UploadTaskSnapshot>(subscriber => {
const task = ref.put(data, metadata);
return fromTask(task).subscribe(subscriber).add(task.cancel);
}).pipe(shareReplay({ bufferSize: 1, refCount: true }));
}

export function putString(
ref: Reference,
data: string,
format?: StringFormat,
metadata?: UploadMetadata,
ref: Reference,
data: string,
format?: StringFormat,
metadata?: UploadMetadata
): Observable<UploadTaskSnapshot> {
return fromTask(ref.putString(data, format, metadata));
return new Observable<UploadTaskSnapshot>(subscriber => {
const task = ref.putString(data, format, metadata);
return fromTask(task).subscribe(subscriber).add(task.cancel);
}).pipe(shareReplay({ bufferSize: 1, refCount: true }));
}

export function percentage(
task: UploadTask,
task: UploadTask
): Observable<{
progress: number;
snapshot: UploadTaskSnapshot;
}> {
return fromTask(task).pipe(
map((s) => ({
progress: (s.bytesTransferred / s.totalBytes) * 100,
snapshot: s,
})),
map(s => ({
progress: (s.bytesTransferred / s.totalBytes) * 100,
snapshot: s
}))
);
}
}
55 changes: 55 additions & 0 deletions test/auth.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/**
* @license
* Copyright 2018 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/* eslint-disable @typescript-eslint/no-floating-promises */

import firebase from 'firebase/app';
import 'firebase/auth';
import { default as TEST_PROJECT, authEmulatorPort } from './config';

const rando = (): string => Math.random().toString(36).substring(5);

describe('RxFire Auth', () => {
let app: firebase.app.App;
let auth: firebase.auth.Auth;

/**
* Each test runs inside it's own app instance and the app
* is deleted after the test runs.
*
* Each test is responsible for seeding and removing data. Helper
* functions are useful if the process becomes brittle or tedious.
* Note that removing is less necessary since the tests are run
* against the emulator.
*/
beforeEach(() => {
app = firebase.initializeApp(TEST_PROJECT, rando());
auth = app.auth();
auth.useEmulator(`http://localhost:${authEmulatorPort}`);
});

afterEach(() => {
app.delete().catch();
});

describe('fromTask', () => {
it('should work', () => {
expect('a').toEqual('a');
})
});

});
8 changes: 8 additions & 0 deletions test/config.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import * as firebaseConfig from '../firebase.json';

export default {
apiKey: 'AIzaSyCD1LqWoxivr0hu7YJ_xF6WyAT4_l-Aw0I',
authDomain: 'rxfire-test-c497c.firebaseapp.com',
Expand All @@ -8,3 +10,9 @@ export default {
appId: '1:498378360465:web:4223d73f0f7dd0aa3d6130',
measurementId: 'G-K35MQE7EN2',
};

export const authEmulatorPort = firebaseConfig.emulators.auth.port;
export const databaseEmulatorPort = firebaseConfig.emulators.database.port;
export const firestoreEmulatorPort = firebaseConfig.emulators.firestore.port;
export const storageEmulatorPort = firebaseConfig.emulators.storage.port;
export const functionsEmulatorPort = firebaseConfig.emulators.functions.port;
6 changes: 6 additions & 0 deletions test/database.rules.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"rules": {
".read": true,
".write": true
}
}
Loading

0 comments on commit 985f4de

Please sign in to comment.