Add key uniqueness validation to subscription model - #119

app/controllers/activity_notification/subscriptions_api_controller.rb

@@ -38,22 +38,13 @@ def index
     #   @return [JSON] Created subscription
     def create
       render_invalid_parameter("Parameter is missing or the value is empty: subscription") and return if params[:subscription].blank?
-      render_invalid_parameter("Parameter is missing or the value is empty: subscription[key]") and return if params[:subscription][:key].blank?
-      @subscription = @target.find_subscription(params[:subscription][:key])
-      if @subscription
-        render status: 409, location: { action: :show, id: @subscription }, json: {
-          status: "conflict because of duplicate key",
-          subscription: subscription_json
-        }
-      else
-        optional_target_names = (params[:subscription][:optional_targets] || {}).keys.select { |key| !key.to_s.start_with?("subscribing_to_") }
-        optional_target_names.each do |optional_target_name|
-          subscribing_param = params[:subscription][:optional_targets][optional_target_name][:subscribing]
-          params[:subscription][:optional_targets]["subscribing_to_#{optional_target_name}"] = subscribing_param unless subscribing_param.nil?
-        end
-        super
-        render status: 201, location: { action: :show, id: @subscription }, json: subscription_json
+      optional_target_names = (params[:subscription][:optional_targets] || {}).keys.select { |key| !key.to_s.start_with?("subscribing_to_") }
+      optional_target_names.each do |optional_target_name|
+        subscribing_param = params[:subscription][:optional_targets][optional_target_name][:subscribing]
+        params[:subscription][:optional_targets]["subscribing_to_#{optional_target_name}"] = subscribing_param unless subscribing_param.nil?
       end
+      super
+      render status: 201, location: { action: :show, id: @subscription }, json: subscription_json
     end
 
     # Finds and shows a subscription from specified key.
@@ -198,7 +189,7 @@ def subscription_json
       # Validate @subscription and render JSON of @subscription
       # @api protected
       def validate_and_render_subscription
-        raise InvalidParameterError, @subscription.errors.full_messages.first if @subscription.invalid?
+        raise RecordInvalidError, @subscription.errors.full_messages.first if @subscription.invalid?
         render json: subscription_json
       end
 

lib/activity_notification/controllers/common_api_controller.rb

@@ -4,10 +4,9 @@ module CommonApiController
     extend ActiveSupport::Concern
 
     included do
-      rescue_from ActiveRecord::RecordNotFound,                with: :render_resource_not_found if defined?(ActiveRecord)
-      rescue_from Mongoid::Errors::DocumentNotFound,           with: :render_resource_not_found if ActivityNotification.config.orm == :mongoid
-      rescue_from Dynamoid::Errors::RecordNotFound,            with: :render_resource_not_found if ActivityNotification.config.orm == :dynamoid
-      rescue_from ActivityNotification::InvalidParameterError, with: ->(e){ render_invalid_parameter(e.message) }
+      rescue_from ActiveRecord::RecordNotFound,      with: :render_resource_not_found if defined?(ActiveRecord)
+      rescue_from Mongoid::Errors::DocumentNotFound, with: :render_resource_not_found if ActivityNotification.config.orm == :mongoid
+      rescue_from Dynamoid::Errors::RecordNotFound,  with: :render_resource_not_found if ActivityNotification.config.orm == :dynamoid
     end
 
     protected

lib/activity_notification/controllers/common_controller.rb

@@ -11,6 +11,7 @@ module CommonController
 
       prepend_before_action :set_target
       before_action :set_view_prefixes
+      rescue_from ActivityNotification::RecordInvalidError, with: ->(e){ render_unprocessable_entity(e.message) }
     end
 
     DEFAULT_VIEW_DIRECTORY = "default"
@@ -112,6 +113,13 @@ def validate_param(param_name)
         render_invalid_parameter("Parameter is missing: #{param_name}") if params[param_name].blank?
       end
 
