Add ApiVersion coercion_modes and fetch_known_versions from Shopify #600

jtgrenz · 2019-08-08T20:36:21Z

This PR allows us to dynamically define api_versions based on the response from https://app.shopify.com/services/apis.json

This removes the need to manually updated the gem every time a new version is released or set to end of life.

When the gem is required, we define a set of known versions from the unauthenticated services/apis.json endpoint and everything behaves more or less like it did before. If an unknown version is attempted to be set (ie as of today, 2020-04 or taco-42) an UnknownVersion error is raised with all known versions as message.

If for some reason the known_version_set cannot be initialized at start, then we default to assuming the version is valid, and rely on the server to handle the request and return an error if necessary. Developers can retry defining the version set by calling ShopifyAPI::ApiVersion.define_known_versions at anytime. Any version that was fetched from the server will have persisted? return true, and any on the fly version instances would return false.

ApiVersion attributes were solidified internally at shopify I think after version support was added to the gem, so I also deprecated and aliased methods like #name and #stable? to #handle and #supported?

lib/shopify_api/api_version.rb

lib/shopify_api/meta.rb

README.md

tanema

I have a couple questions but no show stoppers

README.md

lib/shopify_api/meta.rb

lib/shopify_api/api_version.rb

tanema · 2019-08-09T13:21:24Z

lib/shopify_api/api_version.rb

