Content Views Rework: merging filters & filter rules, CRUD support

The following is a high-level summary of the changes:
1. Merge filter and filter rules together.  With these changes,
   there will be base filter class that the filter types will
   derive from (i.e. package, package group, erratum, puppet module).
   Each filter may have multiple 'parameters' associated with it.
2. Support elastic search indexing of filters
3. Provide v2 api support for CRUD (create/review/update/delete) actions

This commit does not address the following:
- Updates to content view publish/refresh to use the updated models
  (anticipate more changes needed  there)
- 'Green' tests

Both of the above will be addressed in separate commits.

app/controllers/katello/api/v2/filters_controller.rb

@@ -11,88 +11,94 @@
 # http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
 
 module Katello
-class Api::V2::FiltersController < Api::V1::FiltersController
+class Api::V2::FiltersController < Api::V2::ApiController
+  respond_to :json
 
-  skip_before_filter :find_organization
+  before_filter :find_content_view
+  before_filter :find_filter, :except => [:index, :create]
+  before_filter :authorize
 
-  include Api::V2::Rendering
+  wrap_parameters :include => (Filter.attribute_names + %w(repository_ids))
 
-  api :GET, "/content_view_definitions/:content_view_definition_id/filters",
-      "List filters"
-  param :content_view_definition_id, String, :desc => "id of the content view definition", :required => true
+  def rules
+    view_readable = lambda { @view.readable? }
+    view_editable = lambda { @view.editable? }
+
+    {
+        :index   => view_readable,
+        :create  => view_editable,
+        :show    => view_readable,
+        :update  => view_editable,
+        :destroy => view_editable
+    }
+  end
+
+  api :GET, "/content_views/:content_view_id/filters", "List filters"
+  param :content_view_id, :number, :desc => "content view identifier", :required => true
   def index
-    super
+    options = sort_params
+    options[:load_records?] = true
+    options[:filters] = [{ :terms => { :id => @view.filter_ids } }]
+
+    @search_service.model = Filter
+    respond(:collection => item_search(Filter, params, options))
   end
 
-  api :POST, "/content_view_definitions/:content_view_definition_id/filters",
-      "Create a filter for a content view definition"
-  param :content_view_definition_id, String, :desc => "id of the content view definition", :required => true
+  api :POST, "/content_views/:content_view_id/filters",
+      "Create a filter for a content view"
+  param :content_view_id, :number, :desc => "content view identifier", :required => true
   param :filter, Hash, :required => true, :action_aware => true do
     param :name, String, :desc => "name of the filter", :required => true
+    param :type, String, :desc => "type of filter (e.g. rpm, package_group, erratum, puppet_module)", :required => true
+    param :parameters, Hash, :desc => "the filter parameter rules"
   end
   def create
-    filter = Filter.create!(:content_view_definition => @definition, :name => params[:filter][:name])
+    filter = Filter.create_for(params[:type], filter_params.merge(:content_view => @view))
     respond :resource => filter
   end
 
-  api :GET, "/content_view_definitions/:content_view_definition_id/filters/:id",
-      "Show filter info"
-  param :content_view_definition_id, String, :desc => "id of the content view definition", :required => true
-  param :id, String, :desc => "name of the filter", :required => true
+  api :GET, "/content_views/:content_view_id/filters/:id", "Show filter info"
+  param :content_view_id, :number, :desc => "content view identifier", :required => true
+  param :id, :number, :desc => "filter identifier", :required => true
   def show
-    super
+    respond :resource => @filter
   end
 
-  api :DELETE, "/content_view_definitions/:content_view_definition_id/filters/:id",
-      "Delete a filter"
-  param :content_view_definition_id, String, :desc => "id of the content view definition", :required => true
-  param :id, String, :desc => "name of the filter", :required => true
-  def destroy
-    super
+  api :PUT, "/content_views/:content_view_id/filters/:id", "Update a filter"
+  param :content_view_id, :number, :desc => "Content view identifier", :required => true
+  param :id, :number, :desc => "id of the filter", :required => true
+  param :name, String, :desc => "New name for the filter"
+  param :repository_ids, Array, :desc => "List of repository ids"
+  def update
+    @filter.update_attributes!(filter_params)
+    respond :resource => @filter
   end
 
