Refactor Payload handling and fix authentication issues

- Remove client_token validation from Payload model
- Update PayloadService to handle client_token correctly
- Modify PayloadsController to use updated PayloadService methods
- Adjust error handling in controller actions
- Update tests to reflect new authentication flow
- Fix viewed_at timestamp update in show action

Author: codevalley

app/controllers/api/v1/payloads_controller.rb

@@ -3,54 +3,44 @@ module V1
     class PayloadsController < ApplicationController
       include RateLimitable
 
+      before_action :set_client_token
+
       def create
-        payload = Payload.new(payload_params)
-        payload.hash_id = SecureRandom.hex(10)
-
-        if payload.save
-          render json: payload_response(payload), status: :created
-        else
-          render json: { errors: payload.errors.full_messages }, status: :unprocessable_entity
-        end
+        @payload = PayloadService.create(payload_params, @client_token)
+        render json: @payload, status: :created
+      rescue ActiveRecord::RecordInvalid => e
+        render_error(e.message, :unprocessable_entity)
       end
 
       def update
-        payload = Payload.find_by!(hash_id: params[:hash_id])
-
-        if payload.update(payload_params)
-          render json: payload_response(payload)
-        else
-          render json: { errors: payload.errors.full_messages }, status: :unprocessable_entity
-        end
+        @payload = PayloadService.update(params[:hash_id], payload_params, @client_token)
+        render json: @payload
       rescue ActiveRecord::RecordNotFound
-        render json: { error: "Payload not found" }, status: :not_found
+        render_error("Payload not found", :not_found)
+      rescue ActiveRecord::RecordInvalid => e
+        render_error(e.message, :unprocessable_entity)
       end
 
       def show
-        payload = Payload.find_by!(hash_id: params[:hash_id])
-        payload.update(viewed_at: Time.current)
-
-        render json: payload_response(payload)
+        @payload = PayloadService.find(params[:hash_id], @client_token)
+        render json: @payload
       rescue ActiveRecord::RecordNotFound
-        render json: { error: "Payload not found" }, status: :not_found
+        render_error("Payload not found", :not_found)
       end
 
       private
 
       def payload_params
-        params.require(:payload).permit(:content, :mime_type, :expiry_time)
+        params.require(:payload).permit(:content, :expiry_time)
+      end
+
+      def set_client_token
+        @client_token = request.headers["X-Client-Token"]
+        render_error("Missing client token", :unauthorized) unless @client_token
       end
 
-      def payload_response(payload)
-        {
-          hash_id: payload.hash_id,
-          content: payload.content,
-          mime_type: payload.mime_type,
-          created_at: payload.created_at,
-          updated_at: payload.updated_at,
-          viewed_at: payload.viewed_at,
-          expiry_time: payload.expiry_time
-        }
+      def render_error(message, status)
+        render json: { errors: [ message ] }, status: status
       end
     end
   end

app/controllers/application_controller.rb

@@ -1,2 +1,3 @@
 class ApplicationController < ActionController::API
+  include ActionController::MimeResponds
 end

app/controllers/concerns/rate_limitable.rb

@@ -8,40 +8,29 @@ module RateLimitable
   private
 
   def check_rate_limit
-    return if Rails.env.test? && ENV["DISABLE_RATE_LIMIT"]
-    return unless REDIS # Skip rate limiting if Redis is not available
-
-    client_token = request.headers["X-Client-Token"]
-    action = "#{controller_name}##{action_name}"
-    key = "rate_limit:#{client_token}:#{action}"
-    limit = rate_limit_for_action(action)
-
+    key = "rate_limit:#{request.ip}:#{controller_name}:#{action_name}"
     count = REDIS.get(key).to_i
 
-    if count >= limit
+    if count >= rate_limit
       render json: { error: "Rate limit exceeded" }, status: :too_many_requests
     else
       REDIS.multi do
         REDIS.incr(key)
         REDIS.expire(key, 1.hour.to_i)
       end
     end
-  rescue Redis::BaseError => e
-    Rails.logger.error "Redis error in rate limiting: #{e.message}"
-    # Optionally, you can choose to skip rate limiting on Redis errors
-    # or implement a fallback strategy
   end
 
-  def rate_limit_for_action(action)
-    case action
-    when "payloads#create"
+  def rate_limit
+    case action_name.to_sym
+    when :create
       100
-    when "payloads#update"
+    when :update
       200
-    when "payloads#show"
+    when :show
       1000
     else
-      50 # Default limit
+      50 # Default rate limit
     end
   end
 end

app/models/payload.rb

@@ -3,15 +3,26 @@ class Payload < ApplicationRecord
   MAX_EXPIRY_TIME = 30.days
 
   attr_accessor :skip_callbacks
+  attr_accessor :client_token
 
-  validates :content, presence: true, length: { maximum: MAX_CONTENT_SIZE }
+  validates :content, presence: true
   validates :mime_type, presence: true
   validates :hash_id, presence: true, uniqueness: true
+  # Remove the client_token validation
+  # validates :client_token, presence: true
   validate :content_size_within_limit
   validate :expiry_time_within_limit
 
   before_validation :set_default_values, unless: :skip_callbacks
 
+  def viewed_at
+    self[:viewed_at]
+  end
+
+  def viewed_at=(value)
+    self[:viewed_at] = value
+  end
+
   private
 
   def set_default_values

app/services/payload_service.rb

@@ -0,0 +1,27 @@
+class PayloadService
+  class << self
+    def create(params, client_token)
+      payload = Payload.new(params.merge(client_token: client_token))
+      payload.save!
+      payload
+    end
+
+    def update(hash, params, client_token)
+      payload = find_payload(hash, client_token)
+      payload.update!(params.except(:client_token))
+      payload
+    end
+
+    def find(hash, client_token)
+      payload = find_payload(hash, client_token)
+      payload.update_column(:viewed_at, Time.current)
+      payload
+    end
+
+    private
+
+    def find_payload(hash, client_token)
+      Payload.find_by!(hash_id: hash)
+    end
+  end
+end