uploadcare / uploadcare-ruby

Ruby API client that handles uploads and further operations with files by wrapping Uploadcare Upload and REST APIs.
https://uploadcare.com
MIT License
39 stars 28 forks source link

update User-Agent header format (v2) #49

Closed vizvamitra closed 6 years ago

vizvamitra commented 6 years ago

I believe nobody is using Uploadcare::USER_AGENT constant and Uploadcare::user_agent method, so I've deprecated them.

I also think that I should backport this changes to uploadcare-ruby 1.x (updating patch version) so that uploadcare-rails 1.x users could get this update

Usage example:

### Basic example

client = Uploadcare::Client.new(
  public_key: '1234567890',
  # ...
)

client.file('2ce49769-ac35-48e1-a1f3-d00ac86cf2fb')
# User-Agent: UploadcareRuby/2.0.1/1234567890 (Ruby/3.4.0)

### User-Agent can be extended with framework and extension information

client = Uploadcare::Client.new(
  public_key: '1234567890',
  # ...
  user_agent_environment: {
    framework_name: 'Rails',
    framework_version: '5.1.0',
    extension_name: 'UploadcareRails',
    extension_version: '1.1.0',
  }
)

client.file('2ce49769-ac35-48e1-a1f3-d00ac86cf2fb')
# User-Agent: UploadcareRuby/2.0.1/1234567890 (Ruby/3.4.0; Rails/5.1.0; UploadcareRails/1.1.0)

### User can override User-Agent string if he wants

client = Uploadcare::Client.new(
  public_key: '1234567890',
  # ...
  user_agent: 'I do not want so send my statistics'
)

client.file('2ce49769-ac35-48e1-a1f3-d00ac86cf2fb')
# User-Agent: I do not want so send my statistics
coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.5%) to 99.493% when pulling 73ebc62264425d50b7c27002947b3109401447f2 on vizvamitra:feature/update_user_agent_format into 8e7a5ffb6363b1a4dff8bb7379ad8750a8eef32c on uploadcare:master.

dmitry-mukhin commented 6 years ago
  1. yes, we need to backport it to v1
  2. I'm not sure about removing user_agent method though. It seems like it belongs to a Client/API instance.
vizvamitra commented 6 years ago

@dmitry-mukhin nope, Uploadcare module is just a namespace for other logics and API client is an instance of Uploadcare::Api class.

The problem with this method is that it is public, and so it is a part of the gem's public interface. Every part of a public interface should be somehow useful for gem users, but how can this method be useful? I believe it was meant for internal usage only, was occasionally made public and now pollutes gem's public interface.

Same thing with Uploadcare::USER_AGENT constant. Why would gem user ever want to use it in his code?

dmitry-mukhin commented 6 years ago

We don't need the constant. But again, I think getUserAgent (and/or setUserAgent) semantically should be a method of Uploadcare::Api class. Agree that it shouldn't be just in the namespace

vizvamitra commented 6 years ago

@dmitry-mukhin Uploadcare::API's responsibility is to handle API calls. User-Agent is a lower-level detail and thus it belongs to a lower-level class. ApiConnection/UploadConnection classes that handle connection details and they are the two places where User-Agent header is actually being set, but the format of this header itself is an even more lower-level. I thought that it has enough logics in its own to create a class that'll encapsulate this logics.

Ideally an instance of this class should be injected into ApiConnection/UploadConnection during the initialization (as it is an external dependency), but it seems that this kind of design doesn't fit well to our current code.

vizvamitra commented 6 years ago

@dmitry-mukhin check out the changes please