-
Notifications
You must be signed in to change notification settings - Fork 962
feat(database): enable database multi-resource support #159
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,10 +34,12 @@ let _staticInstance: RepoManager; | |
*/ | ||
export class RepoManager { | ||
/** | ||
* @private {!Object.<string, !Repo>} | ||
* @private {!Object.<string, Object<string, !fb.core.Repo>>} | ||
*/ | ||
private repos_: { | ||
[name: string]: Repo; | ||
[name: string]: { | ||
[name: string]: Repo; | ||
}; | ||
} = {}; | ||
|
||
/** | ||
|
@@ -55,14 +57,18 @@ export class RepoManager { | |
|
||
// TODO(koss): Remove these functions unless used in tests? | ||
interrupt() { | ||
for (const repo in this.repos_) { | ||
this.repos_[repo].interrupt(); | ||
for (const instance in this.repos_) { | ||
for (const repo in this.repos_[instance]) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should these be: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, changed this and the function below. |
||
this.repos_[instance][repo].interrupt(); | ||
} | ||
} | ||
} | ||
|
||
resume() { | ||
for (const repo in this.repos_) { | ||
this.repos_[repo].resume(); | ||
for (const instance in this.repos_) { | ||
for (const repo in this.repos_[instance]) { | ||
this.repos_[instance][repo].resume(); | ||
} | ||
} | ||
} | ||
|
||
|
@@ -72,8 +78,8 @@ export class RepoManager { | |
* @param {!FirebaseApp} app | ||
* @return {!Database} | ||
*/ | ||
databaseFromApp(app: FirebaseApp): Database { | ||
const dbUrl: string = app.options[DATABASE_URL_OPTION]; | ||
databaseFromApp(app: FirebaseApp, url?: string): Database { | ||
const dbUrl: string = url || app.options[DATABASE_URL_OPTION]; | ||
if (dbUrl === undefined) { | ||
fatal( | ||
"Can't determine Firebase Database URL. Be sure to include " + | ||
|
@@ -104,12 +110,15 @@ export class RepoManager { | |
* @param {!Repo} repo | ||
*/ | ||
deleteRepo(repo: Repo) { | ||
const appRepos = safeGet(this.repos_, repo.app.name); | ||
// This should never happen... | ||
if (safeGet(this.repos_, repo.app.name) !== repo) { | ||
fatal('Database ' + repo.app.name + ' has already been deleted.'); | ||
if (!appRepos || safeGet(appRepos, repo.repoInfo_.toURLString()) !== repo) { | ||
fatal( | ||
`Database ${repo.app.name}(${repo.repoInfo_}) has already been deleted.` | ||
); | ||
} | ||
repo.interrupt(); | ||
delete this.repos_[repo.app.name]; | ||
delete appRepos[repo.repoInfo_.toURLString()]; | ||
} | ||
|
||
/** | ||
|
@@ -121,12 +130,21 @@ export class RepoManager { | |
* @return {!Repo} The Repo object for the specified server / repoName. | ||
*/ | ||
createRepo(repoInfo: RepoInfo, app: FirebaseApp): Repo { | ||
let repo = safeGet(this.repos_, app.name); | ||
let appRepos = safeGet(this.repos_, app.name); | ||
|
||
if (!appRepos) { | ||
appRepos = {}; | ||
this.repos_[app.name] = appRepos; | ||
} | ||
|
||
let repo = safeGet(appRepos, repoInfo.toURLString()); | ||
if (repo) { | ||
fatal('FIREBASE INTERNAL ERROR: Database initialized multiple times.'); | ||
fatal( | ||
'Database initialized multiple times. Please make sure the format of the database URL matches with each database() call.' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternatively we could just return the existing instance. Would that cause other problems though? I guess app would end up with multiple references to it and might call INTERNAL.delete() multiple times, etc.? I guess I don't really feel strongly. It's probably not too harmful to make them pass the exact same URL every time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I don't know that I'm terribly heartbroken making users pass the extra string around, especially considering that they can still just use the project config instead. |
||
); | ||
} | ||
repo = new Repo(repoInfo, this.useRestClient_, app); | ||
this.repos_[app.name] = repo; | ||
appRepos[repoInfo.toURLString()] = repo; | ||
|
||
return repo; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you use something more specific than "name" for these? I think it's maybe appName and dbUrl or something ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, no problem.