firebase / firebase-admin-go

Firebase Admin Go SDK
Apache License 2.0
1.13k stars 244 forks source link

FR: Remote config support #391

Open tedgonzalez opened 4 years ago

tedgonzalez commented 4 years ago

[REQUIRED] Step 2: Describe your environment

[REQUIRED] Step 3: Describe the problem

Hi do you have plans to support firebase remote config?

lahirumaramba commented 4 years ago

Hi Theodore! Thank you for the feature request! We have recently added Remote Config support in the Admin Node.js SDK. We plan to add the Remote Config API support across the other Admin SDK platforms, including Go. Though I cannot promise you a timeline, this issue will track our progress as we start implementation.

We also greatly appreciate contributions to our codebase, if you would like to contribute to the project by adding Remote Config support! :)

allentv commented 4 years ago

I am thinking of proposing Remote Config for a project and would like to have a working version soon. Here is a skeleton based off the work done in the Node.js version : https://github.com/firebase/firebase-admin-go/compare/dev...allentv:allen/add-remote-config?expand=1

lahirumaramba commented 4 years ago

Hi @allentv, That is great to hear! Thank you for your contribution and we really appreciate it! Let's start reviewing your PR #396 and we can go from there.

allentv commented 4 years ago

The current implementation of #396 is a 1:1 mapping of the REST API docs as shown below:

// Remote Config Client
type Client struct {
    hc        *internal.HTTPClient
    projectID string
}

// Factory function for the client
func (a *App) RemoteConfig(ctx context.Context) (*remoteconfig.Client, error)

// API response
type Response struct {
    RemoteConfig
    Etag string `json:"etag"`
}

// Fetch the config
func (c *Client) GetRemoteConfig(versionNumber string) (*Response, error)

// Update the config based on the ETag from the fetch and run optional validation
func (c *Client) UpdateRemoteConfig(eTag string, validateOnly bool) (*Response, error)

// List the versions
func (c *Client) ListVersions(options *ListVersionsOptions) (*ListVersionsResponse, error)

// Rollback a specific version
func (c *Client) Rollback(versionNumber string) (*Template, error) 

Since fetching the remote config would yield the complete list of parameter values, I propose to add the following methods:

func (c *Client) Get<Type>(key string) <Type> {}

// Examples:
func (c *Client) GetBool(key string) (bool, error) {}
func (c *Client) GetInt(key string) (int, error) {}
func (c *Client) Get(key string) (string, error) {}
...

The above category of methods would fetch the config and convert it into the specific type.

Hope this is good enough to start the discussion. Happy to answer further questions.

hiranya911 commented 4 years ago

I think the API surface should be somewhat similar to what the Node.js Admin SDK offers. To that end:

type Client struct {}

func (c *Client) Template(ctx context.Context) (*Template, error)

type Template struct {
  Conditions []Condition
  Parameters map[string]Parameter
  ParameterGroups map[string]ParameterGroup
  ETag string
  Version Version
}

type Condition struct {
  Name string
  Expression string
  TagColor TagColor
}

type Parameter struct {
  Description string
  DefaultValue ParameterValue
  ConditionalValues map[string]ParameterValue
}

type ParameterValue struct {
  ExplicitValue string
  UseInAppDefault bool
}

// and so on...
allentv commented 4 years ago

The type definitions in the PR are similar to what is in the Node.js SDK. Do I have to implement all the functions in the SDK too?

lahirumaramba commented 3 years ago

Hi @allentv !

