From 41f87de7d0caba27259c3177dae5b11db775c57f Mon Sep 17 00:00:00 2001 From: "G. Wade Johnson" Date: Mon, 12 Oct 2015 07:43:40 -0500 Subject: [PATCH 1/8] Release 0: Properly using migrations. Instead of creating by hand, I used rails generate to build the initial model. --- source/app/controllers/urls_controller.rb | 32 +++++++++++++++++++ source/app/helpers/urls_helper.rb | 6 ++++ source/app/models/url.rb | 9 ++++++ source/app/views/layouts/application.html.erb | 2 +- source/app/views/urls/index.html.erb | 16 ++++++++++ source/app/views/urls/new.html.erb | 11 +++++++ source/app/views/urls/show.html.erb | 19 +++++++++++ source/config/routes.rb | 3 ++ .../db/migrate/20151012124222_create_urls.rb | 11 +++++++ source/db/schema.rb | 25 +++++++++++++++ source/db/seeds.rb | 2 ++ source/spec/models/url_spec.rb | 5 +++ 12 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 source/app/models/url.rb create mode 100644 source/app/views/urls/index.html.erb create mode 100644 source/app/views/urls/new.html.erb create mode 100644 source/app/views/urls/show.html.erb create mode 100644 source/db/migrate/20151012124222_create_urls.rb create mode 100644 source/db/schema.rb create mode 100644 source/spec/models/url_spec.rb diff --git a/source/app/controllers/urls_controller.rb b/source/app/controllers/urls_controller.rb index ef26710..3054316 100644 --- a/source/app/controllers/urls_controller.rb +++ b/source/app/controllers/urls_controller.rb @@ -1,2 +1,34 @@ class UrlsController < ApplicationController + def index + # for now, lets grab them all and pass them to a display page + @urls = Url.all + end + + def new + # default behavior looks good + end + + def create + # Actually create the new shortened url + target= params[:target] + + @url = Url.new(target_link: target) + if !@url.save + flash[:save_error] = ['Unable to save URL. Please try another.']; + flash[:target] = target + render 'new' + else + redirect_to url_path(@url.linkid) + end + end + + def show + @url = Url.find_by(linkid: params[:id]) + end + + def follow + # redirect to the long url + url = Url.find_by(linkid: params[:linkid]) + redirect_to url.target_link + end end diff --git a/source/app/helpers/urls_helper.rb b/source/app/helpers/urls_helper.rb index 83216b1..82acfd3 100644 --- a/source/app/helpers/urls_helper.rb +++ b/source/app/helpers/urls_helper.rb @@ -1,2 +1,8 @@ module UrlsHelper + module_function + LINKID_LENGTH = 8 + + def shortened_linkid(url) + Digest::SHA1.base64digest(url)[0, LINKID_LENGTH].tr('+/','-_') + end end diff --git a/source/app/models/url.rb b/source/app/models/url.rb new file mode 100644 index 0000000..6bb1ded --- /dev/null +++ b/source/app/models/url.rb @@ -0,0 +1,9 @@ +class Url < ActiveRecord::Base + before_save :ensure_short_link + + def ensure_short_link + if self.linkid.nil? || self.linkid.length == 0 + self.linkid = UrlsHelper.shortened_linkid(self.target_link) + end + end +end diff --git a/source/app/views/layouts/application.html.erb b/source/app/views/layouts/application.html.erb index f946432..b488b3b 100644 --- a/source/app/views/layouts/application.html.erb +++ b/source/app/views/layouts/application.html.erb @@ -1,7 +1,7 @@ - Source + URL Shortener <%= stylesheet_link_tag 'application', media: 'all', 'data-turbolinks-track' => true %> <%= javascript_include_tag 'application', 'data-turbolinks-track' => true %> <%= csrf_meta_tags %> diff --git a/source/app/views/urls/index.html.erb b/source/app/views/urls/index.html.erb new file mode 100644 index 0000000..1c3f58a --- /dev/null +++ b/source/app/views/urls/index.html.erb @@ -0,0 +1,16 @@ +

URL Shortcuts

+ +Get new shortened URL +<% if !@urls.nil? && @urls.size != 0 %> + + + + +<% @urls.each do |url| %> + + +<% end %> +
Shortened IDTarget
<%= url.linkid %><% url.target_link %>
+<% else %> +

No shortened URLs found

+<% end %> diff --git a/source/app/views/urls/new.html.erb b/source/app/views/urls/new.html.erb new file mode 100644 index 0000000..965dcaf --- /dev/null +++ b/source/app/views/urls/new.html.erb @@ -0,0 +1,11 @@ +

URL Shortener

+<% if flash[:save_error] %> +

<%= flash[:save_error] %>

+<% end %> +
+ + + + +
+Back to list diff --git a/source/app/views/urls/show.html.erb b/source/app/views/urls/show.html.erb new file mode 100644 index 0000000..495ca72 --- /dev/null +++ b/source/app/views/urls/show.html.erb @@ -0,0 +1,19 @@ +

URL Shortcut

+ +Back to list + +<% if @url.nil? %> +

Unrecognized Shortcut.

