[WIP] Adding support for subcollections and related subresources #61

abellotti · 2017-01-30T23:05:03Z

Dynamically driven by the "subcollections" exposed via OPTIONS /api/:collection
supports queries and actions

Queries:
miq.vms.find(166).tags.collect(&:name)
miq.vms.find(166).tags.select(:categorization).collect(&:categorization)

Subcollection Actions:
miq.vms.find(166).tags.assign(:name => "/managed/location/ny")
miq.vms.find(166).tags.assign([{:name => "/managed/location/chicago"}, {:name => "/managed/cc/001"}])

Subcollection resource actions:
miq.vms.find(166).tags.find(32).unassign
miq.vms.find(166).tags.where(:name => "/managed/location/*").collect(&:unassign)

Fixes: #31
Fixes: #32

abellotti · 2017-01-30T23:05:25Z

abellotti · 2017-02-08T18:47:41Z

@Fryguy subcollection support + lotsa tests added

abellotti · 2017-04-05T21:16:01Z

ping @Fryguy for review, will be needed shortly for Automate work. Thanks.

abellotti · 2017-09-05T15:42:14Z

Ping @Fryguy we need this for Automate Workspace work by @mkanoor

Fryguy · 2017-09-06T17:27:25Z

lib/manageiq/api/client/mixins/collection_action_mixin.rb

+
+  ACTIONS_RETURNING_RESOURCES = %w(create query).freeze
+
+  def exec_action(name, *args, &block)


Should any of these be private?

Fryguy · 2017-09-06T17:28:34Z

lib/manageiq/api/client/mixins/collection_action_mixin.rb

@@ -0,0 +1,50 @@
+module CollectionActionMixin


This needs to be namespaced

Fryguy · 2017-09-06T17:28:53Z

lib/manageiq/api/client/mixins/queryable_mixin.rb

@@ -0,0 +1,95 @@
+module QueryableMixin


Also needs namespace

Fryguy · 2017-09-06T17:29:03Z

lib/manageiq/api/client/mixins/resource_action_mixin.rb

@@ -0,0 +1,24 @@
+module ResourceActionMixin


Fryguy · 2017-09-06T17:29:51Z

lib/manageiq/api/client/subcollection.rb

+            const_get(name, false)
+          else
+            const_set(name, Class.new(self))
+          end


It feels like there is a lot of duplication with collection.

- Dynamically driven by the "subcollections" exposed via OPTIONS /api/:collection - supports queries and actions Queries: miq.vms.find(166).tags.collect(&:name) miq.vms.find(166).tags.select(:categorization).collect(&:categorization) Subcollection Actions: miq.vms.find(166).tags.assign(:name => "/managed/location/ny") miq.vms.find(166).tags.assign([{:name => "/managed/location/chicago"}, {:name => "/managed/cc/001"}]) Subcollection resource actions: miq.vms.find(166).tags.find(32).unassign miq.vms.find(166).tags.where(:name => "/managed/location/*").collect(&:unassign) Fixes: ManageIQ#31 Fixes: ManageIQ#32

- Moving action specific action methods for resources and subresources to seperate mixin. - Moving action specific action methods for collections and subcollections to seperate mixin. - Moving common queryable methods for collections and subcollections to a common QueryableMixin

- Updated options response fixture to include subcollections - Added collection rspecs - Added resource rspecs

- Add tests for subcollections

- Adding resource actions_vms.json - Adding subcollection actions_vm_tags.json

- Adding tests for subresources

with PR# ManageIQ#77

moved to the common collection mixin

Fryguy · 2018-04-03T14:20:51Z

lib/manageiq/api/client/mixins/action_mixin.rb

@@ -1,4 +1,4 @@
-module ActionMixin
+module ManageIQ::API::ActionMixin


Should this be ManageIQ::API::Client::ActionMixin? Otherwise it might collide with some future mixin in ManageIQ itself.

good point 👍 will update all.

Fryguy · 2018-04-03T14:37:14Z

lib/manageiq/api/client/mixins/queryable_mixin.rb

+  end
+
+  def filters_from_query_relation(condition, option)
+    filters = []


I'm sure there's a way to turn this into a simpler .collect

Fryguy · 2018-04-03T14:38:47Z

lib/manageiq/api/client/mixins/queryable_mixin.rb

+      if option.kind_of?(Array)
+        option.each_with_object({}) { |name, hash| hash[name] = "asc" }
+      else
+        option.dup


What is this dup for if you're going to loop over it anyway?

parameters_from_query_relation, filters_from_query_relation and order_parameters_from_query_relation which really live in the mixins/queryable_mixin.rb

…ons.

Fryguy · 2018-04-03T18:50:16Z

lib/manageiq/api/client/mixins/queryable_mixin.rb

+        option.each_with_object({}) { |name, hash| hash[name] = "asc" }
+      else
+        option
+      end


I feel like you can remove this completely if you just deal with defaulting nil values below.

[1] pry(main)> option = {"a" => "asc", "b" => "desc"} [2] pry(main)> option.each {|k, v| puts "#{k} => #{v || "asc"}"} a => asc b => desc [3] pry(main)> option = ["a", "b"] [4] pry(main)> option.each {|k, v| puts "#{k} => #{v || "asc"}"} a => asc b => asc

Fryguy · 2018-04-03T18:51:28Z

lib/manageiq/api/client/mixins/queryable_mixin.rb

+      res_sort_by << sort_attr
+      sort_order =
+        case sort_order
+        when /^asc/i  then "asc"


so this could be when /^asc/i, nil then "asc"

Fryguy · 2018-04-03T18:52:41Z

@abellotti This is a lot to digest +3,715 −135...might need to sit with you unless you can break it up into easily digestable PRs