Thank you again for your contribution! We would like to propose the following API for Go SDK. I see similarities in your implementation (#396) but there are also places where the code needs to be updated. We do not expect you to implement the complete API. :) If you can update #396 to match the corresponding parts of the API proposed below, it would make a great start towards a complete API.

The Go API for Remote Config is not something we have currently prioritized. I can't promise you a timeline but I am going to start the internal approval process for the following proposed API.

Please let me know if you have any questions.

Proposed API for Remote Config in Admin Go SDK

func (a *App) RemoteConfig(ctx context.Context) (*remoteconfig.Client, error)

package "firebase.google.com/go/v4/remoteconfig"

type Client struct {}

func (c *Client) GetTemplate(ctx context.Context) (*Template, error)
func (c *Client) GetTemplateAtVersion(ctx context.Context, versionNumber string) (*Template, error)

func (c *Client) Versions(ctx context.Context, options *ListVersionsOptions) (*VersionIterator, error)

func (c *Client) Rollback(ctx context.Context, versionNumber string) (*Template, error)

func (c *Client) PublishTemplate(ctx context.Context, template *Template) (*Template, error)
func (c *Client) ValidateTemplate(ctx context.Context, template *Template) (*Template, error)
func (c *Client) ForcePublishTemplate(ctx context.Context, template *Template) (*Template, error)
type Template struct {
  Conditions      []*Condition
  Parameters      map[string]*Parameter
  ParameterGroups map[string]*ParameterGroup
  ETag            string
  Version         *Version
}

type TagColor int

const (
  colorUnspecified TagColor = iota
  Blue
  Brown
  Cyan
  DeepOrange
  Green
  Indigo
  Lime
  Orange
  Pink
  Purple
  Teal
)

type Condition struct {
  Name       string
  Expression string
  TagColor   TagColor
}

type Parameter struct {
  Description       string
  DefaultValue      *ParameterValue
  ConditionalValues map[string]*ParameterValue
}

type ParameterValue struct {
  useInAppDefault bool
  explicitValue string
}

func UseInAppDefaultValue() *ParameterValue
func NewExplicitParameterValue(value string) *ParameterValue

type RemoteConfigParameterGroup struct {
  Description string
  Parameters  map[string]*Parameter
}

type Version struct {
  Description    string
  VersionNumber  string
  UpdateTime     time.Time
  UpdateOrigin   string
  UpdateType     string
  UpdateUser     *User
  RollbackSource string
  IsLegacy       bool
}

type VersionIterator struct {}

func (it *VersionIterator) PageInfo() *iterator.PageInfo
func (it *VersionIterator) Next() (*Version, error)

type ListVersionsOptions struct {
  StartTime        time.Time
  EndTime          time.Time
  EndVersionNumber string
  PageSize         int
  PageToken        string
}

type ListVersionsOptions struct {
  Email    string
  ImageURL string
  Name     string
}
allentv commented 3 years ago

@lahirumaramba Thanks for sharing the API spec. I have updated the PR with the above changes.

It seems like ListVersionsOptions has been repeated twice. Am I correct to assume that the last struct refers to User?

lahirumaramba commented 3 years ago

@allentv Good catch! Yes, the last struct should be:

type User struct {
  Email    string
  ImageURL string
  Name     string
}

Thank you again for updating the PR! We really appreciate your contribution!

adarshk-rapido commented 3 years ago

@lahirumaramba @allenktv thanks for this contribution, even we were looking for this, any idea when this will be part of master branch or released ? (sorry i'm unaware of the release process in this repo)

Meanwhile I'm thinking of using the firebase:remote-config branch in go.mod this should be ok right ?

Our usecase is to only work with remote-config.

lahirumaramba commented 3 years ago

Hi @adarshk-rapido, Thank you for your interest! Unfortunately, we are unable to provide a timeline for a new release with the remote config API at the moment.

You can use the remote-config branch if you would like. However, since this is still a work in progress and not officially released yet the final API surface may change.

adarshk-rapido commented 3 years ago

thank you @lahirumaramba

ghost commented 3 years ago

@lahirumaramba @allenktv I was using the remote-config branch for our use case. While using GetRemoteConfig we are facing some issue which we are assuming from the SDK side.

error while parsing response: json: cannot unmarshal string into Go struct field Condition.conditions.tagColor of type remoteconfig.TagColor

I checked with ListVersions and it's working fine. It would be very helpful someone verifies this. Thank you

allentv commented 3 years ago

@MrinalKGhosh I only did an initial implementation and then this issue was opened to discuss the API surface. So not all API endpoints would be working.