+<% else %> + + + + + + + + + +
Shortened Link<%= follow_url(@url.linkid) %>
Target Link<%= @url.target_link %>
+<% end %> + diff --git a/source/config/routes.rb b/source/config/routes.rb index 3f66539..7145cea 100644 --- a/source/config/routes.rb +++ b/source/config/routes.rb @@ -13,6 +13,9 @@ # Example resource route (maps HTTP verbs to controller actions automatically): # resources :products + resources :urls, except: [ :edit, :update, :destroy ] + + get '/:linkid', to: 'urls#follow', as: :follow, constraints: { linkid: /[-_a-zA-Z0-9]{8}/ } # Example resource route with options: # resources :products do diff --git a/source/db/migrate/20151012124222_create_urls.rb b/source/db/migrate/20151012124222_create_urls.rb new file mode 100644 index 0000000..540d927 --- /dev/null +++ b/source/db/migrate/20151012124222_create_urls.rb @@ -0,0 +1,11 @@ +class CreateUrls < ActiveRecord::Migration + def change + create_table :urls do |t| + t.string :linkid, limit: 8 + t.string :target_link, limit: 1024 + + t.timestamps + end + add_index :urls, :linkid, unique: true + end +end diff --git a/source/db/schema.rb b/source/db/schema.rb new file mode 100644 index 0000000..d0cbae7 --- /dev/null +++ b/source/db/schema.rb @@ -0,0 +1,25 @@ +# encoding: UTF-8 +# This file is auto-generated from the current state of the database. Instead +# of editing this file, please use the migrations feature of Active Record to +# incrementally modify your database, and then regenerate this schema definition. +# +# Note that this schema.rb definition is the authoritative source for your +# database schema. If you need to create the application database on another +# system, you should be using db:schema:load, not running all the migrations +# from scratch. The latter is a flawed and unsustainable approach (the more migrations +# you'll amass, the slower it'll run and the greater likelihood for issues). +# +# It's strongly recommended that you check this file into your version control system. + +ActiveRecord::Schema.define(version: 20151012124222) do + + create_table "urls", force: true do |t| + t.string "linkid", limit: 8 + t.string "target_link", limit: 1024 + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index "urls", ["linkid"], name: "index_urls_on_linkid", unique: true + +end diff --git a/source/db/seeds.rb b/source/db/seeds.rb index 4edb1e8..0214630 100644 --- a/source/db/seeds.rb +++ b/source/db/seeds.rb @@ -5,3 +5,5 @@ # # cities = City.create([{ name: 'Chicago' }, { name: 'Copenhagen' }]) # Mayor.create(name: 'Emanuel', city: cities.first) +Url.create([{target_link: 'https://google.com'}, + {target_link: 'http://guides.rubyonrails.org/v4.1.12/action_controller_overview.html'}]) diff --git a/source/spec/models/url_spec.rb b/source/spec/models/url_spec.rb new file mode 100644 index 0000000..209ca4c --- /dev/null +++ b/source/spec/models/url_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe Url, :type => :model do + pending "add some examples to (or delete) #{__FILE__}" +end From a7c7a5a04a3f66afbc6077449a2ac2f80de6fb0d Mon Sep 17 00:00:00 2001 From: "G. Wade Johnson" Date: Mon, 12 Oct 2015 12:34:32 -0500 Subject: [PATCH 2/8] Use increment_counter, supposedly safer. --- source/app/controllers/urls_controller.rb | 3 ++- source/app/views/urls/index.html.erb | 2 +- source/app/views/urls/show.html.erb | 4 ++++ source/db/migrate/20151012132805_add_click_count_to_url.rb | 5 +++++ source/db/schema.rb | 3 ++- 5 files changed, 14 insertions(+), 3 deletions(-) create mode 100644 source/db/migrate/20151012132805_add_click_count_to_url.rb diff --git a/source/app/controllers/urls_controller.rb b/source/app/controllers/urls_controller.rb index 3054316..3583efd 100644 --- a/source/app/controllers/urls_controller.rb +++ b/source/app/controllers/urls_controller.rb @@ -26,9 +26,10 @@ def show @url = Url.find_by(linkid: params[:id]) end + # redirect to the long url def follow - # redirect to the long url url = Url.find_by(linkid: params[:linkid]) + Url.increment_counter( :click_count, url.id ) redirect_to url.target_link end end diff --git a/source/app/views/urls/index.html.erb b/source/app/views/urls/index.html.erb index 1c3f58a..7e5ea5b 100644 --- a/source/app/views/urls/index.html.erb +++ b/source/app/views/urls/index.html.erb @@ -8,7 +8,7 @@ <% @urls.each do |url| %> <%= url.linkid %> - <% url.target_link %> + <%= url.target_link %> <% end %> <% else %> diff --git a/source/app/views/urls/show.html.erb b/source/app/views/urls/show.html.erb index 495ca72..0b1135b 100644 --- a/source/app/views/urls/show.html.erb +++ b/source/app/views/urls/show.html.erb @@ -14,6 +14,10 @@ Target Link <%= @url.target_link %> + + Click Count + <%= @url.click_count %> + <% end %> diff --git a/source/db/migrate/20151012132805_add_click_count_to_url.rb b/source/db/migrate/20151012132805_add_click_count_to_url.rb new file mode 100644 index 0000000..a8d8038 --- /dev/null +++ b/source/db/migrate/20151012132805_add_click_count_to_url.rb @@ -0,0 +1,5 @@ +class AddClickCountToUrl < ActiveRecord::Migration + def change + add_column :urls, :click_count, :integer, :default => 0 + end +end diff --git a/source/db/schema.rb b/source/db/schema.rb index d0cbae7..e5d1be5 100644 --- a/source/db/schema.rb +++ b/source/db/schema.rb @@ -11,13 +11,14 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20151012124222) do +ActiveRecord::Schema.define(version: 20151012132805) do create_table "urls", force: true do |t| t.string "linkid", limit: 8 t.string "target_link", limit: 1024 t.datetime "created_at" t.datetime "updated_at" + t.integer "click_count", default: 0 end add_index "urls", ["linkid"], name: "index_urls_on_linkid", unique: true From 91c758ee45aa151518746679ca15cd4577c692bb Mon Sep 17 00:00:00 2001 From: "G. Wade Johnson" Date: Mon, 12 Oct 2015 19:09:29 -0500 Subject: [PATCH 3/8] Release 2: Add validation support After a challenge from Mike on the last version, I tried to do this without building a whole, separate validation class. I couldn't get message to work on the 'presence' validation, so I did a separate validation function for it. I could have combined all of these into one large validation function, but I decided it was more interesting to make a number of simple validation functions instead. --- source/app/assets/stylesheets/urls.css.scss | 3 ++ source/app/controllers/urls_controller.rb | 19 ++++++++--- source/app/models/url.rb | 37 ++++++++++++++++++++- source/app/views/urls/new.html.erb | 8 +++-- 4 files changed, 59 insertions(+), 8 deletions(-) diff --git a/source/app/assets/stylesheets/urls.css.scss b/source/app/assets/stylesheets/urls.css.scss index a4281ec..7f7b3c3 100644 --- a/source/app/assets/stylesheets/urls.css.scss +++ b/source/app/assets/stylesheets/urls.css.scss @@ -1,3 +1,6 @@ // Place all the styles related to the Urls controller here. // They will automatically be included in application.css. // You can use Sass (SCSS) here: http://sass-lang.com/ +.error { + color: red; +} diff --git a/source/app/controllers/urls_controller.rb b/source/app/controllers/urls_controller.rb index 3583efd..dcf391a 100644 --- a/source/app/controllers/urls_controller.rb +++ b/source/app/controllers/urls_controller.rb @@ -6,16 +6,27 @@ def index def new # default behavior looks good + @target = '' + @save_errors = nil end def create # Actually create the new shortened url - target= params[:target] + @target = params[:target] + @errors = nil - @url = Url.new(target_link: target) + @url = Url.new(target_link: @target) if !@url.save - flash[:save_error] = ['Unable to save URL. Please try another.']; - flash[:target] = target + case + when @url.errors[:target_link].any? + @errors = @url.errors[:target_link] + when @url.errors[:linkid].any? + @errors = @url.errors[:linkid] + when @url.errors[:base].any? + @errors = @url.errors[:base] + else + @errors = ['Unable to save URL. Please try another.']; + end render 'new' else redirect_to url_path(@url.linkid) diff --git a/source/app/models/url.rb b/source/app/models/url.rb index 6bb1ded..276a137 100644 --- a/source/app/models/url.rb +++ b/source/app/models/url.rb @@ -1,8 +1,43 @@ +require 'uri' +require 'net/http' + class Url < ActiveRecord::Base before_save :ensure_short_link + validates :linkid, uniqueness: true + validate :target_link_not_empty, :target_link_must_be_http, :target_link_must_be_reachable + + def target_link_not_empty + return unless errors.empty? + if target_link.empty? + errors[:target_link] << 'Target URL cannot be blank'; + end + end + + def target_link_must_be_http + return unless errors.empty? + unless target_link.match(%r{^https?://}) + errors[:target_link] << 'Target URL has an unrecognized scheme'; + end + end + + def target_link_must_be_reachable + return unless errors.empty? + begin + uri = URI.parse(target_link) + resp = Net::HTTP.get_response(uri) + if resp.code.to_i >= 400 + errors[:target_link] << 'Target URL is not found' + end + rescue URI::InvalidURIError + errors[:target_link] << 'Target URL is not valid' + return + rescue + errors[:target_link] << 'Target URL is not reachable' + end + end def ensure_short_link - if self.linkid.nil? || self.linkid.length == 0 + if self.linkid.nil? || self.linkid.empty? self.linkid = UrlsHelper.shortened_linkid(self.target_link) end end diff --git a/source/app/views/urls/new.html.erb b/source/app/views/urls/new.html.erb index 965dcaf..b22d9cc 100644 --- a/source/app/views/urls/new.html.erb +++ b/source/app/views/urls/new.html.erb @@ -1,11 +1,13 @@