abellotti · 2018-04-03T18:52:51Z

Marked as WIP, I need to revisit Subresource (/api/providers/1/vms/133 vs just /api/vms/133)

abellotti · 2018-04-12T20:15:04Z

Agreed, getting slowly morphed/simplified as https://github.com/abellotti/manageiq-api-client/tree/enhanced_subcollection_support. will create new PR when in good shape. By simply
using resource.rb (eliminating subresource.rb and possibly subcollection.rb), this would now allow for deeply nested resource/subcollection/action calls as we'd reset at the primary CI URL where actions are provided. So allowing for the following:

miq.providers.find_by(:name => "test-provider).vms.find_by(:name => "test-vm").snapshots.collect(&:name)

abellotti · 2018-04-26T18:34:19Z

closing in preference of #80

abellotti added the enhancement label Jan 30, 2017

abellotti force-pushed the adding_subcollection_support branch 4 times, most recently from 75e8f0f to 5f2ddc8 Compare February 7, 2017 16:19

abellotti force-pushed the adding_subcollection_support branch 3 times, most recently from 7954eb1 to 8a554b9 Compare February 8, 2017 16:04

abellotti changed the title ~~[WIP] Adding support for subcollections and related subresources~~ Adding support for subcollections and related subresources Feb 8, 2017

abellotti assigned Fryguy Feb 8, 2017

abellotti force-pushed the adding_subcollection_support branch 2 times, most recently from c23fa2f to d9d208c Compare February 8, 2017 21:40

abellotti mentioned this pull request Feb 13, 2017

[WIP] Adding support for search_options on collection or subcollection #63

Open

abellotti mentioned this pull request Feb 28, 2017

change subcollections check ManageIQ/manageiq-api-client-python#21

Merged

Fryguy reviewed Sep 6, 2017

View reviewed changes

Fryguy requested changes Sep 6, 2017

View reviewed changes

abellotti force-pushed the adding_subcollection_support branch from d9d208c to b98cf9d Compare September 6, 2017 20:06

abellotti added 9 commits April 2, 2018 14:47

Adding rspecs - options, collections and resources

58456c7

- Updated options response fixture to include subcollections - Added collection rspecs - Added resource rspecs

Adding rspecs - subcollections

86572d3

- Add tests for subcollections

Rspecs - Updated test to used action specific responses

1e4c70e

- Adding resource actions_vms.json - Adding subcollection actions_vm_tags.json

Adding rspecs - subresources

2ba97ed

- Adding tests for subresources

Minor Alignment changes, leveraging subclass for collection spec.

52e3387

Updated PR specs to reflect the latest query enhancements made

a720dab

with PR# ManageIQ#77

Assure that all mixins are properly namespaced

4ee881d

Common methods each and self.subclass for collections and subcollections

f2c4814

moved to the common collection mixin

abellotti force-pushed the adding_subcollection_support branch from b98cf9d to f2c4814 Compare April 2, 2018 22:52

This was referenced Apr 2, 2018

[WIP] Resource performance fix #79

Open

Simply getting a subresource should return its actions ManageIQ/manageiq-api#357

Closed

Fryguy reviewed Apr 3, 2018

View reviewed changes

abellotti added 4 commits April 3, 2018 11:09

Fully qualify the mixins modules as ManageIQ::API::Client::*

640628c

Simplified filters_from_query_relation method.

0e28739

Removed duplicated code from collection.rb. Those methods were

b374164

parameters_from_query_relation, filters_from_query_relation and order_parameters_from_query_relation which really live in the mixins/queryable_mixin.rb

Simplifying order_parameters_from_query_relation. no need to dup opti…

d9d6859

…ons.

abellotti changed the title ~~Adding support for subcollections and related subresources~~ [WIP] Adding support for subcollections and related subresources Apr 3, 2018

Fryguy reviewed Apr 3, 2018

View reviewed changes

abellotti closed this Apr 26, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Adding support for subcollections and related subresources #61

[WIP] Adding support for subcollections and related subresources #61

abellotti commented Jan 30, 2017

abellotti commented Jan 30, 2017 •

edited

Loading

abellotti commented Feb 8, 2017

abellotti commented Apr 5, 2017

abellotti commented Sep 5, 2017

Fryguy Sep 6, 2017

Fryguy Sep 6, 2017

Fryguy Sep 6, 2017

Fryguy Sep 6, 2017

Fryguy Sep 6, 2017

Fryguy Apr 3, 2018

abellotti Apr 3, 2018

Fryguy Apr 3, 2018

Fryguy Apr 3, 2018

Fryguy Apr 3, 2018 •

edited

Loading

Fryguy Apr 3, 2018

Fryguy commented Apr 3, 2018

abellotti commented Apr 3, 2018

abellotti commented Apr 12, 2018

abellotti commented Apr 26, 2018


		ACTIONS_RETURNING_RESOURCES = %w(create query).freeze

		def exec_action(name, *args, &block)

		@@ -1,4 +1,4 @@
		module ActionMixin
		module ManageIQ::API::ActionMixin

[WIP] Adding support for subcollections and related subresources #61

[WIP] Adding support for subcollections and related subresources #61

Conversation

abellotti commented Jan 30, 2017

abellotti commented Jan 30, 2017 • edited Loading

abellotti commented Feb 8, 2017

abellotti commented Apr 5, 2017

abellotti commented Sep 5, 2017

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

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

Fryguy Apr 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fryguy commented Apr 3, 2018

abellotti commented Apr 3, 2018

abellotti commented Apr 12, 2018

abellotti commented Apr 26, 2018

abellotti commented Jan 30, 2017 •

edited

Loading

Fryguy Apr 3, 2018 •

edited

Loading