-  api :GET, "/content_view_definitions/:content_view_definition_id/filters/:id/products",
-      "List all the products for a content view definition filter"
-  param :content_view_definition_id, String, :desc => "id of the content view definition", :required => true
-  param :id, String, :desc => "name of the filter", :required => true
-  def list_products
-    super
+  api :DELETE, "/content_views/:content_view_id/filters/:id", "Delete a filter"
+  param :content_view_id, :number, :desc => "content view identifier", :required => true
+  param :id, :number, :desc => "filter identifier", :required => true
+  def destroy
+    @filter.destroy
+    respond :resource => @filter
   end
 
-  api :PUT, "/content_view_definitions/:content_view_definition_id/filters/:id/products",
-      "Update products for a content view definition filter"
-  param :content_view_definition_id, :identifier, :required => true,
-                                                  :desc => "content view definition identifier"
-  param :id, String, :desc => "name of the filter", :required => true
-  param :products, Array, :desc => "Updated list of product ids", :required => true
-  def update_products
-    _update_products! params
-    respond_for_update :resource => @filter
-  end
+  private
 
-  api :GET, "/content_view_definitions/:content_view_definition_id/filters/:id/repositories",
-      "List all the repositories for a content view definition filter"
-  param :content_view_definition_id, String, :desc => "id of the content view definition", :required => true
-  param :id, String, :desc => "name of the filter", :required => true
-  def list_repositories
-    super
+  def find_content_view
+    @view = ContentView.find(params[:content_view_id])
   end
 
-  api :PUT, "/content_view_definitions/:content_view_definition_id/filters/:id/repositories",
-      "Update repositories for a content view definition filter"
-  param :content_view_definition_id, String, :desc => "id of the content view definition", :required => true
-  param :id, String, :desc => "name of the filter", :required => true
-  param :repos, Array, :desc => "Updated list of repo ids", :required => true
-  def update_repositories
-    _update_repositories! params
-    respond_for_update :resource => @filter
+  def find_filter
+    id = params[:id] || params[:filter_id]
+    @filter = Filter.where(:content_view_id => @view).find(id)
   end
 
-  private
+  def filter_params
+    filter_parameters = params.require(:filter).permit(:name, :repository_ids => [])
 
-  def find_definition
-    @definition   = ContentViewDefinition.find(params[:content_view_definition_id])
-    @organization ||= @definition.organization
+    # the :parameters will be vaildated by the model layer (e.g. PackageFilter)
+    filter_parameters[:parameters] = params[:filter][:parameters].with_indifferent_access
+    filter_parameters
   end
 
 end

app/lib/katello/validators/erratum_rule_params_validator.rb → app/lib/katello/validators/erratum_filter_params_validator.rb

@@ -12,12 +12,12 @@
 
 module Katello
 module Validators
-  class ErratumRuleParamsValidator < ActiveModel::EachValidator
+  class ErratumFilterParamsValidator < ActiveModel::EachValidator
     def validate_each(record, attribute, value)
       if value
         if value.key?("units") && (value.key?("date_range") ||
                                    value.key?("errata_type"))
-          record.errors.add(attribute, _("Errata rules cannot contain both id and date_range/errata_type criteria."))
+          record.errors.add(attribute, _("Errata filters cannot contain both id and date_range/errata_type criteria."))
         end
         if value.key?(:date_range)
           _check_date_range(record, attribute, value)
@@ -32,12 +32,12 @@ def validate_each(record, attribute, value)
     def _check_errata_type(record, attribute, value)
       errata_type = value[:errata_type]
       if errata_type.is_a?(Array)
-        invalid_types = errata_type.collect(&:to_s) - ErratumRule::ERRATA_TYPES.keys
+        invalid_types = errata_type.collect(&:to_s) - ErratumFilter::ERRATA_TYPES.keys
         unless invalid_types.empty?
           record.errors.add(attribute,
                             _("Invalid erratum types %{invalid_types} provided. Erratum type can be any of %{valid_types}") %
                             { :invalid_types => invalid_types.join(","),
-                              :valid_types => ErratumRule::ERRATA_TYPES.keys.join(",")})
+                              :valid_types => ErratumFilter::ERRATA_TYPES.keys.join(",")})
         end
       else
         record.errors.add(attribute, _("The erratum type must be an array. Invalid value provided"))
@@ -46,18 +46,18 @@ def _check_errata_type(record, attribute, value)
 
     def _check_date_range(record, attribute, value)
       date_range = value[:date_range]
