Refactor: Perform internal rename and add comments to FilterTable (#3047)

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
This commit is contained in:
Clinton Wolfe 2018-06-04 20:20:59 -04:00 committed by GitHub
parent 8f57ec7824
commit 8c274daaa9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 268 additions and 193 deletions

View file

@ -4,10 +4,10 @@ If you just want to _use_ FilterTable, see filtertable-usage.md . Reading this
## What makes this hard?
The terminology used for many concepts does not help the reader understand what is going on. Additionally, the ways in which the classes relate is not straightforward. Finally, variable names within the classes are often re-used ('filter' is a favorite) or are too short to be meaningful (x and c are both used as variable names, in long blocks).
FilterTable was created in 2016 in an attempt to consolidate the pluralization features of several resources. They each had slightly different featuresets, and were all in the wild, so FilterTable exposes some extensive side-effects to provide those features.
Additionaly, the ways in which the classes relate is not straightforward.
## Where is the code?
The main FilterTable code is in [utils/filter.rb](https://github.com/chef/inspec/blob/master/lib/utils/filter.rb).
@ -24,9 +24,9 @@ This class is responsible for the definition of the filtertable. It provides th
FilterTable::Factory initializes three instance variables:
```
@accessors = []
@connectors = {}
@resource = nil
@filter_methods = []
@custom_properties = {}
@resource = nil # This appears to be unused
```
### FilterTable::Table
@ -43,10 +43,11 @@ A resource class using FilterTable typically will call a sequence similar to thi
```
filter = FilterTable.create
filter.add_accessor(:entries)
.add(:exists?) { |x| !x.entries.empty? }
.add(:thing_ids, field: :thing_id)
filter.connect(self, :table)
filter.register_filter_method(:entries)
.register_custom_property(:exists?) { |x| !x.entries.empty? }
.register_custom_property(:count) { |x| x.entries.count }
.register_custom_property(:thing_ids, field: :thing_id)
filter.install_filter_methods_on_resource(self, :table)
```
Each of those calls supports method chaining.
@ -55,26 +56,39 @@ Each of those calls supports method chaining.
Returns a blank instance of a FilterTable::Factory.
### add\_accessor
### register\_filter\_method
Suggested alternate name: register_chainable_filter_method(:new_method_name)
Legacy name (alias): `add_accessor`
This simply pushes the provided method name onto the `@accessors` instance variable array. See "accessor" behavior section below for what this does.
This simply pushes the provided method name onto the `@filter_methods` instance variable array. See "filter_method" behavior section below for what this does.
After adding the method name to the array, it returns `self` - the FilterTable::Factory instance - so that method chaining will work.
### add
### register\_column
Suggested alternate name 1: register_property_or_matcher_and_filter_criterion
Suggested alternate name 2: register_connector
Legacy name (alias): `add`
This is one of the most confusingly named methods. `add` requires a symbol (which will be used as a method name _to be added to the resource class_), then also accepts a block and/or additional args. These things - name, block, and opts - are packed into a simple struct called a Connector. The name stored in the struct will be `opts[:field]` if provided, and the method name if not.
This is currently simply an alias for `register_custom_property`. See it for details. By calling it with a distinctive name, we'll be able to add functionality in the future (especially around introspection).
The Connector struct is then appended to the Hash `@connectors`, keyed on the method name provided. `self` is then returned for method chaining.
### register\_custom\_matcher
Legacy name (alias): `add`
This is currently simply an alias for `register_custom_property`. See it for details. By calling it with a distinctive name, we'll be able to add functionality in the future (especially around introspection).
### register\_custom\_property
Legacy name (alias): `add`
This method has very complex behavior, ans should likely be split into several use cases. `register_custom_property` requires a symbol (which will be used as a method name _to be added to the resource class_), then also accepts a block and/or additional args. These things - name, block, and opts - are packed into a simple struct called a CustomPropertyType. The name stored in the struct will be `opts[:field]` if provided, and the method name if not.
The CustomPropertyType struct is then appended to the Hash `@custom_properties`, keyed on the method name provided. `self` is then returned for method chaining.
The implementation of the custom property method is generated by `create_custom_property_body`, and varies based on whether a block was provided to `register_custom_property`.
#### Behavior when a block is provided
This behavior is implemented by line 256.
This behavior is implemented by lines 298-304.
If a block is provided, it is turned into a Lambda and used as the method body.
@ -82,7 +96,7 @@ The block will be provided two arguments (though most users only use the first):
1. The FilterTable::Table instance that wraps the raw data.
2. An optional value used as an additional opportunity to filter.
For example, this is common:
For example, this is common in legacy code:
```
filter.add(:exists?) { |x| !x.entries.empty? }
```
@ -103,7 +117,7 @@ If you provide _both_ a block and opts, only the block is used, and the options
#### Behavior when no block is provided
If you do not provide a block, you _must_ provide a `:field` option (though that does no appear to be enforced). The behavior is to define a method with the name provided, that has a conditional return type. The method body is defined in lines 258-266.
If you do not provide a block, you _must_ provide a `:field` option (though that does no appear to be enforced). The behavior is to define a method with the name provided, that has a conditional return type. The method body is defined in lines 306-319.
If called without arguments, it returns an array of the values in the raw data for that column.
```
@ -124,94 +138,88 @@ POSSIBLE BUG: I think this case is broken; it certainly seems ill-advised.
#### Known Options
You can provide options to `add`, after the desired method name.
You can provide options to `register_custom_property` / `add`, after the desired method name.
##### field
This is the most common option. It selects an implementation in which the desired method will be defined such that it returns an array of the row values using the specified key. In other words, this acts as a "column fetcher", like in SQL: "SELECT some_column FROM some_table"
Internally, (line 195-200), a Struct type is created to represent a row of raw data. The struct's attribute list is taken from the `field` options passed to `add`.
Internally, (line 212-217), a Struct type is created to repressent a row of raw data. The struct's attribute list is taken from the `field` options passed to `register_custom_property` / `add`. This new type is strored as `row_eval_context_type`. It is used as the evaluation context for block-mode `where` calls.
* No checking is performed to see if the field name is actually a column in the raw data (the raw data hasn't been fetched yet, so we can't check).
* You can't have two `add` calls that reference the same field, because the Struct would see that as a duplicate attribute.
* You can't have two `register_custom_property` / `add` calls that reference the same field, because the Struct would see that as a duplicate attribute.
POSSIBLE BUG: We could deduplicate the field names when defining the Struct, thus allowing multiple properties to use the same field.
##### type
##### style
`type: :simple` has been seen in the wild. When you call the method like `things.thing_ids => Array`, this has the very useful effect of flattening and uniq'ing the returned array.
The `style` option is intended to effect post-processing of the return value from the generated method. To date there is only one recognized value, `:simple`, which flattens, uniq's, and compact's the array value of the property. This is implemented on line 312.
No other values for `:type` have been seen.
No other values for `:style` have been seen.
### connect
### install_filter_methods_on_resource
Suggested alternate name: install_filtertable
Legacy name (alias): connect
This method is called like this:
```
filter.connect(self, :data_fetching_method_name)
filter.install_filter_methods_on_resource(self, :data_fetching_method_name)
```
`filter` is an instance of FilterTable::Factory. `self` is a reference to the resource class you are authoring. `data_fetching_method_name` is a symbol, the name of a method that will return the actual data to be processed by the FilterTable - as an array of hashes.
Note that 'connect' does not refer to Connectors.
`add` and `add_accessor` did nothing other than add register names for methods that we'd like to have added to the resource class. No filtering ability is present, nor are the methods defined, at this point.
`register_custom_property` and `register_filter_method` did nothing other than add register names for methods that we'd like to have added to the resource class. No filtering ability is present, nor are the methods defined, at this point.
So, `connect`'s job is to actually install everything.
So, `install_filter_methods_on_resource`/`connect`'s job is to actually install everything.
#### Re-pack the "connectors"
First, on lines 188-192, the list of custom methods ("connectors", registered using the `add` method) are repacked into an array of arrays of two elements - the desired method name and the lambda that will be used as the method body. The lambda is created by the private method `create_connector`.
TBD: what exactly create_connector does
First, on lines 188-192, the list of custom properties ("connectors", registered using the `register_custom_property` / `add` method) are repacked into an array of hashes of two elements - the desired method name and the lambda that will be used as the method body. The lambda is created by the private method `create_custom_property_body`; see `register_custom_property` for discussion about how the implementtation behaves.
#### Defines a special Struct type to represent rows in the table
At lines 195-200, a new Struct type is defined, with attributes for each of the known table fields. The motivation for this struct type is to implement the block-mode behavior of `where`. Because each struct represents a row, and it has the attributes (accessors) for the fields, block-mode `where` is implemented by instance-evaling against each row as a struct.
At lines 212-217, a new Struct type `row_eval_context_type` is defined, with attributes for each of the known table fields. The motivation for this struct type is to implement the block-mode behavior of `where`. Because each struct represents a row, and it has the attributes (accessors) for the fields, block-mode `where` is implemented by instance-evaling against each row as a struct.
Additionally, an instance variable, `@__filter` is defined, with an accessor(!). (That's really weird - double-underscore usually means "intended to be private"). `to_s` is implemented, using `@__filter`, or `super` if not defined. I guess we then rely on the `Struct` class to stringify?
Additionally, an instance variable, `@criteria_string` is defined, with an accessor. `to_s` is implemented, using `@criteria_string`, or `super` if not defined. I guess we then rely on the `Struct` class to stringify.
I think `@__filter` is a trace - a string indicating the filter criteria used to create the table. I found no location where this per-row trace data was used.
CONFUSING NAME: `@__filter` meaning a trace of criteria operations is very confusing - the word "filter" is very overloaded.
`@criteria_string` is a trace - a string indicating the filter criteria used to create the table. I found no location where this per-row trace data was used.
Table fields are determined by listing the `field_name`s of the Connectors.
BUG: this means that any `add` call that uses a block but not options will end up with an attribute in the row Struct. Thus, `filter.add(:exists?) { ... }` results in a row Struct that includes an attribute named `exists?` which may be undesired.
BUG: this means that any `register_custom_property` / `add` call that uses a block but not options will end up with an attribute in the row Struct. Thus, `filter.add(:exists?) { ... }` results in a row Struct that includes an attribute named `exists?` which may be undesired. This attribute will never have a value, because when the structs are instantiated, the block for the field is not called.
POSSIBLE MISFEATURE: Defining a Struct for rows means that people who use `entries` (or other data accessors) interact with something unusual. The simplest possible thing would be an Array of Hashes. There is likely something relying on this...
#### Subclass FilterTable::Table into an anonymous class
At line 203, create the local var `table`, which refers to an anonymous class that subclasses FilterTable::Table. The class is opened and two groups of methods are defined.
At line 224, create the local var `table_class`, which refers to an anonymous class that subclasses FilterTable::Table. The class is opened and two groups of methods are defined.
Lines 204-206 install the "connector" methods, using the names and lambdas determined on line 188.
Lines 226-228 install the "custom_property" methods, using the names and lambdas determined on line 219.
Line 208-213 define a method, `new_entry`. Its job is to append a row to the FilterTable::Table as a row Struct, given a plain Hash (presumably as provided by the data fetching method) and a String piece of tracking information (again, confusingly referred to as "filter").
Line 231-236 define a method, `create_eval_context_for_row`. This is used when executing a block-mode `where`; see line 117.
#### Install methods on the resource
Lines 216-232 install the data table accessors and "connector" methods onto the resource that you are authoring.
Lines 245-256 install the "filter_methods" and "custom properties" methods onto the resource that you are authoring.
Line 222-223 collects the names of the methods to define - by agglomerating the names of the data table accessors and "connector" methods. They are treated the same.
Line 245-246 collects the names of the methods to define - by agglomerating the names of the "filter_methods" and "custom properties" methods. They are treated the same.
Line 224 uses `send` with a block to call `define_method` on the resource class that you're authoring. Using a block with `send` is undocumented, but is treated as an implicit argument (per stackoverflow) , so the end result is that the block is used as the body for the new method being defined.
Line 247 uses `send` with a block to call `define_method` on the resource class that you're authoring. Using a block with `send` is undocumented, but is treated as an implicit argument (per stackoverflow) , so the end result is that the block is used as the body for the new method being defined.
The method body is wrapped in an exception-catching facility that catches skipped or failed resource exceptions and wraps them in a specialized exception catcher class. TBD: understand this better.
Line 226 constructs an instance of the anonymous FilterTable::Table subclass defined at 203. It passes three args:
Line 250 constructs an instance of the anonymous FilterTable::Table subclass defined at 224. It passes three args:
1. `self`. TBD: which class is this referring to at this point?
2. The return value of calling the data fetcher method.
1. `self`. A reference to the resource instance.
2. The return value of calling the data fetcher method (that is an array of hashes, the raw data).
3. The string ' with', which is probably informing the criteria stringification. The extra space is intentional, as it follows the resource name: 'my_things with color == :red' might be a result.
And that new FilterTable::Table subclass instance is stored in a local variable, named, confusingly, "filter".
On line 227, we then immediately call a method on that "FilterTable::Table subclass instance". The method name is the same as the one we're defining on the resource - but we're calling it on the Table. Recall we defined all the "custom_property" methods on the Table subclass at line 226-228. The method gets called with any args or block passed, and since it's the last thing, it provides the return value.
On line 227, we then immediately call a method on that "FilterTable::Table subclass instance". The method name is the same as the one we're defining on the resource - but we're calling it on the Table. Recall we defined all the "connector" methods on the Table subclass at line 204-206. The method gets called with any args or block passed, and since it's the last thing, it provides the return value.
VERY WORRISOME THING: So, the Table subclass has methods for the "connectors" (for example, `thing_ids` or `exist?`. What about the "accessors" - `where` and `entries`? Are those in the FilterTable::Table class, or method_missing'd?
VERY WORRISOME THING: So, the Table subclass has methods for the "custom_properties" (for example, `thing_ids` or `exist?`. What about the "filter_methods" - `where` and `entries`? Are those in the FilterTable::Table class, or method_missing'd?
## What is its behavior? (FilterTable::Table)
@ -226,6 +234,18 @@ Assume that your resource has a method, `fetch_data`, which returns a fixed arra
```
Assume that you then perform this sequence in your resource class body:
```
filter = FilterTable.create
filter.register_filter_method(:entries)
filter.register_filter_method(:where)
filter.register_custom_property(:exists?) { |x| !x.exists.empty? }
filter.register_custom_property(:names, field: :name)
filter.install_filter_methods_on_resource(self, :fetch_data)
```
Legacy code equivalent:
```
filter = FilterTable.create
filter.add_accessor(:entries)
@ -235,16 +255,16 @@ filter.add(:names, field: :name)
filter.connect(self, :fetch_data)
```
We know from the above exploration of `connect` that we now have several new methods on the resource class, all of which delegate to the FilterTable::Table implementation.
We know from the above exploration of `install_filter_methods_on_resource` / `connect` that we now have several new methods on the resource class, all of which delegate to the FilterTable::Table implementation.
### FilterTable::Table constructor and internals
Factory calls the FilterTable::Table constructor at 142-144 with three args. Table stores them into instance vars:
* @resource - this was passed in as `self`; I think this would be the resource class
* @params - the raw table data. (confusing name)
* @filters - This looks to be stringification trace data; the string ' with' was passed in by Factory.
Factory calls the FilterTable::Table constructor at 87-92 with three args. Table stores them into instance vars:
* @resource_instance - this was passed in as `self` from line 250
* @raw_data - an array of hashes
* @criteria_string - This looks to be stringification trace data; the string ' with' was passed in by Factory.
params and filters get `attr_reader`s.
All three get exposed via `attr_reader`s.
### `entries` behavior
@ -252,13 +272,13 @@ From usage, I expect entries to return a structure that resembles an array of ha
#### A new method `entries` is defined on the resource class
That is performed by Factory#connect line 224.
That is performed by Factory#connect line 247.
#### It delegates to FilterTable::Table#entries
This is a real method defined in filter.rb line 120.
This is a real method defined in filter.rb line 142.
It loops over the provided raw data (@params) and builds an array, calling `new_entry` (see Factory 208-213) on each row; also appending a stringification trace to each entry. The array is returned.
It loops over the provided raw data (@raw_data) and builds an array, calling `create_eval_context_for_row` (see Factory 231-236) on each row; also appending a stringification trace to each entry. The array is returned.
#### `entries` conclusion
@ -266,19 +286,21 @@ Not Surprising: It does behave as expected - an array of hashlike structs repres
Surprising: this is a real method with a concrete implementation. That means that you can't call `filter.add_accessor` with arbitrary method names - `:entries` means something very specific.
Surpising: I would not recommend this method be used for data access; instead I would recommend using `raw_data`.
### `where` behavior
From usage, I expect this to take either method params or a block (both of which are magical), perform filtering, and return some object that contains only the filtered rows.
So, what happens when you call `add_accessor(:where)` and then call `resource.where`?
So, what happens when you call `register_filter_method(:where)` and then call `resource.where`?
#### A new method `where` is defined on the resource class
That is performed by Factory#connect line 224.
That is performed by Factory#connect line 247.
#### It delegates to FilterTable::Table#where
Like `entries`, this is a real implemented method on FilterTable::Table, at line 93.
Like `entries`, this is a real implemented method on FilterTable::Table, at line 97.
The method accepts all params as the local var `conditions` which defaults to an empty Hash. A block, if any, is also explicitly assigned the name `block`.
@ -288,23 +310,23 @@ MISFEATURE: The first guard clause simply returns the Table if `conditions` is n
The second guard clause is a sensible degenerate case - return the existing Table if there are no conditions and no block. So `thing.where` is OK.
Line 97 initializes a local var, `filters`, which again is a stringification tracker.
Line 102 initializes a local var, `new_criteria_string`, which again is a stringification tracker.
Line 98 confuses things further. A local var `table` is initialized to the value of instance var `@params`. The naming here is poorly chosen - @params is the initial raw data (and it has an attr_reader, no need for the @); and `table` is the new _raw data_ - not a new FilterTable class or instance.
Line 103 initializes a var to track the `filtered_raw_data`.
Lines 99-102 loop over the provided Hash `conditions`. It repeatedly downfilters `table` by calling the private method `filter_lines` on it. `filter_lines` does some syntactic sugaring for common types, Ints and Floats and Regexp matching. Additionally, the 99-102 loop builds up the stringification tracker, `filter`, by stringifying the field name and target value.
Lines 107-110 loop over the provided Hash `conditions`. It repeatedly downfilters `filtered_raw_data` by calling the private method `filter_raw_data` on it. `filter_raw_data` does some syntactic sugaring for common types, Ints and Floats and Regexp matching. Additionally, the 107-110 loop builds up the stringification tracker, `new_criteria_string`, by stringifying the field name and target value.
BUG: (Issue 2943) - Lines 99-102 do not validate the names of the provided fields.
BUG: (Issue 2943) - Lines 107-110 do not validate the names of the provided fields.
Line 104-109 begins work if a filtration block has been provided. At this point, `table` has been initialized with the raw data, and (if method params were provided) has also been filtered down.
Line 115-122 begins work if a filtration block has been provided. At this point, `filtered_raw_data` has been initialized with the raw data, and (if method params were provided) has also been filtered down.
Line 105 filters the rows of the raw data using an odd approach: each row is evaluated using `Array#find_all`, with the block `{ |e| new_entry(e, '').instance_eval(&block) }` `new_entry` wraps each raw row in a Struct (the `''` indicates we're not trying to save stringification data here). (Recall that new_entry was defined by `FilterTable::Factory#connect`, line 195). Then the provided block is `instance_eval`'d against the Struct. Because the Struct was defined with attributes (that is, accessor methods) for each declared field name (from FilterTable::Factory#add), you can use field names in the block, and each row-as-struct will be able to respond.
Line 117 filters the rows of the raw data using an interesting approach. Each row is inflated to a Struct using `create_eval_context_for_row` (see line 231). Then the provided block is `instance_eval`'d against the Struct. Because the Struct was defined with attributes (that is, accessor methods) for each declared field name (from FilterTable::Factory#register_custom_property), you can use field names in the block, and each row-as-struct will be able to respond.
_That just explained a major spooky side-effect for me._
Lines 106-108 do something with stringification tracing. TODO.
Lines 119-121 do something with stringification tracing. TODO.
Finally, at line 111, the FilterTable::Table anonymous subclass is again used to construct a new instance, passing on the resource reference, the newly filtered raw data table, and the newly adjusted stringification tracer.
Finally, at line 124, the FilterTable::Table anonymous subclass is again used to construct a new instance, passing on the resource reference, the newly filtered raw data table, and the newly adjusted stringificatioon tracer.
That new Table instance is returned, and thus `where` allows you to chain.
@ -313,4 +335,3 @@ That new Table instance is returned, and thus `where` allows you to chain.
Unsurprising: How where works with method params.
Surprising: How where works in block mode, instance_eval'ing against each row-as-Struct.
Surprising: You can use method-mode and block-mode together if you want.
Problematic: Many confusing variable names here. A lot of clarity could be gained by simple search and replace.

View file

@ -35,15 +35,15 @@ class Thing < Inspec.resource(1)
# FilterTable setup
filter_table_config = FilterTable.create
filter_table_config.add_accessor(:where)
filter_table_config.add_accessor(:entries)
filter_table_config.add(:exist?) { |filter_table| !filter_table.entries.empty? }
filter_table_config.add(:count) { |filter_table| filter_table.entries.count }
filter_table_config.add(:thing_ids, field: :thing_id)
filter_table_config.add(:colors, field: :color, type: :simple)
filter_table_config.connect(self, :fetch_data)
def fetch_data
filter_table_config.register_filter_method(:where)
filter_table_config.register_filter_method(:entries)
filter_table_config.register_custom_matcher(:exist?) { |filter_table| !filter_table.entries.empty? }
filter_table_config.register_custom_property(:count) { |filter_table| filter_table.entries.count }
filter_table_config.register_column(:thing_ids, field: :thing_id)
filter_table_config.register_column(:colors, field: :color, style: :simple)
filter_table_config.install_filter_methods_on_resource(self, :fetch_data)
def fetch_data
# This method should return an array of hashes - the raw data. We'll hardcode it here.
[
{ thing_id: 1, color: :red },
@ -61,9 +61,9 @@ end
Note that all of the methods on `filter_table_config` support chaining, so you will sometimes see it as:
```ruby
filter_table_config.add_accessor(:where)
.add_accessor(:entries)
.add(:exist?) { |filter_table| !filter_table.entries.empty? }
filter_table_config.register_filter_method(:where)
.register_filter_method(:entries)
.register_custom_matcher(:exist?) { |filter_table| !filter_table.entries.empty? }
```
etc.
@ -136,7 +136,7 @@ The filtering is fancy, not just straight equality.
### A `where` method you can call with a block, referencing some fields
You can also call the `where` method with a block. The block is executed row-wise. If it returns truthy, the row is included in the results. Additionally, within the block each field declared with the `add` configuration method is available as a data accessor.
You can also call the `where` method with a block. The block is executed row-wise. If it returns truthy, the row is included in the results. register_custom_propertyitionally, within the block each field declared with the `register_custom_property` configuration method is available as a data accessor.
```ruby
@ -145,7 +145,7 @@ You can also call the `where` method with a block. The block is executed row-wis
its('count') { should cmp 3 }
end
# You can access any field you declared using `add`
# You can access any fields you declared using `register_column`
describe things.where { thing_id > 2 } do
its('count') { should cmp 1 }
end
@ -166,7 +166,7 @@ Some other methods return a Table object, and they may be chained without a re-f
### An `entries` method that will return an array of Structs
The other `add_accessor` call enables a pre-defined method, `entries`. `entries` is much simpler than `where` - in fact, its behavior is unrelated. It returns an encapsulated version of the raw data - a plain array, containing Structs as row-entries. Each struct has an attribute for each time you called `add`.
The other `register_filter_method` call enables a pre-defined method, `entries`. `entries` is much simpler than `where` - in fact, its behavior is unrelated. It returns an encapsulated version of the raw data - a plain array, containing Structs as row-entries. Each struct has an attribute for each time you called `register_column`.
Overall, in my opinion, `entries` is less useful than `params` (which returns the raw data). Wrapping in Structs does not seem to add much benefit.
@ -202,9 +202,9 @@ If you call `entries` without chaining it after `where`, calling entries will tr
### You get an `exist?` matcher defined on the resource and the table
This `add` call:
This `register_custom_matcher` call:
```ruby
filter_table_config.add(:exist?) { |filter_table| !filter_table.entries.empty? }
filter_table_config.register_custom_matcher(:exist?) { |filter_table| !filter_table.entries.empty? }
```
causes a new method to be defined on both the resource class and the Table class. The body of the method is taken from the block that is provided. When the method it called, it will receive the FilterTable::Table instance as its first parameter. (It may also accept a second param, but that doesn't make sense for this method - see thing_ids).
@ -225,9 +225,9 @@ As when you are implementing matchers on a singular resource, the only thing tha
### You get an `count` property defined on the resource and the table
This `add` call:
This `register_custom_property` call:
```ruby
filter_table_config.add(:count) { |filter_table| filter_table.entries.count }
filter_table_config.register_custom_property(:count) { |filter_table| filter_table.entries.count }
```
causes a new method to be defined on both the resource class and the Table class. As with `exists?`, the body is taken from the block.
@ -246,12 +246,12 @@ causes a new method to be defined on both the resource class and the Table class
### A `thing_ids` method that will return an array of plain values when called without params
This `add` call:
This `register_column` call:
```ruby
filter_table_config.add(:thing_ids, field: :thing_id)
filter_table_config.register_column(:thing_ids, field: :thing_id)
```
will cause a method to be defined on both the resource and the Table. Note that this `add` call does not provide a block; so FilterTable::Factory generates a method body. The `:field` option specifies which column to access in the raw data (that is, which hash key in the array-of-hashes).
will cause a method to be defined on both the resource and the Table. Note that this `register_column` call does not provide a block; so FilterTable::Factory generates a method body. The `:field` option specifies which column to access in the raw data (that is, which hash key in the array-of-hashes).
The implementation provided by Factory changes behavior based on calling pattern. If no params or block is provided, a simple array is returned, containing the column-wise values in the raw data.
@ -284,7 +284,7 @@ The implementation provided by Factory changes behavior based on calling pattern
### A `colors` method that will return a flattened and uniq'd array of values
This method behaves just like `thing_ids`, except that it returns the values of the `color` column. In addition, the `type: :simple` option causes it to flatten and uniq the array of values when called without args or a block.
This method behaves just like `thing_ids`, except that it returns the values of the `color` column. In addition, the `style: :simple` option causes it to flatten and uniq the array of values when called without args or a block.
```ruby
# Three rows in the data: red, blue, red
@ -298,9 +298,9 @@ This method behaves just like `thing_ids`, except that it returns the values of
### A `colors` method that can filter on a value and return a Table
You also get this for `thing_ids`. This is unrelated to `type: :simple` for `colors`.
You also get this for `thing_ids`. This is unrelated to `style: :simple` for `colors`.
People definitely use this in the wild. It reads badly to me; I think this is a legacy usage that we should consider deprecating. To me, this seems to imply that there is a sub-resource (here, colors) we are auditing.
People definitely use this in the wild. It reads badly to me; I think this is a legacy usage that we should consider deprecating. To me, this seems to imply that there is a sub-resource (here, colors) we are auditing. At least two core resouces (`xinetd_conf` and `users`) advocate this as their primary use.
```ruby
# Filter on colors
@ -316,7 +316,7 @@ People definitely use this in the wild. It reads badly to me; I think this is a
### A `colors` method that can filter on a block and return a Table
You also get this for `thing_ids`. This is unrelated to `type: :simple` for `colors`.
You also get this for `thing_ids`. This is unrelated to `style: :simple` for `colors`.
I haven't seen this used in the wild, but its existence gives me a headache.
@ -353,12 +353,12 @@ I haven't seen this used in the wild, but its existence gives me a headache.
end
```
### You can call `params` on any Table to get the raw data
### You can call `params` or `raw_data` on any Table to get the raw data
People _definitely_ use this out in the wild. Unlike `entries`, which wraps each row in a Struct and omits undeclared fields, `params` simply returns the actual raw data array-of-hashes. It is not `dup`'d.
People _definitely_ use this out in the wild. Unlike `entries`, which wraps each row in a Struct and omits undeclared fields, `raw_data` simply returns the actual raw data array-of-hashes. It is not `dup`'d.
```ruby
tacky_things = things.where(color: :blue).params.select { |row| row[:tackiness] }
tacky_things = things.where(color: :blue).raw_data.select { |row| row[:tackiness] }
tacky_things.map { |row| row[:thing_id] }.each do |thing_id|
# Use to audit a singular Thing
describe thing(thing_id) do
@ -367,13 +367,13 @@ People _definitely_ use this out in the wild. Unlike `entries`, which wraps each
end
```
### You can call `resource` on any Table to get the resource instance
### You can call `resource_instance` on any Table to get the resource instance
You could use this to do something fairly complicated.
```ruby
describe things.where do # Just getting a Table object
its('resource.some_method') { should cmp 'some_value' }
its('resource_instance.some_method') { should cmp 'some_value' }
end
```
@ -381,7 +381,7 @@ However, the resource instance won't know about the filtration, so I'm not sure
## Gotchas and Surprises
### Methods defined with `add` will change their return type based on their call pattern
### Methods defined with `register_column` will change their return type based on their call pattern
To me, calling things.thing_ids should always return the same type of value. But if you call it with args or a block, it not only runs a filter, but also changes its return type to Table.
@ -402,13 +402,13 @@ To me, calling things.thing_ids should always return the same type of value. Bu
### `entries` will not have fields present in the raw data
`entries` will only know about the fields declared by `add` with `field:`. And...
`entries` will only know about the fields declared by `register_column` with `field:`. And...
### `entries` will have things that are not fields
Each time you call `add` - even for things like `count` and `exists?` - that will add an attribute to the Struct that is used to represent a row. Those attributes will always be nil.
Each time you call `register_custom_property`, `register_custom_matcher` or `register_column` - even for things like `count` and `exists?` - that will add an attribute to the Struct that is used to represent a row. Those attributes will always be nil.
### `add` does not know about what fields are in the raw data
### `register_column` does not know about what fields are in the raw data
This is because the raw data fetcher is not called until as late as possible. That's good - it might be expensive - but it also means we can't scan it for columns. There are ways around that.
@ -416,11 +416,11 @@ This is because the raw data fetcher is not called until as late as possible. T
### You can't call resource methods on a Table directly
### You can't use a column name in a `where` block unless it was declared as a field using `add`
### You can't use a column name in a `where` block unless it was declared as a field using `register_column`
```ruby
# This will give a NameError - :tackiness is in the raw
# data hash but not declared using `add`.
# data hash but not declared using `register_custom_property`.
describe things.where { tackiness == 'very' } do
its('count') { should cmp 1 }
end

View file

@ -4,7 +4,8 @@
# author: Christoph Hartmann
module FilterTable
module Show; end
# This is used as a sentinel value in custom property filtering
module NoCriteriaProvided; end
class ExceptionCatcher
def initialize(original_resource, original_exception)
@ -82,56 +83,77 @@ module FilterTable
end
class Table
attr_reader :params, :resource
def initialize(resource, params, filters)
@resource = resource
@params = params
@params = [] if @params.nil?
@filters = filters
attr_reader :raw_data, :resource_instance, :criteria_string
def initialize(resource_instance, raw_data, criteria_string)
@resource_instance = resource_instance
@raw_data = raw_data
@raw_data = [] if @raw_data.nil?
@criteria_string = criteria_string
end
# Filter the raw data based on criteria (as method params) or by evaling a
# block; then construct a new Table of the same class as ourselves,
# wrapping the filtered data, and return it.
def where(conditions = {}, &block)
return self if !conditions.is_a?(Hash)
return self if conditions.empty? && !block_given?
filters = ''
table = @params
conditions.each do |field, condition|
filters += " #{field} == #{condition.inspect}"
table = filter_lines(table, field, condition)
# Initialize the details of the new Table.
new_criteria_string = criteria_string
filtered_raw_data = raw_data
# If we were provided params, interpret them as criteria to be evaluated
# against the raw data. Criteria are assumed to be hash keys.
conditions.each do |raw_field_name, desired_value|
new_criteria_string += " #{raw_field_name} == #{desired_value.inspect}"
filtered_raw_data = filter_raw_data(filtered_raw_data, raw_field_name, desired_value)
end
# If we were given a block, make a special Struct for each row, that has an accessor
# for each field declared using `register_custom_property`, then instance-eval the block
# against the struct.
if block_given?
table = table.find_all { |e| new_entry(e, '').instance_eval(&block) }
# Perform the filtering.
filtered_raw_data = filtered_raw_data.find_all { |row_as_hash| create_eval_context_for_row(row_as_hash, '').instance_eval(&block) }
# Try to interpret the block for updating the stringification.
src = Trace.new
src.instance_eval(&block)
filters += Trace.to_ruby(src)
new_criteria_string += Trace.to_ruby(src)
end
self.class.new(@resource, table, @filters + filters)
self.class.new(resource, filtered_raw_data, new_criteria_string)
end
def new_entry(*_)
def create_eval_context_for_row(*_)
raise "#{self.class} must not be used on its own. It must be inherited "\
'and the #new_entry method must be implemented. This is an internal '\
'and the #create_eval_context_for_row method must be implemented. This is an internal '\
'error and should not happen.'
end
def resource
resource_instance
end
def params
# TODO: consider deprecating
raw_data
end
def entries
f = @resource.to_s + @filters.to_s + ' one entry'
@params.map do |line|
new_entry(line, f)
row_criteria_string = resource.to_s + criteria_string + ' one entry'
raw_data.map do |row|
create_eval_context_for_row(row, row_criteria_string)
end
end
def get_field(field)
@params.map do |line|
line[field]
def get_column_values(field)
raw_data.map do |row|
row[field]
end
end
def to_s
@resource.to_s + @filters
resource.to_s + criteria_string
end
alias inspect to_s
@ -159,111 +181,143 @@ module FilterTable
x === y # rubocop:disable Style/CaseEquality
end
def filter_lines(table, field, condition)
m = case condition
when Float then method(:matches_float)
when Integer then method(:matches_int)
when Regexp then method(:matches_regex)
else method(:matches)
end
def filter_raw_data(current_raw_data, field, desired_value)
method_ref = case desired_value
when Float then method(:matches_float)
when Integer then method(:matches_int)
when Regexp then method(:matches_regex)
else method(:matches)
end
table.find_all do |line|
next unless line.key?(field)
m.call(line[field], condition)
current_raw_data.find_all do |row|
next unless row.key?(field)
method_ref.call(row[field], desired_value)
end
end
end
class Factory
Connector = Struct.new(:field_name, :block, :opts)
CustomPropertyType = Struct.new(:field_name, :block, :opts)
def initialize
@accessors = []
@connectors = {}
@resource = nil
@filter_methods = []
@custom_properties = {}
@resource = nil # TODO: this variable is never initialized
end
def connect(resource, table_accessor) # rubocop:disable Metrics/AbcSize
# create the table structure
connectors = @connectors
struct_fields = connectors.values.map(&:field_name)
connector_blocks = connectors.map do |method, c|
[method.to_sym, create_connector(c)]
end
def install_filter_methods_on_resource(resource_class, raw_data_fetcher_method_name)
struct_fields = @custom_properties.values.map(&:field_name)
# the struct to hold single items from the #entries method
entry_struct = Struct.new(*struct_fields.map(&:to_sym)) do
attr_accessor :__filter
# A context in which you can access the fields as accessors
row_eval_context_type = Struct.new(*struct_fields.map(&:to_sym)) do
attr_accessor :criteria_string
def to_s
@__filter || super
@criteria_string || super
end
end unless struct_fields.empty?
# the main filter table
table = Class.new(Table) {
connector_blocks.each do |x|
define_method x[0], &x[1]
properties_to_define = @custom_properties.map do |method_name, custom_property_structure|
{ method_name: method_name, method_body: create_custom_property_body(custom_property_structure) }
end
# Define the filter table subclass
table_class = Class.new(Table) {
# Install each custom property onto the FilterTable subclass
properties_to_define.each do |property_info|
define_method property_info[:method_name], &property_info[:method_body]
end
define_method :new_entry do |hashmap, filter = ''|
return entry_struct.new if hashmap.nil?
res = entry_struct.new(*struct_fields.map { |x| hashmap[x] })
res.__filter = filter
# Install a method that can wrap all the fields into a context with accessors
define_method :create_eval_context_for_row do |row_as_hash, criteria_string = ''|
return row_eval_context_type.new if row_as_hash.nil?
res = row_eval_context_type.new(*struct_fields.map { |field| row_as_hash[field] })
res.criteria_string = criteria_string
res
end
}
# Define all access methods with the parent resource
# Define all access methods with the parent resource_class
# These methods will be configured to return an `ExceptionCatcher` object
# that will always return the original exception, but only when called
# upon. This will allow method chains in `describe` statements to pass the
# `instance_eval` when loaded and only throw-and-catch the exception when
# the tests are run.
accessors = @accessors + @connectors.keys
accessors.each do |method_name|
resource.send(:define_method, method_name.to_sym) do |*args, &block|
methods_to_install_on_resource_class = @filter_methods + @custom_properties.keys
methods_to_install_on_resource_class.each do |method_name|
resource_class.send(:define_method, method_name.to_sym) do |*args, &block|
begin
filter = table.new(self, method(table_accessor).call, ' with')
filter.method(method_name.to_sym).call(*args, &block)
# self here is the resource instance
filter_table_instance = table_class.new(self, method(raw_data_fetcher_method_name).call, ' with')
filter_table_instance.method(method_name.to_sym).call(*args, &block)
rescue Inspec::Exceptions::ResourceFailed, Inspec::Exceptions::ResourceSkipped => e
FilterTable::ExceptionCatcher.new(resource, e)
FilterTable::ExceptionCatcher.new(resource_class, e)
end
end
end
end
def add_accessor(method_name)
alias connect install_filter_methods_on_resource
# TODO: This should almost certainly be privatized. Every FilterTable client should get :entries and :where;
# InSpec core resources do not define anything else, other than azure_generic_resource, which is likely a mis-use.
def register_filter_method(method_name)
if method_name.nil?
throw RuntimeError, "Called filter.add_delegator for resource #{@resource} with method name nil!"
# TODO: @resource is never initialized
throw RuntimeError, "Called filter.add_accessor for resource #{@resource} with method name nil!"
end
@accessors.push(method_name)
@filter_methods.push(method_name.to_sym)
self
end
def add(method_name, opts = {}, &block)
if method_name.nil?
alias add_accessor register_filter_method
def register_custom_property(property_name, opts = {}, &property_implementation)
if property_name.nil?
# TODO: @resource is never initialized
throw RuntimeError, "Called filter.add for resource #{@resource} with method name nil!"
end
@connectors[method_name.to_sym] =
Connector.new(opts[:field] || method_name, block, opts)
@custom_properties[property_name.to_sym] =
CustomPropertyType.new(opts[:field] || property_name, property_implementation, opts)
self
end
alias add register_custom_property
alias register_column register_custom_property
alias register_custom_matcher register_custom_property
private
def create_connector(c)
return ->(cond = Show) { c.block.call(self, cond) } if !c.block.nil?
lambda { |condition = Show, &cond_block|
if condition == Show && !block_given?
r = where(nil).get_field(c.field_name)
r = r.flatten.uniq.compact if c.opts[:style] == :simple
r
else
where({ c.field_name => condition }, &cond_block)
# This provides the implementation for methods requested using
# register_custom_property(:some_method_name, opts, &block)
# Some usage in the wild involves people passing a desired value to the generated method, like:
# things.ids(23)
# I'm calling this the 'filter_criterion_value'. I speculate that a default value is
# provided here so that users can meaningfully query for nil.
def create_custom_property_body(custom_property_struct)
if !custom_property_struct.block.nil?
# If the custom method provided its own block, rely on it.
lambda do |filter_criteria_value = NoCriteriaProvided|
# Here self is an instance of the FilterTable subclass that wraps the raw data.
# Call the block with two args - the table instance, and any filter criteria value.
custom_property_struct.block.call(self, filter_criteria_value)
end
}
else
# No block definition, so the property was registered using (field: :some_field)
# This does however support passing a block to this method, and filtering using it, like Enumerable#select.
lambda do |filter_criteria_value = NoCriteriaProvided, &cond_block|
if filter_criteria_value == NoCriteriaProvided && !block_given?
# No second-order block given. Just return an array of the values in the selected column.
result = where(nil).get_column_values(custom_property_struct.field_name) # TODO: the where(nil). is likely unneeded
result = result.flatten.uniq.compact if custom_property_struct.opts[:style] == :simple
result
else
# A secondary block was provided. Rely on where() to execute the block, while also filtering on any single value
# Suspected bug: if filter_criteria_value == NoCriteriaProvided, this is unlikely to match - see hash condition handling in where() above.
where(custom_property_struct.field_name => filter_criteria_value, &cond_block)
end
end
end
end
end