URL Shortener

-<% if flash[:save_error] %> -

<%= flash[:save_error] %>

+<% if !@errors.nil? && @errors.size > 0 %> + <% @errors.each do |err| %> +

<%= err %>

+ <% end %> <% end %>
- +
Back to list From 637b3dc2964a67d90b5f043bd0578dc46251ffc3 Mon Sep 17 00:00:00 2001 From: "G. Wade Johnson" Date: Tue, 13 Oct 2015 14:18:58 -0500 Subject: [PATCH 4/8] Add some spec tests for model code --- source/Gemfile | 8 ++- source/spec/helpers/urls_helper_spec.rb | 18 ++++++ source/spec/models/url_spec.rb | 80 ++++++++++++++++++++++- source/spec/rails_helper.rb | 50 +++++++++++++++ source/spec/spec_helper.rb | 85 +++++++++++++++++++++++++ 5 files changed, 237 insertions(+), 4 deletions(-) create mode 100644 source/spec/helpers/urls_helper_spec.rb create mode 100644 source/spec/rails_helper.rb create mode 100644 source/spec/spec_helper.rb diff --git a/source/Gemfile b/source/Gemfile index 9627b8b..f5b9534 100644 --- a/source/Gemfile +++ b/source/Gemfile @@ -26,8 +26,11 @@ gem 'sdoc', '~> 0.4.0', group: :doc # Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring gem 'spring', group: :development +# Bootstrap +gem 'bootstrap-sass', '3.2.0' + # Use ActiveModel has_secure_password -# gem 'bcrypt', '~> 3.1.7' +gem 'bcrypt', '~> 3.1.7' # Use unicorn as the app server # gem 'unicorn' @@ -37,5 +40,6 @@ gem 'spring', group: :development # Use debugger # gem 'debugger', group: [:development, :test] - gem 'rspec-rails', group: [:development, :test] +gem 'rspec-rails', group: [:development, :test] +gem 'autoprefixer-rails' diff --git a/source/spec/helpers/urls_helper_spec.rb b/source/spec/helpers/urls_helper_spec.rb new file mode 100644 index 0000000..202c442 --- /dev/null +++ b/source/spec/helpers/urls_helper_spec.rb @@ -0,0 +1,18 @@ +require_relative '../../app/helpers/urls_helper' + +describe 'UrlsHelper' do + let(:target) { 'http://covermymeds.com/' } + let(:linkid) { UrlsHelper.shortened_linkid(target) } + + it 'returns a proper length linkid' do + expect(linkid.length).to eq UrlsHelper::LINKID_LENGTH + end + + it 'linkid only contains legal characters' do + expect(linkid).to match(/^[-_a-zA-Z0-9]{8}$/) + end + + it 'linkid length can\'t change with changing schema' do + expect(UrlsHelper::LINKID_LENGTH).to eq 8 + end +end diff --git a/source/spec/models/url_spec.rb b/source/spec/models/url_spec.rb index 209ca4c..993c8fb 100644 --- a/source/spec/models/url_spec.rb +++ b/source/spec/models/url_spec.rb @@ -1,5 +1,81 @@ require 'rails_helper' +require_relative '../../app/models/url' +require_relative '../../app/helpers/urls_helper' -RSpec.describe Url, :type => :model do - pending "add some examples to (or delete) #{__FILE__}" +RSpec.describe Url, type: :model do + let(:target) { 'http://anomaly.org/wade/projects/index.html' } + let(:url) { Url.new(target_link: target) } + + it 'has a defined interface' do + expect(url).to respond_to :ensure_short_link + expect(url).to respond_to :target_link_not_empty + expect(url).to respond_to :target_link_must_be_http + expect(url).to respond_to :target_link_must_be_reachable + end +end + +describe '#ensure_short_link', type: :model do + let(:target) { 'http://anomaly.org/wade/projects/index.html' } + let(:linkid) { UrlsHelper.shortened_linkid(target) } + + context 'No linkid already set' do + let(:url) { Url.new(target_link: target) } + + it 'uses ensure_short_link to generate linkid' do + expect(url.linkid).to be_nil + url.ensure_short_link + expect(url.linkid).not_to be_empty + expect(url.linkid).to eq linkid + end + end + + context 'Linkid is empty' do + let(:url) { Url.new(target_link: target, linkid: '') } + + it 'uses ensure_short_link to generate linkid' do + expect(url.linkid).to be_empty + url.ensure_short_link + expect(url.linkid).not_to be_empty + expect(url.linkid).to eq linkid + end + end + + context 'Linkid is not reset' do + let(:url) { Url.new(target_link: target, linkid: 'deadbeef') } + + it 'uses ensure_short_link to generate linkid' do + url.ensure_short_link + expect(url.linkid).to eq 'deadbeef' + end + end +end + +describe 'target_link_validation', type: :model do + context 'not #valid?' do + it 'detects missing target' do + expect(Url.new(target_link: '')).not_to be_valid + end + it 'detects missing scheme' do + expect(Url.new(target_link: 'asdfghfdsafg')).not_to be_valid + end + it 'detects wrong scheme' do + expect(Url.new(target_link: 'ftp://example.com/')).not_to be_valid + end + it 'detects unroutable address' do + expect(Url.new(target_link: 'http://a.a.a.a.a.a.a.a.a/')).not_to be_valid + end + it 'detects unroutable address' do + missing = 'http://localhost/this_page_does_not_exist' + expect(Url.new(target_link: missing)).not_to be_valid + end + end + + context '#valid?' do + it 'recognizes http request' do + expect(Url.new(target_link: 'http://google.com/')).to be_valid + end + it 'recognizes https request' do + expect(Url.new(target_link: 'https://google.com/')).to be_valid + end + end end diff --git a/source/spec/rails_helper.rb b/source/spec/rails_helper.rb new file mode 100644 index 0000000..e6c0b68 --- /dev/null +++ b/source/spec/rails_helper.rb @@ -0,0 +1,50 @@ +# This file is copied to spec/ when you run 'rails generate rspec:install' +ENV["RAILS_ENV"] ||= 'test' +require 'spec_helper' +require File.expand_path("../../config/environment", __FILE__) +require 'rspec/rails' +# Add additional requires below this line. Rails is not loaded until this point! + +# Requires supporting ruby files with custom matchers and macros, etc, in +# spec/support/ and its subdirectories. Files matching `spec/**/*_spec.rb` are +# run as spec files by default. This means that files in spec/support that end +# in _spec.rb will both be required and run as specs, causing the specs to be +# run twice. It is recommended that you do not name files matching this glob to +# end with _spec.rb. You can configure this pattern with the --pattern +# option on the command line or in ~/.rspec, .rspec or `.rspec-local`. +# +# The following line is provided for convenience purposes. It has the downside +# of increasing the boot-up time by auto-requiring all files in the support +# directory. Alternatively, in the individual `*_spec.rb` files, manually +# require only the support files necessary. +# +# Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } + +# Checks for pending migrations before tests are run. +# If you are not using ActiveRecord, you can remove this line. +ActiveRecord::Migration.maintain_test_schema! + +RSpec.configure do |config| + # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures + config.fixture_path = "#{::Rails.root}/spec/fixtures" + + # If you're not using ActiveRecord, or you'd prefer not to run each of your + # examples within a transaction, remove the following line or assign false + # instead of true. + config.use_transactional_fixtures = true + + # RSpec Rails can automatically mix in different behaviours to your tests + # based on their file location, for example enabling you to call `get` and + # `post` in specs under `spec/controllers`. + # + # You can disable this behaviour by removing the line below, and instead + # explicitly tag your specs with their type, e.g.: + # + # RSpec.describe UsersController, :type => :controller do + # # ... + # end + # + # The different available types are documented in the features, such as in + # https://relishapp.com/rspec/rspec-rails/docs + config.infer_spec_type_from_file_location! +end diff --git a/source/spec/spec_helper.rb b/source/spec/spec_helper.rb new file mode 100644 index 0000000..275ba49 --- /dev/null +++ b/source/spec/spec_helper.rb @@ -0,0 +1,85 @@ +# This file was generated by the `rails generate rspec:install` command. Conventionally, all +# specs live under a `spec` directory, which RSpec adds to the `$LOAD_PATH`. +# The generated `.rspec` file contains `--require spec_helper` which will cause this +# file to always be loaded, without a need to explicitly require it in any files. +# +# Given that it is always loaded, you are encouraged to keep this file as +# light-weight as possible. Requiring heavyweight dependencies from this file +# will add to the boot time of your test suite on EVERY test run, even for an +# individual file that may not need all of that loaded. Instead, consider making +# a separate helper file that requires the additional dependencies and performs +# the additional setup, and require it from the spec files that actually need it. +# +# The `.rspec` file also contains a few flags that are not defaults but that +# users commonly want. +# +# See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration +RSpec.configure do |config| + # rspec-expectations config goes here. You can use an alternate + # assertion/expectation library such as wrong or the stdlib/minitest + # assertions if you prefer. + config.expect_with :rspec do |expectations| + # This option will default to `true` in RSpec 4. It makes the `description` + # and `failure_message` of custom matchers include text for helper methods + # defined using `chain`, e.g.: + # be_bigger_than(2).and_smaller_than(4).description + # # => "be bigger than 2 and smaller than 4" + # ...rather than: + # # => "be bigger than 2" + expectations.include_chain_clauses_in_custom_matcher_descriptions = true + end + + # rspec-mocks config goes here. You can use an alternate test double + # library (such as bogus or mocha) by changing the `mock_with` option here. + config.mock_with :rspec do |mocks| + # Prevents you from mocking or stubbing a method that does not exist on + # a real object. This is generally recommended, and will default to + # `true` in RSpec 4. + mocks.verify_partial_doubles = true + end + +# The settings below are suggested to provide a good initial experience +# with RSpec, but feel free to customize to your heart's content. +=begin + # These two settings work together to allow you to limit a spec run + # to individual examples or groups you care about by tagging them with + # `:focus` metadata. When nothing is tagged with `:focus`, all examples + # get run. + config.filter_run :focus + config.run_all_when_everything_filtered = true + + # Limits the available syntax to the non-monkey patched syntax that is recommended. + # For more details, see: + # - http://myronmars.to/n/dev-blog/2012/06/rspecs-new-expectation-syntax + # - http://teaisaweso.me/blog/2013/05/27/rspecs-new-message-expectation-syntax/ + # - http://myronmars.to/n/dev-blog/2014/05/notable-changes-in-rspec-3#new__config_option_to_disable_rspeccore_monkey_patching + config.disable_monkey_patching! + + # Many RSpec users commonly either run the entire suite or an individual + # file, and it's useful to allow more verbose output when running an + # individual spec file. + if config.files_to_run.one? + # Use the documentation formatter for detailed output, + # unless a formatter has already been configured + # (e.g. via a command-line flag). + config.default_formatter = 'doc' + end + + # Print the 10 slowest examples and example groups at the + # end of the spec run, to help surface which specs are running + # particularly slow. + config.profile_examples = 10 + + # Run specs in random order to surface order dependencies. If you find an + # order dependency and want to debug it, you can fix the order by providing + # the seed, which is printed after each run. + # --seed 1234 + config.order = :random + + # Seed global randomization in this process using the `--seed` CLI option. + # Setting this allows you to use `--seed` to deterministically reproduce + # test failures related to randomization by passing the same `--seed` value + # as the one that triggered the failure. + Kernel.srand config.seed +=end +end From 3a60c129bbc29f9949118d387447073eaf0395b0 Mon Sep 17 00:00:00 2001 From: "G. Wade Johnson" Date: Tue, 13 Oct 2015 15:08:01 -0500 Subject: [PATCH 5/8] Fix Rubocop complaints --- source/app/controllers/urls_controller.rb | 20 +++++----- source/app/models/url.rb | 45 +++++++++++------------ 2 files changed, 31 insertions(+), 34 deletions(-) diff --git a/source/app/controllers/urls_controller.rb b/source/app/controllers/urls_controller.rb index dcf391a..1a56cf5 100644 --- a/source/app/controllers/urls_controller.rb +++ b/source/app/controllers/urls_controller.rb @@ -10,6 +10,12 @@ def new @save_errors = nil end + def _extract_url_errors(url) + index = [:target_link, :linkid, :base].find { |i| url.errors[i].any? } + return nil if index.nil? + url.errors[index] + end + def create # Actually create the new shortened url @target = params[:target] @@ -17,16 +23,8 @@ def create @url = Url.new(target_link: @target) if !@url.save - case - when @url.errors[:target_link].any? - @errors = @url.errors[:target_link] - when @url.errors[:linkid].any? - @errors = @url.errors[:linkid] - when @url.errors[:base].any? - @errors = @url.errors[:base] - else - @errors = ['Unable to save URL. Please try another.']; - end + @errors = _extract_url_errors(@url) || + ['Unable to save URL. Please try another.'] render 'new' else redirect_to url_path(@url.linkid) @@ -40,7 +38,7 @@ def show # redirect to the long url def follow url = Url.find_by(linkid: params[:linkid]) - Url.increment_counter( :click_count, url.id ) + Url.increment_counter(:click_count, url.id) redirect_to url.target_link end end diff --git a/source/app/models/url.rb b/source/app/models/url.rb index 276a137..80a368d 100644 --- a/source/app/models/url.rb +++ b/source/app/models/url.rb @@ -2,43 +2,42 @@ require 'net/http' class Url < ActiveRecord::Base - before_save :ensure_short_link + before_save :ensure_short_link validates :linkid, uniqueness: true - validate :target_link_not_empty, :target_link_must_be_http, :target_link_must_be_reachable + validate :target_link_not_empty, :target_link_must_be_http, + :target_link_must_be_reachable def target_link_not_empty return unless errors.empty? - if target_link.empty? - errors[:target_link] << 'Target URL cannot be blank'; - end + return unless target_link.empty? + errors[:target_link] << 'Target URL cannot be blank' end def target_link_must_be_http return unless errors.empty? - unless target_link.match(%r{^https?://}) - errors[:target_link] << 'Target URL has an unrecognized scheme'; - end + return if target_link.match(%r{^https?://}) + errors[:target_link] << 'Target URL has an unrecognized scheme' + end + + def _get_target_response_code + uri = URI.parse(target_link) + resp = Net::HTTP.get_response(uri) + resp.code.to_i end def target_link_must_be_reachable return unless errors.empty? - begin - uri = URI.parse(target_link) - resp = Net::HTTP.get_response(uri) - if resp.code.to_i >= 400 - errors[:target_link] << 'Target URL is not found' - end - rescue URI::InvalidURIError - errors[:target_link] << 'Target URL is not valid' - return - rescue - errors[:target_link] << 'Target URL is not reachable' - end + return if _get_target_response_code < 400 + errors[:target_link] << 'Target URL is not found' + rescue URI::InvalidURIError + errors[:target_link] << 'Target URL is not valid' + return + rescue + errors[:target_link] << 'Target URL is not reachable' end def ensure_short_link - if self.linkid.nil? || self.linkid.empty? - self.linkid = UrlsHelper.shortened_linkid(self.target_link) - end + return unless linkid.nil? || linkid.empty? + self.linkid = UrlsHelper.shortened_linkid(target_link) end end From 1c79e74f727ede8cbf343785cc553ea53faf0ef5 Mon Sep 17 00:00:00 2001 From: "G. Wade Johnson" Date: Tue, 13 Oct 2015 19:09:34 -0500 Subject: [PATCH 6/8] Used scaffold to get spec tests for controller. --- source/app/controllers/urls_controller.rb | 6 +- source/app/models/url.rb | 2 +- source/app/views/urls/index.html.erb | 2 +- source/app/views/urls/new.html.erb | 4 +- .../spec/controllers/urls_controller_spec.rb | 105 ++++++++++++++++++ source/spec/requests/urls_spec.rb | 10 ++ source/spec/routing/urls_routing_spec.rb | 22 ++++ source/spec/views/urls/index.html.erb_spec.rb | 14 +++ source/spec/views/urls/new.html.erb_spec.rb | 14 +++ source/spec/views/urls/show.html.erb_spec.rb | 11 ++ 10 files changed, 183 insertions(+), 7 deletions(-) create mode 100644 source/spec/controllers/urls_controller_spec.rb create mode 100644 source/spec/requests/urls_spec.rb create mode 100644 source/spec/routing/urls_routing_spec.rb create mode 100644 source/spec/views/urls/index.html.erb_spec.rb create mode 100644 source/spec/views/urls/new.html.erb_spec.rb create mode 100644 source/spec/views/urls/show.html.erb_spec.rb diff --git a/source/app/controllers/urls_controller.rb b/source/app/controllers/urls_controller.rb index 1a56cf5..82ecdd1 100644 --- a/source/app/controllers/urls_controller.rb +++ b/source/app/controllers/urls_controller.rb @@ -18,7 +18,7 @@ def _extract_url_errors(url) def create # Actually create the new shortened url - @target = params[:target] + @target = params[:target_link] @errors = nil @url = Url.new(target_link: @target) @@ -27,12 +27,12 @@ def create ['Unable to save URL. Please try another.'] render 'new' else - redirect_to url_path(@url.linkid) + redirect_to @url end end def show - @url = Url.find_by(linkid: params[:id]) + @url = Url.find(params[:id]) end # redirect to the long url diff --git a/source/app/models/url.rb b/source/app/models/url.rb index 80a368d..c1c8d6e 100644 --- a/source/app/models/url.rb +++ b/source/app/models/url.rb @@ -9,7 +9,7 @@ class Url < ActiveRecord::Base def target_link_not_empty return unless errors.empty? - return unless target_link.empty? + return unless target_link.nil? || target_link.empty? errors[:target_link] << 'Target URL cannot be blank' end diff --git a/source/app/views/urls/index.html.erb b/source/app/views/urls/index.html.erb index 7e5ea5b..4d922da 100644 --- a/source/app/views/urls/index.html.erb +++ b/source/app/views/urls/index.html.erb @@ -7,7 +7,7 @@ Shortened IDTarget <% @urls.each do |url| %> - <%= url.linkid %> + <%= url.linkid %> <%= url.target_link %> <% end %> diff --git a/source/app/views/urls/new.html.erb b/source/app/views/urls/new.html.erb index b22d9cc..d18364f 100644 --- a/source/app/views/urls/new.html.erb +++ b/source/app/views/urls/new.html.erb @@ -6,8 +6,8 @@ <% end %>
- - + +
Back to list diff --git a/source/spec/controllers/urls_controller_spec.rb b/source/spec/controllers/urls_controller_spec.rb new file mode 100644 index 0000000..0eae928 --- /dev/null +++ b/source/spec/controllers/urls_controller_spec.rb @@ -0,0 +1,105 @@ +require 'rails_helper' + +# This spec was generated by rspec-rails when you ran the scaffold generator. +# It demonstrates how one might use RSpec to specify the controller code that +# was generated by Rails when you ran the scaffold generator. +# +# It assumes that the implementation code is generated by the rails scaffold +# generator. If you are using any extension libraries to generate different +# controller code, this generated spec may or may not pass. +# +# It only uses APIs available in rails and/or rspec-rails. There are a number +# of tools you can use to make these specs even more expressive, but we're +# sticking to rails and rspec-rails APIs to keep things simple and stable. +# +# Compared to earlier versions of this generator, there is very limited use of +# stubs and message expectations in this spec. Stubs are only used when there +# is no simpler way to get a handle on the object needed for the example. +# Message expectations are only used when there is no simpler way to specify +# that an instance is receiving a specific message. + +RSpec.describe UrlsController, :type => :controller do + + # This should return the minimal set of attributes required to create a valid + # Url. As you add validations to Url, be sure to + # adjust the attributes here as well. + let(:valid_attributes) { + { target_link: 'http://google.com/' } + } + + let(:invalid_attributes) { + { target_link: 'wqaesrdtfyjguhj' } + } + + # This should return the minimal set of values that should be in the session + # in order to pass any filters (e.g. authentication) defined in + # UrlsController. Be sure to keep this updated too. + let(:valid_session) { {} } + + describe 'GET index' do + it 'assigns all urls as @urls' do + url = Url.create! valid_attributes + get :index, {}, valid_session + expect(assigns(:urls)).to eq([url]) + end + end + + describe 'GET show' do + it 'assigns the requested url as @url' do + url = Url.create! valid_attributes + get :show, {:id => url.id}, valid_session + expect(assigns(:url)).to eq(url) + end + end + + describe 'GET new' do + subject { get :new, {}, valid_session } + + it 'renders the new Url form' do + expect(subject.status).to eq 200 + expect(subject).to render_template :new + end + end + + describe 'POST create' do + describe 'with valid params' do + it 'creates a new Url' do + expect { + post :create, valid_attributes, valid_session + }.to change(Url, :count).by(1) + # TODO: Need to figure out correct incantation for this link + expect(response).to redirect_to(Url.last) + end + end + + describe 'with invalid params' do + subject { post :create, invalid_attributes, valid_session } + it 're-renders the new template' do + expect(subject).to render_template :new + end + end + + describe 'with missing target' do + subject { post :create, { target_link: '' }, valid_session } + it 're-renders the new template' do + expect(subject).to render_template :new + end + end + + describe 'with unreachable link' do + subject { post :create, { target_link: 'http://a.a.a.a.a.a.a/' }, valid_session } + it 're-renders the new template' do + expect(subject).to render_template :new + end + end + end + + describe 'GET follow' do + let(:url) { Url.create! valid_attributes } + + it 'redirects to target' do + get :follow, { linkid: url.linkid }, valid_session + expect(response).to redirect_to url.target_link + end + end +end diff --git a/source/spec/requests/urls_spec.rb b/source/spec/requests/urls_spec.rb new file mode 100644 index 0000000..cd1976c --- /dev/null +++ b/source/spec/requests/urls_spec.rb @@ -0,0 +1,10 @@ +require 'rails_helper' + +RSpec.describe "Urls", :type => :request do + describe "GET /urls" do + it "works! (now write some real specs)" do + get urls_path + expect(response).to have_http_status(200) + end + end +end diff --git a/source/spec/routing/urls_routing_spec.rb b/source/spec/routing/urls_routing_spec.rb new file mode 100644 index 0000000..b8c16e0 --- /dev/null +++ b/source/spec/routing/urls_routing_spec.rb @@ -0,0 +1,22 @@ +require "rails_helper" + +RSpec.describe UrlsController, :type => :routing do + describe "routing" do + + it "routes to #index" do + expect(:get => "/urls").to route_to("urls#index") + end + + it "routes to #new" do + expect(:get => "/urls/new").to route_to("urls#new") + end + + it "routes to #show" do + expect(:get => "/urls/1").to route_to("urls#show", :id => "1") + end + + it "routes to #create" do + expect(:post => "/urls").to route_to("urls#create") + end + end +end diff --git a/source/spec/views/urls/index.html.erb_spec.rb b/source/spec/views/urls/index.html.erb_spec.rb new file mode 100644 index 0000000..e119f6f --- /dev/null +++ b/source/spec/views/urls/index.html.erb_spec.rb @@ -0,0 +1,14 @@ +require 'rails_helper' + +RSpec.describe "urls/index", :type => :view do + before(:each) do + assign(:urls, [ + Url.create!(target_link: 'http://google.com/'), + Url.create!(target_link: 'http://www.covermymeds.com/') + ]) + end + + it "renders a list of urls" do + render + end +end diff --git a/source/spec/views/urls/new.html.erb_spec.rb b/source/spec/views/urls/new.html.erb_spec.rb new file mode 100644 index 0000000..e614f23 --- /dev/null +++ b/source/spec/views/urls/new.html.erb_spec.rb @@ -0,0 +1,14 @@ +require 'rails_helper' + +RSpec.describe "urls/new", :type => :view do + before(:each) do + assign(:url, Url.new()) + end + + it "renders new url form" do + render + + assert_select "form[action=?][method=?]", urls_path, "post" do + end + end +end diff --git a/source/spec/views/urls/show.html.erb_spec.rb b/source/spec/views/urls/show.html.erb_spec.rb new file mode 100644 index 0000000..473998f --- /dev/null +++ b/source/spec/views/urls/show.html.erb_spec.rb @@ -0,0 +1,11 @@ +require 'rails_helper' + +RSpec.describe "urls/show", :type => :view do + before(:each) do + @url = assign(:url, Url.create!(target_link: 'http://google.com/')) + end + + it "renders attributes in