-      start_date_int = date_range[:start]
-      end_date_int = date_range[:end]
+      start_date_int = date_range[:start].to_time.to_i if date_range.has_key?(:start)
+      end_date_int = date_range[:end].to_time.to_i if date_range.has_key?(:end)
 
-      if start_date_int && !(start_date_int.is_a?(Fixnum))
-        record.errors.add(attribute, _("The erratum rule start date is in an invalid format or type."))
+      if start_date_int && (!(start_date_int.is_a?(Fixnum)) || !(date_range[:start].is_a?(String)))
+        record.errors.add(attribute, _("The erratum filter parameter start date is in an invalid format or type."))
       end
-      if end_date_int && !(end_date_int.is_a?(Fixnum))
-        record.errors.add(attribute, _("The erratum rule end date is in an invalid format or type."))
+      if end_date_int && (!(end_date_int.is_a?(Fixnum)) || !(date_range[:end].is_a?(String)))
+        record.errors.add(attribute, _("The erratum filter parameter end date is in an invalid format or type."))
       end
 
       if start_date_int && end_date_int && !(start_date_int <= end_date_int)
-        record.errors.add(attribute, _("Invalid date range. The erratum rule start date must come before the end date"))
+        record.errors.add(attribute, _("Invalid date range. The erratum filter parameter start date must come before the end date"))
       end
     end
   end

app/lib/katello/validators/rule_params_validator.rb → app/lib/katello/validators/filter_params_validator.rb

@@ -12,17 +12,17 @@
 
 module Katello
 module Validators
-  class RuleParamsValidator < ActiveModel::EachValidator
+  class FilterParamsValidator < ActiveModel::EachValidator
     def validate_each(record, attribute, value)
-      rule_type = record.rule_type.downcase
+      filter_type = record.filter_type.downcase
 
       if value && value[:units].present?
         if !value[:units].is_a?(Array)
-          record.errors.add(attribute, _("Invalid %s rule specified. Units must be an array.") % rule_type)
+          record.errors.add(attribute, _("Invalid %s filter parameter specified. Units must be an array.") % filter_type)
         else
           value[:units].each do |unit|
             unless unit.key?(:name)
-              record.errors.add(attribute, _("Invalid %s rule specified. Missing 'name'.") % rule_type)
+              record.errors.add(attribute, _("Invalid %s filter parameter specified. Missing 'name'.") % filter_type)
               break
             end
           end

app/lib/katello/validators/rule_version_validator.rb → app/lib/katello/validators/filter_version_validator.rb

@@ -12,12 +12,12 @@
 
 module Katello
 module Validators
-  class RuleVersionValidator < ActiveModel::EachValidator
+  class FilterVersionValidator < ActiveModel::EachValidator
     def validate_each(record, attribute, value)
       if value && value[:units].present? && value[:units].is_a?(Array)
         value[:units].each do |unit|
           if unit.key?(:version) && (unit.key?(:min_version) || unit.key?(:max_version))