+      # Render Invalid Parameter error with 400 status
+      # @api protected
+      # @return [void]
+      def render_unprocessable_entity(message)
+        render status: 422, json: error_response(code: 422, message: "Unprocessable entity", type: message)
+      end
+
       # Returns JavaScript view for ajax request or redirects to back as default.
       # @api protected
       # @return [Response] JavaScript view for ajax request or redirects to back as default

lib/activity_notification/controllers/concerns/swagger/error_responses.rb

@@ -38,5 +38,18 @@ def self.extended(base)
         end
       end
     end
+
+    module UnprocessableEntityError #:nodoc:
+      def self.extended(base)
+        base.response 422 do
+          key :description, "Unprocessable entity"
+          content 'application/json' do
+            schema do
+              key :'$ref', :Error
+            end
+          end
+        end
+      end
+    end
   end
 end

lib/activity_notification/controllers/concerns/swagger/subscriptions_api.rb

@@ -101,27 +101,9 @@ module Swagger::SubscriptionsApi #:nodoc:
               end
             end
           end
-          response 409 do
-            key :description, "Conflict because of duplicate key"
-            content 'application/json' do
-              schema do
-                key :type, :object
-                property :status do
-                  key :type, :string
-                  key :description, "Conflict status"
-                  key :default, "conflict because of duplicate key"
-                  key :example, "conflict because of duplicate key"
-                end
-                property :subscription do
-                  key :type, :object
-                  key :'$ref', :Subscription
-                  key :description, "Found duplicate subscription"
-                end
-              end
-            end
-          end
           extend Swagger::ErrorResponses::InvalidParameterError
           extend Swagger::ErrorResponses::ResourceNotFoundError
+          extend Swagger::ErrorResponses::UnprocessableEntityError
         end
       end
 
@@ -274,6 +256,7 @@ module Swagger::SubscriptionsApi #:nodoc:
           extend Swagger::ErrorResponses::InvalidParameterError
           extend Swagger::ErrorResponses::ForbiddenError
           extend Swagger::ErrorResponses::ResourceNotFoundError
+          extend Swagger::ErrorResponses::UnprocessableEntityError
         end
       end
 
@@ -298,6 +281,7 @@ module Swagger::SubscriptionsApi #:nodoc:
           extend Swagger::ErrorResponses::InvalidParameterError
           extend Swagger::ErrorResponses::ForbiddenError
           extend Swagger::ErrorResponses::ResourceNotFoundError
+          extend Swagger::ErrorResponses::UnprocessableEntityError
         end
       end
 
@@ -322,6 +306,7 @@ module Swagger::SubscriptionsApi #:nodoc:
           extend Swagger::ErrorResponses::InvalidParameterError
           extend Swagger::ErrorResponses::ForbiddenError
           extend Swagger::ErrorResponses::ResourceNotFoundError
+          extend Swagger::ErrorResponses::UnprocessableEntityError
         end
       end
 
@@ -346,6 +331,7 @@ module Swagger::SubscriptionsApi #:nodoc:
           extend Swagger::ErrorResponses::InvalidParameterError
           extend Swagger::ErrorResponses::ForbiddenError
           extend Swagger::ErrorResponses::ResourceNotFoundError
+          extend Swagger::ErrorResponses::UnprocessableEntityError
         end
       end
 
@@ -378,6 +364,7 @@ module Swagger::SubscriptionsApi #:nodoc:
           extend Swagger::ErrorResponses::InvalidParameterError
           extend Swagger::ErrorResponses::ForbiddenError
           extend Swagger::ErrorResponses::ResourceNotFoundError
+          extend Swagger::ErrorResponses::UnprocessableEntityError
         end
       end
 
@@ -410,6 +397,7 @@ module Swagger::SubscriptionsApi #:nodoc:
           extend Swagger::ErrorResponses::InvalidParameterError
           extend Swagger::ErrorResponses::ForbiddenError
           extend Swagger::ErrorResponses::ResourceNotFoundError