" do + render + end +end From 9f75c0aa6ed812e054c383e953938c7d92207383 Mon Sep 17 00:00:00 2001 From: "G. Wade Johnson" Date: Wed, 14 Oct 2015 10:37:42 -0500 Subject: [PATCH 7/8] Clean up rubocop violations --- .../spec/controllers/urls_controller_spec.rb | 32 ++++++++----------- source/spec/requests/urls_spec.rb | 6 ++-- source/spec/routing/urls_routing_spec.rb | 23 +++++++------ source/spec/views/urls/index.html.erb_spec.rb | 4 +-- source/spec/views/urls/new.html.erb_spec.rb | 8 ++--- source/spec/views/urls/show.html.erb_spec.rb | 4 +-- 6 files changed, 35 insertions(+), 42 deletions(-) diff --git a/source/spec/controllers/urls_controller_spec.rb b/source/spec/controllers/urls_controller_spec.rb index 0eae928..c82542b 100644 --- a/source/spec/controllers/urls_controller_spec.rb +++ b/source/spec/controllers/urls_controller_spec.rb @@ -18,18 +18,13 @@ # Message expectations are only used when there is no simpler way to specify # that an instance is receiving a specific message. -RSpec.describe UrlsController, :type => :controller do - +RSpec.describe UrlsController, type: :controller do # This should return the minimal set of attributes required to create a valid # Url. As you add validations to Url, be sure to # adjust the attributes here as well. - let(:valid_attributes) { - { target_link: 'http://google.com/' } - } + let(:valid_attributes) { { target_link: 'http://google.com/' } } - let(:invalid_attributes) { - { target_link: 'wqaesrdtfyjguhj' } - } + let(:invalid_attributes) { { target_link: 'wqaesrdtfyjguhj' } } # This should return the minimal set of values that should be in the session # in order to pass any filters (e.g. authentication) defined in @@ -47,7 +42,7 @@ describe 'GET show' do it 'assigns the requested url as @url' do url = Url.create! valid_attributes - get :show, {:id => url.id}, valid_session + get :show, { id: url.id }, valid_session expect(assigns(:url)).to eq(url) end end @@ -64,10 +59,8 @@ describe 'POST create' do describe 'with valid params' do it 'creates a new Url' do - expect { - post :create, valid_attributes, valid_session - }.to change(Url, :count).by(1) - # TODO: Need to figure out correct incantation for this link + expect { post :create, valid_attributes, valid_session } + .to change(Url, :count).by(1) expect(response).to redirect_to(Url.last) end end @@ -87,7 +80,8 @@ end describe 'with unreachable link' do - subject { post :create, { target_link: 'http://a.a.a.a.a.a.a/' }, valid_session } + bad_link = 'http://a.a.a.a.a.a.a/' + subject { post :create, { target_link: bad_link }, valid_session } it 're-renders the new template' do expect(subject).to render_template :new end @@ -95,11 +89,11 @@ end describe 'GET follow' do - let(:url) { Url.create! valid_attributes } + let(:url) { Url.create! valid_attributes } - it 'redirects to target' do - get :follow, { linkid: url.linkid }, valid_session - expect(response).to redirect_to url.target_link - end + it 'redirects to target' do + get :follow, { linkid: url.linkid }, valid_session + expect(response).to redirect_to url.target_link + end end end diff --git a/source/spec/requests/urls_spec.rb b/source/spec/requests/urls_spec.rb index cd1976c..757a6cb 100644 --- a/source/spec/requests/urls_spec.rb +++ b/source/spec/requests/urls_spec.rb @@ -1,8 +1,8 @@ require 'rails_helper' -RSpec.describe "Urls", :type => :request do - describe "GET /urls" do - it "works! (now write some real specs)" do +RSpec.describe 'Urls', type: :request do + describe 'GET /urls' do + it 'works! (now write some real specs)' do get urls_path expect(response).to have_http_status(200) end diff --git a/source/spec/routing/urls_routing_spec.rb b/source/spec/routing/urls_routing_spec.rb index b8c16e0..0ea4d16 100644 --- a/source/spec/routing/urls_routing_spec.rb +++ b/source/spec/routing/urls_routing_spec.rb @@ -1,22 +1,21 @@ -require "rails_helper" +require 'rails_helper' -RSpec.describe UrlsController, :type => :routing do - describe "routing" do - - it "routes to #index" do - expect(:get => "/urls").to route_to("urls#index") +RSpec.describe UrlsController, type: :routing do + describe 'routing' do + it 'routes to #index' do + expect(get: '/urls').to route_to('urls#index') end - it "routes to #new" do - expect(:get => "/urls/new").to route_to("urls#new") + it 'routes to #new' do + expect(get: '/urls/new').to route_to('urls#new') end - it "routes to #show" do - expect(:get => "/urls/1").to route_to("urls#show", :id => "1") + it 'routes to #show' do + expect(get: '/urls/1').to route_to('urls#show', id: '1') end - it "routes to #create" do - expect(:post => "/urls").to route_to("urls#create") + it 'routes to #create' do + expect(post: '/urls').to route_to('urls#create') end end end diff --git a/source/spec/views/urls/index.html.erb_spec.rb b/source/spec/views/urls/index.html.erb_spec.rb index e119f6f..4cb55d3 100644 --- a/source/spec/views/urls/index.html.erb_spec.rb +++ b/source/spec/views/urls/index.html.erb_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -RSpec.describe "urls/index", :type => :view do +RSpec.describe 'urls/index', type: :view do before(:each) do assign(:urls, [ Url.create!(target_link: 'http://google.com/'), @@ -8,7 +8,7 @@ ]) end - it "renders a list of urls" do + it 'renders a list of urls' do render end end diff --git a/source/spec/views/urls/new.html.erb_spec.rb b/source/spec/views/urls/new.html.erb_spec.rb index e614f23..63a3ab7 100644 --- a/source/spec/views/urls/new.html.erb_spec.rb +++ b/source/spec/views/urls/new.html.erb_spec.rb @@ -1,14 +1,14 @@ require 'rails_helper' -RSpec.describe "urls/new", :type => :view do +RSpec.describe 'urls/new', type: :view do before(:each) do - assign(:url, Url.new()) + assign(:url, Url.new) end - it "renders new url form" do + it 'renders new url form' do render - assert_select "form[action=?][method=?]", urls_path, "post" do + assert_select 'form[action=?][method=?]', urls_path, 'post' do end end end diff --git a/source/spec/views/urls/show.html.erb_spec.rb b/source/spec/views/urls/show.html.erb_spec.rb index 473998f..eb1263d 100644 --- a/source/spec/views/urls/show.html.erb_spec.rb +++ b/source/spec/views/urls/show.html.erb_spec.rb @@ -1,11 +1,11 @@ require 'rails_helper' -RSpec.describe "urls/show", :type => :view do +RSpec.describe 'urls/show', type: :view do before(:each) do @url = assign(:url, Url.create!(target_link: 'http://google.com/')) end - it "renders attributes in