-            ver_msg = _("Invalid rule combination specified, 'version'" +
+            ver_msg = _("Invalid filter parameter combination specified, 'version'" +
                         " and 'min_version' or 'max_version' cannot be specified in the same tuple")
 
             record.errors.add(attribute, ver_msg)

app/lib/katello/validators/package_group_rule_params_validator.rb → app/lib/katello/validators/package_group_filter_params_validator.rb

@@ -12,16 +12,16 @@
 
 module Katello
 module Validators
-  class PackageGroupRuleParamsValidator < ActiveModel::EachValidator
+  class PackageGroupFilterParamsValidator < ActiveModel::EachValidator
     def validate_each(record, attribute, value)
       if value
         unless value[:units].blank?
           if !value[:units].is_a?(Array)
-            record.errors.add(attribute, _("Invalid package rule specified. Units must be an array."))
+            record.errors.add(attribute, _("Invalid package group filter parameter specified. Units must be an array."))
           else
             value[:units].each do |unit|
               unless unit.key?(:name)
-                record.errors.add(attribute, _("Invalid package group rule specified. Missing package 'name'."))
+                record.errors.add(attribute, _("Invalid package group filter parameter specified. Missing package 'name'."))
                 break
               end
             end

app/models/katello/content_view.rb

@@ -39,7 +39,7 @@ def create_event
   has_many :composite_content_views, :class_name => "ContentView"
   has_many :content_views, :through => :component_content_views
   has_many :content_view_repositories, :dependent => :destroy
-  has_many :repositories, :through => :content_view_repositories
+  has_many :repositories, :through => :content_view_repositories, :after_remove => :remove_repository
   has_many :filters, :dependent => :destroy
 
   has_many :changeset_content_views, :class_name => "Katello::ChangesetContentView", :dependent => :destroy
@@ -431,6 +431,19 @@ def cp_environment_id(env)
     ContentViewEnvironment.where(:content_view_id => self, :environment_id => env).first.cp_id
   end
 
+  protected
+
+  def remove_repository(repository)
+    filters.each do |filter_item|
+      repo_exists = Repository.unscoped.joins(:filters).where(
+          Filter.table_name => {:id => filter_item.id}, :id => repository.id).count
+      if repo_exists
+        filter_item.repositories.delete(repository)
+        filter_item.save!
+      end
+    end
+  end
+
   private
 
   def generate_cp_environment_label(env)

app/models/katello/erratum_filter.rb

@@ -12,27 +12,31 @@
 
 module Katello
 class ErratumFilter < Filter
+  use_index_of Filter if Katello.config.use_elasticsearch
 
-  ERRATA_TYPES = {'bugfix' => _('Bug Fix'),
-                  'enhancement' => _('Enhancement'),
-                  'security' => _('Security')}.with_indifferent_access
+  CONTENT_TYPE = Errata::CONTENT_TYPE
 
-  validates_with Validators::ErratumRuleParamsValidator, :attributes => :parameters
+  ERRATA_TYPES = { 'bugfix' => _('Bug Fix'),
+                   'enhancement' => _('Enhancement'),
+                   'security' => _('Security') }.with_indifferent_access
+
+  before_create :set_parameters
+
+  validates_with Validators::ErratumFilterParamsValidator, :attributes => :parameters
 
   def params_format
-    {:units => [[:id]], :date_range => [:start, :end], :errata_type => {}}
+    { :units => [[:id]], :date_range => [:start, :end], :errata_type => {}, :inclusion => {}, :created_at => {} }
   end
 
   [:start, :end].each do |date_type|
-    define_method("#{date_type}_date") do
-      dt = parameters["date_range"].try(:[], date_type)
-      Time.at(dt) if dt
+    define_method("#{ date_type }_date") do
+      parameters["date_range"].try(:[], date_type)
     end
 
-    define_method("#{date_type}_date=") do |date|
+    define_method("#{ date_type }_date=") do |date|
       parameters["date_range"] ||= {}
       if date
-        parameters["date_range"][date_type] = date.to_i
+        parameters["date_range"][date_type] = date
       else
         parameters["date_range"].delete(date_type)
         parameters.delete("date_range") if parameters["date_range"].empty?
@@ -64,22 +68,20 @@ def generate_clauses(repo)
       #                         [:errata_id_sort, "DESC"],'errata_id').collect(&:errata_id)
       #   end
       # end.compact.flatten
-      ids = parameters[:units].collect do |unit|
-        unit[:id]
-      end
+      ids = parameters[:units].collect{ |unit| unit[:id] }
       ids.compact!
 
-      {"id" => {"$in" => ids}}  unless ids.empty?
+      { "id" => { "$in" => ids } } unless ids.empty?
     else
       if parameters.key? "date_range"
         dr = {}
         dr["$gte"] = start_date.as_json if start_date
         dr["$lte"] = end_date.as_json if end_date
-        rule_clauses << {"issued" => dr}
+        rule_clauses << { "issued" => dr }
       end
       if errata_types
           # {"type": {"$in": ["security", "enhancement", "bugfix"]}
-        rule_clauses << {"type" => {"$in" => errata_types}}
+        rule_clauses << { "type" => { "$in" => errata_types } }
       end
 
       case rule_clauses.size
@@ -88,7 +90,7 @@ def generate_clauses(repo)
       when 1
         return rule_clauses.first
       else
-        return {'$and' => rule_clauses}
+        return { '$and' => rule_clauses }
       end
     end
   end
@@ -103,5 +105,16 @@ def as_json(options = {})
     json_val.delete("parameters")
     json_val
   end
+
+  private
+
+  def set_parameters
+    unless parameters.blank?
+      parameters[:created_at] = Time.zone.now
+      parameters[:inclusion] = false unless parameters.has_key?(:inclusion)
+    end
+    self
+  end
+
 end
 end