-      @versions = {}
+      def latest_stable_version
+        warn(
+          '[DEPRECATED] ShopifyAPI::ApiVersion.latest_stable_version is deprecated and will be removed in a future version.'


Is there a recommended way to do this same thing after this method is removed? Is this just discouraged going forward?

I think we wanted to discourage anyone trying to set the version to the latest version automatically since it's likely to break apps. Bumping to the latest version should hopefully be intentional.

It can still be computed however since each ApiVersion has a latest_supported attribute, its just a little more work.

ShopifyAPI::Meta.admin_versions.find(&:latest_supported?) works at the expense of another api call, but we could also cache the versions in Meta.admin_versions

latest_supported_version is used by the ShopifyApp gem https://github.com/Shopify/shopify_app/blob/0900ddcb2f268f6efe3e184739ee83eeab3a0020/lib/generators/shopify_app/install/install_generator.rb#L12 to run the generators.

It can now use ShopifyAPI::Meta.admin_versions.find(&:latest_supported?) to find that info.

jtgrenz · 2019-08-12T20:32:12Z

updated per @alexaitken 's comments on slack. ApiVersion now supports two coercion modes. The default will just accept whatever handle you give it and leave it to the server to deal with any invalid versions, probably what you would want in production since you should have everything tested already.

The second stricter mode will not allow you to set any version that is not pre-defined. Setting ApiVersion.coercion_mode = :predefined_only will require you to fetch the known versions from Shopify with ApiVersion.define_known_versions. Theres no more remote request when requiring the gem and any developers who want to pre-define versions on boot can do so in an initializer.

Little more flexibility for different developers for different environments.

Readme still needs to be updated and this needs a rebase

lib/shopify_api/api_version.rb

jtgrenz · 2019-08-15T15:49:47Z

Just pushed an update and rebased against master.

I made ShopifyAPI::ApiVersion a PORO so its more light weight like Alex suggested. This now means that ShopifyAPI::Meta.admin_versions no longer returns an array of ShopifyAPI::ApiVersions but instead falls back to using ActiveResource magic and returns an array of ShopifyAPI::Meta::Versions instead.

When using ShopifyAPI::ApiVersion.define_known_versions, we still grab the array of versions from Meta but we then build and cache new lighter weight ApiVersion objects from the fetched ShopifyAPI::Meta::Version.attributes hash. Instead of using persisted? to verify if it was fetched from Shopify or just defined at runtime, you would use verified?

eapache

A few minor comments, and the README needs updating, but approach LGTM.

eapache · 2019-08-15T17:20:01Z

lib/shopify_api/api_version.rb


-    def self.define_version(version)
-      @versions ||= {}
+      def define_known_versions


I might call this fetch_known_versions to be explicit that it involves a remote lookup

Yeah I think I had that at first and at some point changed it back to minimize the number of changes or something? 🤔 It sorta made sense at the time, but I think you're right that fetch is a better choice.

Not sure if I should just rename this or alias it and issue a depreciation warning

eapache · 2019-08-15T17:20:38Z

lib/shopify_api/api_version.rb


-      @versions[version.name] = version
-    end
+      def clear_defined_versions


clear_known_versions?

same concern as define/fetch_known_versions

lib/shopify_api/meta.rb

lib/shopify_api/api_version.rb

lib/shopify_api/meta.rb

README.md

eapache · 2019-08-16T16:04:21Z

README.md

+ShopifyAPI::ApiVersion.fetch_known_versions
+```
+
+Known versions are fetched and cached from https://app.shopify.com/services/apis.json. Trying to use a version outside this set will raise an error. To switch back to naïve lookup and create a version if its not found, call `ShopifyAPI::ApiVersion.version_lookup_mode = :define_on_unknown` (this is the default mode).


We should maybe mention that this set currently-supported APIs only

well it includes release candidate as well which is unsupported.

jtgrenz · 2019-08-16T16:31:00Z

Just pushed an update that cleans up a few things. I suppose this is a good time to summarize the whole change again.

New

Adds ShopifyAPI::Meta. This Meta class inherits from ActiveResource::Base and not ShopifyAPI::Base. It returns read only information about our Apis and their versions. It fetches data from https://app.shopify.com/services/apis.json and does not use authentication.
Adds apis.json and api_versions.json test fixtures.
Adds MetaTestCase to test that Meta.admin_versions returns an array of admin versions.

Changed

ShopifyAPI::ApiVersion

Class methods

replaces self. class methods with class << self style methods.
adds .versions method to return hash of known versions in { handle: version.attributes } format.
adds .version_lookup_mode and .version_lookup_mode= which changes how we look up versions from the know versions list. Valid options are :raise_on_unknown and :define_on_unknown
adds .find_version which returns a version from the known versions set or instantiates a new one and adds it to the set (based on version_lookup_mode)
adds .fetch_known_versions which looks up version info from server via ShopifyAPI::Meta and adds them to the known version set.
adds .add_to_known_versions which adds a version to the known version set
adds private .sanitize_known_versions which removes any version from the known set that is not verified.
adds private .unknown_version_error_message which returns an error message for an unknown version when version_lookup_mode is :raise_on_unknown.
Adds ApiVersion.initialize. Accepts an attributes hash argument. handle: is required, all other attributes default to false.

Instance Methods

redefines #construct_api_path and #construct_graphql_path to use the version handle in the path.
adds #handle_as_date method to convert a version handle into a date for comparison. If handle == :unstable, the date is set to the year 3000 so its always in the future.
adds #unstable? which is true if handle == 'unstable'
adds accessors for all attributes.
adds #== equality method. Returns true if the version handles between two objects match.

Misc

deletes ShopifyAPI::DefinedVersions
removes ShopifyAPI::ApiVersion.define_known_versions from shopify_api.rb
updates calls to deprecated methods to use new methods.
updates readme.

Deprecated

ApiVersion

.coerce_to_version deprecated. use .find_version
.define_known_versions deprecated. Use .fetch_known_versions
.clear_defined_versions deprecated. Use. .clear_known_versions
.latest_stable_version deprecated. Use ShopifyAPI::Meta.admin_versions.find(&:latest_supported)
#name deprecated. Use #handle
#stable? deprecated. Use #supported?.

Removed

Removed ShopifyAPI::ApiVersion::Release and ShopifyAPI::ApiVersion::Unstable. Theres nothing really special about these that require another class.

tylerball

This approach is great, after living with API Versioning for a while I think it's clear we should leave it up to developers to define which versions they're using.

Related to this, I'm not sure developers will use predefined_only, as its a more involved setup and extra request when they could just set and forget. I'm okay with it being available though, as it could be useful in our tooling and shopify_apps generators as discussed.

alexaitken

We can close #603 and go with this setup.

should this be a breaking change that requires a major version bump @nwtn @tylerball

jtgrenz · 2019-08-29T14:13:44Z

sweet. I'll fix these merge conflicts and 🚢

jtgrenz · 2019-08-29T16:15:34Z

fixed merge conflicts and rebased after merging #605. I added a new commit to add more methods to ApiVersion::NullVersion so it raises a more useful error if you try to call any of the new methods added to ApiVersion (like handle, display_name, verified?, etc).

fix errors

minor renames

method renames and minior refactor rebase cleanup

jtgrenz · 2019-09-03T14:17:43Z

Forgot about this on friday. Rebased again and it should be good to merge as soon as CI is done.

@tylerball did we want to release this as version 8.0.0? Is there anything else we need to include in a major version bump?

jtgrenz requested review from tylerball and alexaitken August 8, 2019 20:36

jtgrenz requested a review from a team as a code owner August 8, 2019 20:36

eapache reviewed Aug 9, 2019

View reviewed changes

lib/shopify_api/api_version.rb Outdated Show resolved Hide resolved

lib/shopify_api/api_version.rb Outdated Show resolved Hide resolved

lib/shopify_api/meta.rb Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

tanema approved these changes Aug 9, 2019

View reviewed changes

jtgrenz mentioned this pull request Aug 12, 2019

subclass collection and don't override AR collection #602

Merged

alexaitken reviewed Aug 12, 2019

View reviewed changes

lib/shopify_api/api_version.rb Outdated Show resolved Hide resolved

alexaitken mentioned this pull request Aug 14, 2019

[Breaking] Remove defined versions from gem. #603

Closed

jtgrenz force-pushed the auto-define-api-versions branch from a17c1f5 to 18ac855 Compare August 15, 2019 15:03

eapache reviewed Aug 15, 2019

View reviewed changes

jtgrenz changed the title ~~dynamically define api version from server at boot~~ Add ApiVersion coercion_modes and fetch_known_versions from Shopify Aug 15, 2019

jtgrenz mentioned this pull request Aug 15, 2019

Coercers with remote version fetch #606

Closed

eapache reviewed Aug 16, 2019

View reviewed changes

lib/shopify_api/meta.rb Outdated Show resolved Hide resolved

jtgrenz force-pushed the auto-define-api-versions branch 2 times, most recently from 02b828a to 66e9427 Compare August 16, 2019 15:58

eapache approved these changes Aug 16, 2019

View reviewed changes

jtgrenz force-pushed the auto-define-api-versions branch from 66e9427 to 815e7e0 Compare August 16, 2019 16:38

jtgrenz requested a review from tanema August 16, 2019 20:07

jtgrenz requested a review from alexaitken August 26, 2019 15:55

eapache mentioned this pull request Aug 27, 2019

Dynamic API Version support Shopify/shopify_python_api#339

Closed

tanema mentioned this pull request Aug 27, 2019

Add support Collection#products endpoint #609

Merged

tylerball approved these changes Aug 27, 2019

View reviewed changes

alexaitken approved these changes Aug 28, 2019

View reviewed changes

jtgrenz mentioned this pull request Aug 29, 2019

Packaging for release 7.1.0 #613

Merged

jtgrenz force-pushed the auto-define-api-versions branch 2 times, most recently from aa71979 to 1b9b8de Compare August 29, 2019 16:13

jtgrenz added 6 commits September 3, 2019 10:15

dynamically define api version from server at boot

1e75b47

add coercion modes for apiversion validation

c76e916

fix errors

make ApiVersion a PORO

62f7a03

minor renames

update readme

8192ad2

add timeout of 5s to meta

adaf51e

method renames and minior refactor rebase cleanup

update NullVersion to raise on new ApiVersion methods

0ee8d7e

jtgrenz force-pushed the auto-define-api-versions branch from 1b9b8de to 0ee8d7e Compare September 3, 2019 14:16

jtgrenz merged commit 7c5a458 into master Sep 3, 2019

jtgrenz deleted the auto-define-api-versions branch September 3, 2019 14:39

jtgrenz temporarily deployed to rubygems September 5, 2019 14:13 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ApiVersion coercion_modes and fetch_known_versions from Shopify #600

Add ApiVersion coercion_modes and fetch_known_versions from Shopify #600

jtgrenz commented Aug 8, 2019

tanema left a comment

tanema Aug 9, 2019

jtgrenz Aug 9, 2019

alexaitken Aug 15, 2019

jtgrenz commented Aug 12, 2019 •

edited

Loading

jtgrenz commented Aug 15, 2019 •

edited

Loading

eapache left a comment

eapache Aug 15, 2019

jtgrenz Aug 15, 2019 •

edited

Loading

eapache Aug 15, 2019

jtgrenz Aug 15, 2019

eapache Aug 16, 2019

jtgrenz Aug 16, 2019

jtgrenz commented Aug 16, 2019 •

edited

Loading

tylerball left a comment

alexaitken left a comment

jtgrenz commented Aug 29, 2019

jtgrenz commented Aug 29, 2019

jtgrenz commented Sep 3, 2019

Add ApiVersion coercion_modes and fetch_known_versions from Shopify #600

Add ApiVersion coercion_modes and fetch_known_versions from Shopify #600

Conversation

jtgrenz commented Aug 8, 2019

tanema left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtgrenz commented Aug 12, 2019 • edited Loading

jtgrenz commented Aug 15, 2019 • edited Loading

eapache left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtgrenz Aug 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtgrenz commented Aug 16, 2019 • edited Loading

New

Changed

ShopifyAPI::ApiVersion

Misc

Deprecated

ApiVersion

Removed

tylerball left a comment

Choose a reason for hiding this comment

alexaitken left a comment

Choose a reason for hiding this comment

jtgrenz commented Aug 29, 2019

jtgrenz commented Aug 29, 2019

jtgrenz commented Sep 3, 2019

jtgrenz commented Aug 12, 2019 •

edited

Loading

jtgrenz commented Aug 15, 2019 •

edited

Loading

jtgrenz Aug 15, 2019 •

edited

Loading

jtgrenz commented Aug 16, 2019 •

edited

Loading