+          extend Swagger::ErrorResponses::UnprocessableEntityError
         end
       end
     end

lib/activity_notification/helpers/errors.rb

@@ -2,5 +2,5 @@ module ActivityNotification
   class ConfigError < StandardError; end
   class DeleteRestrictionError < StandardError; end
   class NotifiableNotFoundError < StandardError; end
-  class InvalidParameterError < StandardError; end
+  class RecordInvalidError < StandardError; end
 end

lib/activity_notification/models/concerns/subscriber.rb

@@ -46,9 +46,19 @@ def find_or_create_subscription(key, subscription_params = {})
     # Creates new subscription of the target.
     #
     # @param [Hash] subscription_params Parameters to create subscription record
-    # @raise [ActivityNotification::InvalidParameterError] Failed to save subscription due to invalid parameter
+    # @raise [ActivityNotification::RecordInvalidError] Failed to save subscription due to model validation
     # @return [Subscription] Created subscription instance
     def create_subscription(subscription_params = {})
+      subscription = build_subscription(subscription_params)
+      raise RecordInvalidError, subscription.errors.full_messages.first unless subscription.save
+      subscription
+    end
+
+    # Builds new subscription of the target.
+    #
+    # @param [Hash] subscription_params Parameters to build subscription record
+    # @return [Subscription] Built subscription instance
+    def build_subscription(subscription_params = {})
       created_at = Time.current
       if subscription_params[:subscribing] == false && subscription_params[:subscribing_to_email].nil?
         subscription_params[:subscribing_to_email] = subscription_params[:subscribing] 
@@ -75,7 +85,6 @@ def create_subscription(subscription_params = {})
           )
       end
       subscription.assign_attributes(optional_targets: optional_targets)
-      raise InvalidParameterError, subscription.errors.full_messages.first unless subscription.save
       subscription
     end
 

lib/activity_notification/orm/active_record/subscription.rb

@@ -17,7 +17,7 @@ class Subscription < ::ActiveRecord::Base
         serialize  :optional_targets, Hash
 
         validates  :target,               presence: true
-        validates  :key,                  presence: true
+        validates  :key,                  presence: true, uniqueness: { scope: :target }
         validates_inclusion_of :subscribing,          in: [true, false]
         validates_inclusion_of :subscribing_to_email, in: [true, false]
         validate   :subscribing_to_email_cannot_be_true_when_subscribing_is_false

lib/activity_notification/orm/dynamoid/extension.rb

@@ -169,6 +169,78 @@ def scan_filter
   end
 end
 
+# Entend Dynamoid to support uniqueness validator
+# @private
+module Dynamoid # :nodoc: all
+  # https://github.com/Dynamoid/dynamoid/blob/master/lib/dynamoid/validations.rb
+  # @private
+  module Validations
+    # Validates whether or not a field is unique against the records in the database.
+    class UniquenessValidator < ActiveModel::EachValidator
+      # Validate the document for uniqueness violations.
+      # @param [Document] document The document to validate.
+      # @param [Symbol] attribute  The name of the attribute.
+      # @param [Object] value      The value of the object.
+      def validate_each(document, attribute, value)
+        return unless validation_required?(document, attribute)
+        document.errors.add(attribute, :taken, options.except(:scope).merge(value: value)) if not_unique?(document, attribute, value)
+      end
+
+      private
+
+      # Are we required to validate the document?
+      # @api private
+      def validation_required?(document, attribute)
+        document.new_record? ||
+          document.send("attribute_changed?", attribute.to_s) ||
+          scope_value_changed?(document)
+      end
+
+      # Scope reference has changed?
+      # @api private
+      def scope_value_changed?(document)
+        Array.wrap(options[:scope]).any? do |item|
+          document.send("attribute_changed?", item.to_s)
+        end
+      end
+
+      # Check whether a record is uniqueness.
+      # @api private
+      def not_unique?(document, attribute, value)
+        klass = document.class
+        while klass.superclass.respond_to?(:validators) && klass.superclass.validators.include?(self)
+          klass = klass.superclass
+        end
+        criteria = create_criteria(klass, document, attribute, value)
+        criteria.exists?
+      end
+
+      # Create the validation criteria.
+      # @api private
+      def create_criteria(base, document, attribute, value)
+        criteria = scope(base, document)
+        filter_criteria(criteria, document, attribute)
+      end
+
+      # Scope the criteria to the scope options provided.
+      # @api private
+      def scope(criteria, document)
+        Array.wrap(options[:scope]).each do |item|
+          criteria = filter_criteria(criteria, document, item)
+        end
+        criteria
+      end
+
+      # Filter the criteria.
+      # @api private
+      def filter_criteria(criteria, document, attribute)
+        value = document.read_attribute(attribute)
+        value.nil? ? criteria.where("#{attribute}.null" => true) : criteria.where(attribute => value)
+      end
+    end
+  end
+end
+
 module ActivityNotification
   # Dynamoid extension module for ActivityNotification.
   module DynamoidExtension

