From 500d69dacfe6ab19de44029a53bc3ff682c5cd04 Mon Sep 17 00:00:00 2001 From: Alexandru Popescu Date: Tue, 28 Jan 2025 18:03:06 +0200 Subject: [PATCH] Add controller and test endpoint --- Gemfile | 3 + Gemfile.lock | 7 ++ README.md | 37 ++++--- app.rb | 6 -- app/controllers/coffee_shop_controller.rb | 79 +++++++++++++++ app/services/coffee_shop_finder_service.rb | 10 +- bin/{ruby-interview => start} | 2 +- config.ru | 7 ++ .../controller/coffee_shop_controller_spec.rb | 99 +++++++++++++++++++ spec/spec_helper.rb | 2 + 10 files changed, 229 insertions(+), 23 deletions(-) create mode 100644 app/controllers/coffee_shop_controller.rb rename bin/{ruby-interview => start} (56%) create mode 100644 config.ru create mode 100644 spec/controller/coffee_shop_controller_spec.rb diff --git a/Gemfile b/Gemfile index 2569a85..1313126 100644 --- a/Gemfile +++ b/Gemfile @@ -24,4 +24,7 @@ gem 'tzinfo-data', platforms: %i[mingw mswin x64_mingw jruby] gem 'csv' gem 'httparty' gem 'pry' +gem 'rack' +gem 'rack-test' +gem 'rackup' gem 'sinatra' diff --git a/Gemfile.lock b/Gemfile.lock index 97cc317..8988c22 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -40,6 +40,10 @@ GEM rack-session (2.1.0) base64 (>= 0.1.0) rack (>= 3.0.0) + rack-test (2.2.0) + rack (>= 1.3) + rackup (2.2.1) + rack (>= 3) rainbow (3.1.1) regexp_parser (2.10.0) rspec (3.13.0) @@ -91,6 +95,9 @@ DEPENDENCIES httparty pry puma (~> 6.5) + rack + rack-test + rackup rspec (~> 3.10) rubocop sinatra diff --git a/README.md b/README.md index 8f4adba..f62e8ec 100644 --- a/README.md +++ b/README.md @@ -2,13 +2,6 @@ This repo will serve as a starting point for your code challenge. Feel free to change anything in order to complete it: Change framework, other tests, new gems etc. -## Get this repo - -- Fork this repo -- Clone your fork - -## Prerequisites -- Have RVM installed: https://letmegooglethat.com/?q=install+rvm+on+ubuntu ## Local setup 1. Install ruby: `$ rvm install 3.4.1` @@ -16,18 +9,36 @@ This repo will serve as a starting point for your code challenge. Feel free to c 3. Install bundler: `$ gem install bundler` 4. Install the dependencies with bundler: `$ bundle install` -## Run sample CLI command -`$ bin/ruby-interview` +## Run CLI command to start the server +`$ bin/start` ## Run tests `$ bundle exec rspec` ## Tools -- Write HTTP APIs [rails](https://rubyonrails.org/) or [roda](https://roda.jeremyevans.net/documentation.html) or others -- Write CLI tools [thor](http://whatisthor.com/) or [tty](https://ttytoolkit.org/) or others (including [rake](https://github.com/ruby/rake)) -- Test your code with [rspec](https://rspec.info/) --- +## Use +# Start the server: `$ bin/start` +# Choose a provider to call the endpoind, I choose Thunder Client in VS Code. (Postman, Insomnia) +`http://localhost:9292/api/closest_shops?lat=47.6&lon=-122.4` +# Should see a reponse similar to: + + +# Also can test from terminal +'curl "http://localhost:9292/api/closest_shops?lat=47.6&lon=-122.4"' +'Coffee shops nearest (47.6, -122.4) by distance: + +0.0645 <--> Starbucks Seattle2 +0.0861 <--> Starbucks Seattle +10.0793 <--> Starbucks SF' + + +## Disclaimer + +# Authentication is not taken into account +# I did not deploy it anywhere -Good luck! +# I tought about setting up a DB, querying the endpoint to seed it and then seeding it periodically. +# But taking into the consideration the size of the csv file, the minimal number of requests I choose to prioritize always having the latest data and calling the endpoint through my CoffeeShopFinderServiceon each request diff --git a/app.rb b/app.rb index 080cbd1..3776579 100644 --- a/app.rb +++ b/app.rb @@ -1,9 +1,3 @@ # frozen_string_literal: true -# app.rb require_relative 'config/environment' - -# Mount the controller -map '/api' do - run CoffeeShopController -end diff --git a/app/controllers/coffee_shop_controller.rb b/app/controllers/coffee_shop_controller.rb new file mode 100644 index 0000000..fb28f30 --- /dev/null +++ b/app/controllers/coffee_shop_controller.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'sinatra/base' +require 'json' +require_relative '../services/coffee_shop_finder_service' + +class CoffeeShopController < Sinatra::Base + configure do + set :protection, except: [:host_authorization] + set :show_exceptions, false + set :raise_errors, true + enable :logging + end + + before do + content_type :json + end + + get '/closest_shops' do + content_type 'text/plain' + lat, lon = validate_coordinates!(params) + + shops_with_distances = coffee_shop_finder_service.closest_shops(lat, lon) + format_response(shops_with_distances, lat, lon) + rescue StandardError => e + handle_error(e) + end + + private + + # Format shops into "Name --> distance <-- (user-lat, user_lon)" strings + def format_response(shops_with_distances, user_lat, user_lon) + header = "Coffee shops nearest (#{user_lat}, #{user_lon}) by distance:\n\n" + + header + shops_with_distances.map do |shops_with_distance| + shop = shops_with_distance[:shop] + distance = shops_with_distance[:distance] + + "#{distance} <--> #{shop.name}" + end.join("\n") + end + + def validate_coordinates!(parameters) + error!(400, 'Invalid coordinates') unless parameters[:lat] && parameters[:lon] + error!(400, 'Coordinates must be numeric') unless numeric?(parameters[:lat]) && numeric?(parameters[:lon]) + + lat = parameters[:lat].to_f + lon = parameters[:lon].to_f + error!(400, 'Invalid coordinates') unless lat.between?(-90, 90) && lon.between?(-180, 180) + + [lat, lon] + end + + def numeric?(str) + return false unless str =~ /\A[-+]?[0-9]*\.?[0-9]+\Z/ + + Float(str) + end + + # Handle errors with appropriate HTTP status codes + def handle_error(error) + status_code = case error.message + when /Invalid CSV/ then 400 + when /Failed to fetch CSV/ then 502 + else 500 + end + + status status_code + { error: error.message }.to_json + end + + def error!(code, message) + halt code, { error: message }.to_json + end + + def coffee_shop_finder_service + CoffeeShopFinderService.new + end +end diff --git a/app/services/coffee_shop_finder_service.rb b/app/services/coffee_shop_finder_service.rb index b4d9332..52dc2f2 100644 --- a/app/services/coffee_shop_finder_service.rb +++ b/app/services/coffee_shop_finder_service.rb @@ -22,9 +22,9 @@ def fetch_and_parse_coffee_shops end def parse_coffee_shops(response) - CSV.parse(response.body, headers: true).map do |row| + CSV.parse(response.body, headers: headers).map do |row| validate_csv_row!(row) - CoffeeShop.new(row['Name'], row['X Coordinate'], row['Y Coordinate']) + CoffeeShop.new(row['Name'], row['Lat Coordinate'], row['Lon Coordinate']) end rescue CSV::MalformedCSVError => e raise "Malformed CSV: #{e.message}" @@ -40,7 +40,11 @@ def fetch_csv # Validate CSV row structure def validate_csv_row!(row) - missing = %w[Name X Coordinate Y Coordinate].reject { |h| row[h] } + missing = headers.reject { |h| row[h] } raise "Invalid CSV headers: #{missing.join(', ')}" if missing.any? end + + def headers + ['Name', 'Lat Coordinate', 'Lon Coordinate'] + end end diff --git a/bin/ruby-interview b/bin/start similarity index 56% rename from bin/ruby-interview rename to bin/start index 57f8e8c..c939cab 100755 --- a/bin/ruby-interview +++ b/bin/start @@ -1,4 +1,4 @@ #!/usr/bin/env ruby # frozen_string_literal: true -puts 'Good luck!' \ No newline at end of file +system('bundle exec rackup config.ru') diff --git a/config.ru b/config.ru new file mode 100644 index 0000000..28c5b99 --- /dev/null +++ b/config.ru @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require_relative 'app' + +map '/api' do + run CoffeeShopController +end diff --git a/spec/controller/coffee_shop_controller_spec.rb b/spec/controller/coffee_shop_controller_spec.rb new file mode 100644 index 0000000..7b658c4 --- /dev/null +++ b/spec/controller/coffee_shop_controller_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'rack/test' +require_relative '../../app/controllers/coffee_shop_controller' +require_relative '../../app/services/coffee_shop_finder_service' + +RSpec.describe CoffeeShopController do # rubocop:disable Metrics/BlockLength + include Rack::Test::Methods + + def app + @app ||= Rack::Builder.new do + map '/api' do + run CoffeeShopController + end + end + end + + before(:all) do + CoffeeShopController.set :environment, :test + end + + let(:valid_lat) { 40.7128 } + let(:valid_lon) { -74.0060 } + let(:invalid_coord) { 200.0 } + + let(:mock_shops) do + [ + { shop: double(name: 'Shop A', distance: 0.5), distance: 0.5 }, + { shop: double(name: 'Shop B', distance: 1.2), distance: 1.2 } + ] + end + + before do + allow_any_instance_of(CoffeeShopController) + .to receive(:format_response) do |_instance, _shops_with_distances, lat, lon| + "Coffee shops nearest (#{lat}, #{lon}) by distance:\n\n0.5 <--> Shop A\n1.2 <--> Shop B" + end + end + + describe 'GET /api/closest_shops' do # rubocop:disable Metrics/BlockLength + context 'with valid coordinates' do + before do + allow(CoffeeShopFinderService).to receive_message_chain(:new, :closest_shops).and_return(mock_shops) + end + + it 'returns a 200 OK' do + get '/api/closest_shops', lat: valid_lat, lon: valid_lon + expect(last_response.status).to eq(200) + expect(last_response.content_type).to include('text/plain') + expect(last_response.body).to include('0.5 <--> Shop A') + expect(last_response.body).to include('1.2 <--> Shop B') + end + end + + context 'with invalid coordinates' do + it 'returns 400 for missing lat' do + get '/api/closest_shops', lon: valid_lon + expect(last_response.status).to eq(400) + expect(JSON.parse(last_response.body)).to include('error' => 'Invalid coordinates') + end + + it 'returns 400 for non-numeric lat' do + get '/api/closest_shops', lat: 'abc', lon: valid_lon + expect(last_response.status).to eq(400) + end + + it 'returns 400 for out-of-range lat' do + get '/api/closest_shops', lat: invalid_coord, lon: valid_lon + expect(last_response.status).to eq(400) + end + end + + context 'when service raises errors' do + it 'returns 502 for CSV fetch failure' do + allow(CoffeeShopFinderService).to receive_message_chain(:new, :closest_shops) + .and_raise(StandardError.new('Failed to fetch CSV')) + + get '/api/closest_shops', lat: valid_lat, lon: valid_lon + expect(last_response.status).to eq(502) + end + + it 'returns 400 for invalid CSV data' do + allow(CoffeeShopFinderService).to receive_message_chain(:new, :closest_shops) + .and_raise(StandardError.new('Invalid CSV')) + + get '/api/closest_shops', lat: valid_lat, lon: valid_lon + expect(last_response.status).to eq(400) + end + + it 'returns 500 for unexpected errors' do + allow(CoffeeShopFinderService).to receive_message_chain(:new, :closest_shops) + .and_raise(StandardError.new('Unknown error')) + + get '/api/closest_shops', lat: valid_lat, lon: valid_lon + expect(last_response.status).to eq(500) + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1f0af8b..8800722 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,6 +18,7 @@ require 'byebug' require 'pry' +require 'rack/test' RSpec.configure do |config| # rspec-expectations config goes here. You can use an alternate @@ -95,4 +96,5 @@ # test failures related to randomization by passing the same `--seed` value # as the one that triggered the failure. Kernel.srand config.seed + config.include Rack::Test::Methods end