Implemented the API for releasing rulesets #610
Conversation
|
Under what circumstances would we expect to get a |
|
I'm not really sure -- just going by intuition here. If these releases are guaranteed to always exist, we can certainly get rid of the createRelease logic and further simplify the implementation. There is also a deleteRelease operation supported on the REST API, which I haven't played with. What happens if somebody deletes a release (perhaps accidentally)? |
|
My mental model is that creating a release is basically "enabling" the service (e.g., linking a GCS bucket up to work with Firebase Storage), and deleting the release is the inverse (e.g., removing the bucket, disabling client access). |
|
Fair enough. I've removed the createRelease functionality from the PR. |
| readonly createTime: string; | ||
| readonly updateTime: string; | ||
| readonly createTime?: string; | ||
| readonly updateTime?: string; |
There was a problem hiding this comment.
Is there a reason to expect that we won't get the create and updateTimes back, or you're just signaling that we don't rely on them for anything?
There was a problem hiding this comment.
I use the same interface as an input argument when updating/patching a release. In this case I wanted the flexibility to not specify these 2 attributes. And you're right -- we also don't rely on them for anything else.
src/security-rules/security-rules.ts
Outdated
|
|
||
| /** | ||
| * Releases the specified ruleset as the current Cloud Firestore ruleset. This makes the specified ruleset | ||
| * the currently applied ruleset for Cloud Firestore. |
There was a problem hiding this comment.
Total nit; seems like the second sentence is more complete and could stand on its own here.
|
|
||
| private releaseRuleset(ruleset: string | RulesetMetadata, releaseName: string): Promise<void> { | ||
| if (!validator.isNonEmptyString(ruleset) && | ||
| (!validator.isNonNullObject(ruleset) || !validator.isNonEmptyString(ruleset.name))) { |
There was a problem hiding this comment.
It's interesting that the name could come in either as a String or as part of the RulesetMetadata Object. Why did you opt for this and not the simpler signature of only passing in the name as a String, making it the caller's job to pluck out the name if it had an Object?
There was a problem hiding this comment.
We can use the same private method for releasing storage rulesets. By doing it here we save a few lines from being duplicated.
* Implementing the Firebase Security Rules API (#604) * Implementing the Firebase Security Rules API * More argument validation and assertions * Cleaning up the rules impl * Internal API renamed * Fixing a typo in a comment * Implemented createRulesFileFromSource() and createRuleset() APIs (#607) * Implementing the Firebase Security Rules API * More argument validation and assertions * Adding the rest of the CRUD operations for rulesets * Cleaning up the rules impl * Cleaned up tests * Adding some missing comments * Removing support for multiple rules files in create() * Implemented the deleteRuleset() API (#609) * Added deleteRuleset API * Merged with source * Implemented the API for releasing rulesets (#610) * Implemented the API for releasing rulesets * Removed createRelease logic * Updated comment * Added the getStorageRuleset() API (#613) * Implemented the API for releasing rulesets * Removed createRelease logic * Added getStorageRules() API * Removed some redundant tests * Implementing the remaining releaseRuleset APIs (#616) * Implemented the listRulesetMetadata() API (#622) * Adding the rules API to the public API surface (#625) * Added rules API to the public admin namespace * Updated docs * Addressing comments regarding the d.ts file * Updated App typings * Rules integration tests (#633) * Rules integration tests * Refactored by adding some helper methods * Cleaned up some conditionals * Added verification for deleteRuleset test * Renamed tempRulesets * Handling ruleset limit exceeded error (#636) * Fixing alignment of an annotation * Updated comments
3 participants
Implemented the
releaseFirestoreRuleset()API. This attempts to PATCH the existing release.