lib/activity_notification/orm/dynamoid/subscription.rb

@@ -31,7 +31,7 @@ class Subscription
         global_secondary_index hash_key: :target_key, range_key: :created_at, projected_attributes: :all
 
         validates  :target,               presence: true
-        validates  :key,                  presence: true
+        validates  :key,                  presence: true, uniqueness: { scope: :target_key }
         validates_inclusion_of :subscribing,          in: [true, false]
         validates_inclusion_of :subscribing_to_email, in: [true, false]
         validate   :subscribing_to_email_cannot_be_true_when_subscribing_is_false

lib/activity_notification/orm/mongoid/subscription.rb

@@ -28,7 +28,7 @@ class Subscription
         field :optional_targets,          type: Hash,    default: {}
 
         validates  :target,               presence: true
-        validates  :key,                  presence: true
+        validates  :key,                  presence: true, uniqueness: { scope: :target }
         validates_inclusion_of :subscribing,          in: [true, false]
         validates_inclusion_of :subscribing_to_email, in: [true, false]
         validate   :subscribing_to_email_cannot_be_true_when_subscribing_is_false

spec/concerns/models/subscriber_spec.rb

@@ -74,9 +74,9 @@
       end
 
       context "without params" do
-        it "raises ActivityNotification::InvalidParameterError it is invalid" do
+        it "raises ActivityNotification::RecordInvalidError it is invalid" do
           expect { test_instance.create_subscription }
-          .to raise_error(ActivityNotification::InvalidParameterError)
+          .to raise_error(ActivityNotification::RecordInvalidError)
         end
       end
 
@@ -121,11 +121,11 @@
       end
 
       context "with false as subscribing and true as subscribing_to_email params" do
-        it "raises ActivityNotification::InvalidParameterError it is invalid" do
+        it "raises ActivityNotification::RecordInvalidError it is invalid" do
           expect {
             params = { key: 'key_1', subscribing: false, subscribing_to_email: true }
             test_instance.create_subscription(params)
-          }.to raise_error(ActivityNotification::InvalidParameterError)
+          }.to raise_error(ActivityNotification::RecordInvalidError)
         end
       end
 
@@ -160,11 +160,11 @@
       end
 
       context "with false as subscribing and true as optional_targets params" do
-        it "raises ActivityNotification::InvalidParameterError it is invalid" do
+        it "raises ActivityNotification::RecordInvalidError it is invalid" do
           expect {
             params = { key: 'key_1', subscribing: false, optional_targets: { subscribing_to_console_output: true } }
             test_instance.create_subscription(params)
-          }.to raise_error(ActivityNotification::InvalidParameterError)
+          }.to raise_error(ActivityNotification::RecordInvalidError)
         end
       end
     end