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

Add public config #1

Closed

Conversation

thecoolwinter
Copy link

What kind of change does this PR introduce?

  • Adds a StorageApiConfig struct with the public properties
    • url
    • headers
  • Modifies StorageApi, StorageBucketApi and SupabaaseStorageClient to use the new config object.
  • Adds an initializer to each above class to initialize directly with a config object.

What is the current behavior?

Config variables (url, headers) aren't available to a public API. This leads to problems when dealing with auth changes and other changing things that may need to change headers after initalization.

What is the new behavior?

Config can be changed in various ways

// Add an authorization header
client.config.headers["Authorization"] = "Bearer: 1234"

// Change the base url
client.config.url = "app.supabase.io"

// Remove an authorization header
client.config.headers["Authorization"] = nil

// Initialize a client using a config object
let config = StorageApiConfig(url: "app.supabase.io", headers: [:])

let client = SupabaseStorageClient(config)

Other notes

This is also a modification related to the changes on this PR on postgrest-swift.

@satishbabariya
Copy link
Member

@thecoolwinter this will differ from js and dart lib implementations, JS and Dart

@thecoolwinter
Copy link
Author

@satishbabariya That is true, and both those libraries keep their confit variables private, only allowing them to be set at initialization. However, there is a need to be able to set and change these config variables such as in an auth situation.

When changing a user or a user session ends, the headers need to be updated for the next requests. If we simply replace the object by initializing it again it opens it up the possibility of memory leaks within user's code due to the SupabaseStorage client being deinitialized but potentially has running tasks still going.

Instead the variables should be public and mutable.

Additionally the rationale for moving them to an object is just that it simplifies the API so that any other params that need to be set can be added to that config struct, it also reduces the number of public variables and methods on the API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants