From fba2039efcfc35968e78df2fba1c75846443db29 Mon Sep 17 00:00:00 2001 From: sjdonado Date: Tue, 18 Mar 2025 11:04:50 +0100 Subject: [PATCH] refactor: request thread safety context --- app/controllers/link.cr | 133 ++++++++++++++++++++-------------------- app/controllers/ping.cr | 13 ++-- app/lib/controller.cr | 42 +++++++++---- app/routes.cr | 38 +++++++----- 4 files changed, 126 insertions(+), 100 deletions(-) diff --git a/app/controllers/link.cr b/app/controllers/link.cr index 6072166..79ade57 100644 --- a/app/controllers/link.cr +++ b/app/controllers/link.cr @@ -1,7 +1,4 @@ require "uuid" -require "user_agent_parser" - -require "../lib/controller.cr" module App::Controllers class LinkController < App::Lib::BaseController @@ -9,17 +6,20 @@ module App::Controllers include App::Lib include App::Services - ClickTracker.init + def initialize(@env : HTTP::Server::Context) + ClickTracker.init + super(@env) + end - def create(env) - user = env.get("user").as(User) - body = parse_body(env, ["url"]) + def create + user = current_user + body = parse_body(["url"]) url = body["url"].to_s query = Database::Query.where(url: url, user_id: user.id.as(String)).limit(1) existing_link = Database.all(Link, query, preload: [:clicks]).first? if existing_link - return link_response(existing_link) + return render_json({"data" => App::Serializers::Link.new(existing_link)}) end link = Link.new @@ -30,42 +30,41 @@ module App::Controllers changeset = Database.insert(link) if !changeset.valid? - raise App::UnprocessableEntityException.new(env, map_changeset_errors(changeset.errors)) + raise App::UnprocessableEntityException.new(@env, map_changeset_errors(changeset.errors)) end link.clicks = [] of App::Models::Click - link_response(link) + render_json({"data" => App::Serializers::Link.new(link)}, 201) end - def redirect(env) - slug = env.params.url["slug"] + def redirect + slug = @env.params.url["slug"] - link = nil - Database.raw_query("SELECT id, slug, url FROM links WHERE slug = (?) LIMIT 1", slug) do |result| + link_data = nil + Database.raw_query("SELECT id, url FROM links WHERE slug = (?) LIMIT 1", slug) do |result| if result.move_next - link = { - id: result.read(String), - url: result.read(String), - } + link_data = {result.read(String), result.read(String)} end end - raise App::NotFoundException.new(env) if !link + raise App::NotFoundException.new(@env) unless link_data - remote_address = env.request.headers["Cf-Connecting-Ip"]?.try(&.presence) || env.request.remote_address.try &.to_s - user_agent_str = env.request.headers["User-Agent"]? || "Unknown" + remote_address = @env.request.headers["Cf-Connecting-Ip"]?.try(&.presence) || @env.request.remote_address.try &.to_s + user_agent_str = @env.request.headers["User-Agent"]? || "Unknown" client_ip = IpLookup.extract_ip(remote_address) || "Unknown" - env.response.status_code = 301 - env.response.headers["Location"] = link.not_nil![:url] - env.response.headers["X-Forwarded-For"] = client_ip - env.response.headers["User-Agent"] = user_agent_str + @env.response.status_code = 301 + @env.response.headers["Connection"] = "close" - spawn track_click(env, link.not_nil![:id], client_ip, user_agent_str) + @env.response.headers["Location"] = link_data[1] + @env.response.headers["X-Forwarded-For"] = client_ip + @env.response.headers["User-Agent"] = user_agent_str + + spawn track_click(link_data[0], client_ip, user_agent_str) end - def list_all(env) - user = env.get("user").as(User) - limit, cursor = pagination_params(env) + def list_all + user = current_user + limit, cursor = pagination_params query = Database::Query.where(user_id: user.id.as(String)) query = query.where("id < ?", cursor) if cursor @@ -75,32 +74,32 @@ module App::Controllers return paginated_response(links, limit) { |link| App::Serializers::Link.new(link) } end - def get(env) - user = env.get("user").as(User) - link_id = env.params.url["id"] + def get + user = current_user + link_id = @env.params.url["id"] query = Database::Query.where(id: link_id.as(String), user_id: user.id.as(String)).limit(1) link = Database.all(Link, query).first? - raise App::NotFoundException.new(env) if link.nil? + raise App::NotFoundException.new(@env) if link.nil? clicks_query = Database::Query.where(link_id: link_id.as(String)) .order_by("id DESC") .limit(100) link.clicks = Database.all(Click, clicks_query) - link_response(link) + render_json({"data" => App::Serializers::Link.new(link)}) end - def list_clicks(env) - user = env.get("user").as(User) - link_id = env.params.url["id"] + def list_clicks + user = current_user + link_id = @env.params.url["id"] # Verify link exists and belongs to user link_query = Database::Query.where(id: link_id.as(String), user_id: user.id.as(String)).limit(1) link = Database.all(Link, link_query).first? - raise App::NotFoundException.new(env) if link.nil? + raise App::NotFoundException.new(@env) if link.nil? - limit, cursor = pagination_params(env) + limit, cursor = pagination_params query = Database::Query.where(link_id: link_id.as(String)) query = query.where("id < ?", cursor) if cursor @@ -110,22 +109,22 @@ module App::Controllers return paginated_response(clicks, limit) { |click| App::Serializers::Click.new(click) } end - def update(env) - user = env.get("user").as(User) - id = env.params.url["id"] - body = parse_body(env, ["url"]) + def update + user = current_user + id = @env.params.url["id"] + body = parse_body(["url"]) new_url = body["url"].to_s query = Database::Query.where(id: id).limit(1) link = Database.all(Link, query, preload: [:clicks]).first? - raise App::NotFoundException.new(env) if link.nil? - raise App::ForbiddenException.new(env) if link.user_id != user.id + raise App::NotFoundException.new(@env) if link.nil? + raise App::ForbiddenException.new(@env) if link.user_id != user.id # Check for existing URL existing_query = Database::Query.where(url: new_url, user_id: user.id.to_s).limit(1) if Database.all(Link, existing_query).first? - raise App::UnprocessableEntityException.new(env, { "url" => ["URL already exists"] }) + raise App::UnprocessableEntityException.new(@env, { "url" => ["URL already exists"] }) end link.url = new_url @@ -133,31 +132,35 @@ module App::Controllers changeset = Database.update(link) if !changeset.valid? - raise App::UnprocessableEntityException.new(env, map_changeset_errors(changeset.errors)) + raise App::UnprocessableEntityException.new(@env, map_changeset_errors(changeset.errors)) end - link_response(link) + render_json({"data" => App::Serializers::Link.new(link)}) end - def delete(env) - user = env.get("user").as(User) - id = env.params.url["id"] + def delete + user = current_user + id = @env.params.url["id"] link = Database.get(Link, id) - raise App::NotFoundException.new(env) if !link - raise App::ForbiddenException.new(env) if link.user_id != user.id + raise App::NotFoundException.new(@env) if !link + raise App::ForbiddenException.new(@env) if link.user_id != user.id result = Database.raw_exec("DELETE FROM links WHERE id = (?)", link.id) if result.rows_affected == 0 - raise App::UnprocessableEntityException.new(env, { "id" => ["Row delete failed"] }) + raise App::UnprocessableEntityException.new(@env, { "id" => ["Row delete failed"] }) end - env.response.status_code = 204 + @env.response.status_code = 204 end - private def track_click(env, link_id, client_ip, user_agent_str) - source = env.params.query["utm_source"]? || "Direct" - referer = env.request.headers["Referer"]?.try { |r| begin URI.parse(r).host rescue r end } || source + private def current_user : User + @env.get("user").as(User) + end + + private def track_click(link_id, client_ip, user_agent_str) + source = @env.params.query["utm_source"]? || "Direct" + referer = @env.request.headers["Referer"]?.try { |r| begin URI.parse(r).host rescue r end } || source ClickTracker.track( link_id: link_id, @@ -168,28 +171,24 @@ module App::Controllers ) end - private def pagination_params(env) - limit = (env.params.query["limit"]? || "100").to_i - cursor = env.params.query["cursor"]? + private def pagination_params + limit = (@env.params.query["limit"]? || "100").to_i + cursor = @env.params.query["cursor"]? {limit, cursor} end - private def link_response(link) - {"data" => App::Serializers::Link.new(link)}.to_json - end - private def paginated_response(items, limit) has_more = items.size > limit items = items[0...limit] if has_more next_cursor = has_more ? items.last.id : nil - { + render_json({ "data" => items.map { |item| yield item }, "pagination" => { "has_more" => has_more, "next" => next_cursor } - }.to_json + }) end end end diff --git a/app/controllers/ping.cr b/app/controllers/ping.cr index e73554d..c707048 100644 --- a/app/controllers/ping.cr +++ b/app/controllers/ping.cr @@ -1,10 +1,13 @@ require "../lib/controller.cr" -module App::Controllers::Ping - class Get < App::Lib::BaseController - def call(env) - response = {"data" => "pong"} - response.to_json +module App::Controllers + class PingController < App::Lib::BaseController + def initialize(@env : HTTP::Server::Context) + super(@env) + end + + def ping + render_json({data: "pong"}) end end end diff --git a/app/lib/controller.cr b/app/lib/controller.cr index 0fd6808..e721583 100644 --- a/app/lib/controller.cr +++ b/app/lib/controller.cr @@ -1,29 +1,45 @@ module App::Lib abstract class BaseController - def map_changeset_errors(errors) + protected getter env : HTTP::Server::Context + + def initialize(@env : HTTP::Server::Context); end + + # Convert changeset errors to API-friendly format + protected def map_changeset_errors(errors) errors.reduce({} of String => Array(String)) do |memo, error| - memo[error[:field]] = memo[error[:field]]? || [] of String - memo[error[:field]] << error[:message] + field = error[:field].to_s + message = error[:message].to_s + + memo[field] ||= [] of String + memo[field] << message memo end end - def parse_body(env, fields) - json_params = env.params.json.to_h - missing_fields = [] of String + protected def parse_body(required_fields : Array(String) = [] of String) + json_params = @env.params.json.try(&.to_h) || {} of String => JSON::Any + json_params = json_params.transform_values(&.to_s) # Convert JSON::Any to String - fields.each do |field| - unless json_params.has_key?(field) - missing_fields << field - end - end + missing_fields = required_fields.reject { |field| json_params.has_key?(field) } unless missing_fields.empty? - error_message = missing_fields.map { |field| "#{field}: Required field" }.join(", ") - raise App::BadRequestException.new(env, error_message) + error_message = missing_fields.join(", ") + " required" + raise App::BadRequestException.new(@env, error_message) end json_params end + + protected def render_json(data, status_code : Int32 = 200) + @env.response.status_code = status_code + @env.response.content_type = "application/json" + data.to_json + end + + protected def param(key : String) : String + @env.params.url[key] + rescue KeyError + raise App::BadRequestException.new(@env, "Missing required parameter: #{key}") + end end end diff --git a/app/routes.cr b/app/routes.cr index 73e773a..f74e970 100644 --- a/app/routes.cr +++ b/app/routes.cr @@ -1,45 +1,53 @@ require "./controllers/**" module App + # CORS handling middleware before_all do |env| - env.response.headers["Access-Control-Allow-Origin"] = "*" - env.response.headers["Access-Control-Allow-Methods"] = "GET, POST, PUT, DELETE, OPTIONS" - env.response.headers["Access-Control-Allow-Headers"] = "Content-Type, Accept, Origin, X-Api-Key" + if env.request.path.starts_with?("/api/") + env.response.headers["Access-Control-Allow-Origin"] = "*" + env.response.headers["Access-Control-Allow-Methods"] = "GET, POST, PUT, DELETE, OPTIONS" + env.response.headers["Access-Control-Allow-Headers"] = "Content-Type, Accept, Origin, X-Api-Key" + end end - after_all do |env| - env.response.content_type = "application/json" + # Error handling middleware + error 404 do |env| + {error: "Not Found"}.to_json end - - get "/api/ping" do |env| - Controllers::Ping::Get.new.call(env) + error 500 do |env| + {error: "Internal Server Error"}.to_json end get "/:slug" do |env| - Controllers::Link::Index.new.call(env) + Controllers::LinkController.new(env).redirect + end + + # namespace /api + get "/api/ping" do |env| + Controllers::PingController.new(env).ping end get "/api/links" do |env| - Controllers::Link::All.new.call(env) + Controllers::LinkController.new(env).list_all end get "/api/links/:id" do |env| - Controllers::Link::Get.new.call(env) + Controllers::LinkController.new(env).get end get "/api/links/:id/clicks" do |env| - Controllers::Link::Clicks.new.call(env) + Controllers::LinkController.new(env).list_clicks end post "/api/links" do |env| - Controllers::Link::Create.new.call(env) + Controllers::LinkController.new(env).create end put "/api/links/:id" do |env| - Controllers::Link::Update.new.call(env) + Controllers::LinkController.new(env).update end delete "/api/links/:id" do |env| - Controllers::Link::Delete.new.call(env) + Controllers::LinkController.new(env).delete end end