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

docs: Design section for the managing Firestore data. #1150

Closed
wants to merge 3 commits into from
Closed
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
46 changes: 43 additions & 3 deletions metrics/cli/docs/04_managing_firestore_data.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,17 @@ The `dartbase_admin` package requires the following parameters:

2. The JSON of the service account's key, which can be downloaded using the following command:

```bash
gcloud iam service-accounts keys create key.json --iam-account [email protected]
```
```bash
gcloud iam service-accounts keys create key.json --iam-account [email protected]
```

_**Note**: The above-described key is a `private key` for the chosen service account, which requires additional secure factors when working with it.
Therefore, to make sure the user does not need to worry about managing created `private key`, the CLI tool should deactivate the `key` after finishing work with it using the following command:_

```bash
gcloud iam service-accounts keys delete KEY-ID [email protected]
```


Example of creating document using dart package:

Expand Down Expand Up @@ -112,3 +120,35 @@ Cons:
### Decision

As we've analyzed above, the [Cloud Functions](#using-cloud-functions-for-firebase) does not allow us to automate the Cloud Firestore data managing, so we should choose the [Dart Package](#using-dart-package) since it provides a more clean and fully automated way of managing Cloud Firestore data.

## Design

The following sections provide an implementation of managing Firestore data integration into the Metrics CLI tool.

Let's take a look on the main classes the managing Firestore data integration requires.

### FirestoreConfigRepository

The `FirestoreConfigRepository` is a repository, which provides methods for managing the [feature config](https://github.com/platform-platform/monorepo/blob/master/docs/19_security_audit_document.md#the-feature_config-collection)
and [allowed domains](https://github.com/platform-platform/monorepo/blob/master/docs/19_security_audit_document.md#the-allowed_email_domains-collection) collections data.
To implement these methods, the repository using the [dartbase_admin package](https://pub.dev/packages/dartbase_admin) as described in the [decision](#decision).

### FirebaseService
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it would be great to create a separate Service for Firestore to make it more SRP. The FirebaseService will be responsible for configuring the Firebase project, and the FirestoreService will be responsible for managing the Firestore Database data. What do you think?


The `FirebaseService` is an existing service interface, in which we should add the following methods required for this integration:
- `seedFirestoreData` - seeds Firestore database with required data;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to name this method something like setInitialData or addIntialData?

- `initAdminFirestore` - initializes Firestore database with Admin privileges.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the FirebaseService shouldn't know anything about implementation details, so it should not know anything about Firestore Admin i believe.


### GcloudService

The `GcloudService` is an existing service interface, in which we should add the following methods required for this integration:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to add more context here about why do we need these methods. The same for other sections, I believe it would be great to have a bit more context.

- `downloadServiceKey` - downloads a service account's key;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- `downloadServiceKey` - downloads a service account's key;
- `createServiceKey` - downloads a service account's key;

Does it make sense since it seems like this method will create the key firstly and then download it?

- `deactivateServiceKey` - deactivates a service account's key.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add any methods to the GcloudCli class to implement these methods in the GcloudCliServiceAdapter?


### FirebaseServiceAdapter

The `FirebaseServiceAdapter` is an adapter for the [`FirebaseService`](#FirebaseService), which implements new added methods using the [`FirestoreConfigRepository`](#FirestoreConfigRepository) and the [dartbase_admin package](https://pub.dev/packages/dartbase_admin).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we mention somewhere here that we should move the FeatureConfig and FeatureConfigData to the core package?

Here is a class diagram, which demonstrates the structure and relationships of classes the managing Firestore data integration requires:

![Managing Firestore Data class diagram](http://www.plantuml.com/plantuml/proxy?cache=no&fmt=svg&src=https://github.com/platform-platform/monorepo/raw/design_doc_firestore_data/metrics/cli/docs/diagrams/managing_firestore_data_class_diagram.puml)
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
@startuml firestore_managing_data_class_diagram

package deploy {
class Deployer {
- _fileHelper: FileHelper
- _npmService : NpmService
- _gitService : GitService
- _flutterService : FlutterService
- _gcloudService : GCloudService
- _firebaseService : FirebaseService
+ deploy() : Future<void>
}
}

package firebase {
package service as firebase.service {
interface FirebaseService {
+ login(): Future<String>
+ addProject(String gcloudProjectId): Future<void>
+ initAdminFirestore(String projectId, String servicePath) : Future<void>
+ seedFirestoreData() : Future<void>
+ deployHosting(String appPath): Future<void>
+ deployFirestore(String firestorePath): Future<void>
}
}

package adapter as firebase.adapter {
class FirebaseServiceAdapter {
- _firebaseCli: FirebaseCli
- _firestoreConfigRepository: FirestoreConfigRepository
- _ prompter: Prompter
}
}

package repository as firebase.repository {
class FirestoreConfigRepository {
- _firestore : Firestore
+ addAllowedDomain(String domain) : Future<void>
+ setFeatureConfig(bool isPasswordSignInOptionEnabled, bool isDebugMenuEnabled): Future<void>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this class have a method used to init the FirebaseAdmin SDK?

}
}
}

package core {
package data.model {
class FeatureConfigData {
+ Map<String, dynamic> toJson()
+ factory fromJson(Map<String, dynamic> json)
}
}

package domain.entities {
class FeatureConfig {
+ isPasswordSignInOptionEnabled : bool
+ isDebugMenuEnabled : bool
}
}
}

package dartbase_admin {}

package gcloud {
interface GCloudService {
+ login() : Future<void>
+ createProject() : Future<String>
+ downloadServiceKey(String path, String serviceEmail) : Future<void>
+ deactivateServiceKey(String path) : Future<void>
}
}

Deployer --> FirebaseServiceAdapter : uses
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Deployer --> FirebaseServiceAdapter : uses
Deployer --> FirebaseService : uses

?

Deployer --> GCloudService : uses

FeatureConfigData --|> FeatureConfig

FirebaseServiceAdapter ..|> FirebaseService
FirebaseServiceAdapter --> FirestoreConfigRepository : uses
FirebaseServiceAdapter -up-> dartbase_admin : uses

FirestoreConfigRepository -up-> dartbase_admin : uses
FirestoreConfigRepository --> FeatureConfigData : uses

@enduml