" do + it 'renders attributes in

' do render end end From e7df1d0f90611acfd28f3f6abe40d2bd53f71622 Mon Sep 17 00:00:00 2001 From: "G. Wade Johnson" Date: Wed, 14 Oct 2015 16:25:15 -0500 Subject: [PATCH 8/8] Correct issues identified by Mike Gee. * if/else handling of error * Use of .blank? * Correct start of string anchor --- source/app/controllers/urls_controller.rb | 6 +++--- source/app/models/url.rb | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/source/app/controllers/urls_controller.rb b/source/app/controllers/urls_controller.rb index 82ecdd1..3d5629a 100644 --- a/source/app/controllers/urls_controller.rb +++ b/source/app/controllers/urls_controller.rb @@ -22,12 +22,12 @@ def create @errors = nil @url = Url.new(target_link: @target) - if !@url.save + if @url.save + redirect_to @url + else @errors = _extract_url_errors(@url) || ['Unable to save URL. Please try another.'] render 'new' - else - redirect_to @url end end diff --git a/source/app/models/url.rb b/source/app/models/url.rb index c1c8d6e..be860a5 100644 --- a/source/app/models/url.rb +++ b/source/app/models/url.rb @@ -9,13 +9,13 @@ class Url < ActiveRecord::Base def target_link_not_empty return unless errors.empty? - return unless target_link.nil? || target_link.empty? + return unless target_link.blank? errors[:target_link] << 'Target URL cannot be blank' end def target_link_must_be_http return unless errors.empty? - return if target_link.match(%r{^https?://}) + return if target_link.match(%r{\Ahttps?://}) errors[:target_link] << 'Target URL has an unrecognized scheme' end