-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wade branch #16
base: master
Are you sure you want to change the base?
Wade branch #16
Changes from 6 commits
41f87de
a7c7a5a
91c758e
637b3dc
3a60c12
1c79e74
9f75c0a
e7df1d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,44 @@ | ||
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 | ||
@target = '' | ||
@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_link] | ||
@errors = nil | ||
|
||
@url = Url.new(target_link: @target) | ||
if [email protected] | ||
@errors = _extract_url_errors(@url) || | ||
['Unable to save URL. Please try another.'] | ||
render 'new' | ||
else | ||
redirect_to @url | ||
end | ||
end | ||
|
||
def show | ||
@url = Url.find(params[:id]) | ||
end | ||
|
||
# redirect to the long url | ||
def follow | ||
url = Url.find_by(linkid: params[:linkid]) | ||
Url.increment_counter(:click_count, url.id) | ||
redirect_to url.target_link | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +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? | ||
return unless target_link.nil? || target_link.empty? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ActiveSupport adds |
||
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?://}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use target_link = "This is not a valid link.\nhttps://google.com"
fail 'Tricked you!' if target_link.match(%r{^https?://})
#=> RuntimeError: Tricked you! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've been trying to break that habit for over a decade in Perl. |
||
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? | ||
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 | ||
return unless linkid.nil? || linkid.empty? | ||
self.linkid = UrlsHelper.shortened_linkid(target_link) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
<h1>URL Shortcuts</h1> | ||
|
||
<a href="<%= new_url_path %>">Get new shortened URL</a> | ||
<% if [email protected]? && @urls.size != 0 %> | ||
<table> | ||
<thead> | ||
<tr><th>Shortened ID</th><th>Target</th></tr> | ||
</thead> | ||
<% @urls.each do |url| %> | ||
<tr><td><a href="<%= url_url(url.id) %>"><%= url.linkid %></a></td> | ||
<td><a href="<%= url.target_link %>"><%= url.target_link %></a></td></tr> | ||
<% end %> | ||
</table> | ||
<% else %> | ||
<p>No shortened URLs found</p> | ||
<% end %> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
<h1>URL Shortener</h1> | ||
<% if [email protected]? && @errors.size > 0 %> | ||
<% @errors.each do |err| %> | ||
<p class="error"><%= err %></p> | ||
<% end %> | ||
<% end %> | ||
<form id="new_url" method="post" action="<%= urls_path %>"> | ||
<input name="authenticity_token" type="hidden" value="<%= form_authenticity_token %>"/> | ||
<label for="#target_link">Target:</label> | ||
<input type="text" name="target_link" id="target_link" size="100" placeholder="http://example.com/cool_page.html" value="<%= @target || '' %>"> | ||
<button class="btn btn-primary" type="submit">Shorten</button> | ||
</form> | ||
<a href="<%= urls_path %>">Back to list</a> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
<h1>URL Shortcut</h1> | ||
|
||
<a href="<%= urls_path %>">Back to list</a> | ||
|
||
<% if @url.nil? %> | ||
<p>Unrecognized Shortcut.</p> | ||
<% else %> | ||
<table> | ||
<tr> | ||
<th>Shortened Link</th> | ||
<td><a href="<%= follow_path(@url.linkid) %>"><%= follow_url(@url.linkid) %></a></td> | ||
</tr> | ||
<tr> | ||
<th>Target Link</th> | ||
<td><a href="<%= @url.target_link %>"><%= @url.target_link %></a></td> | ||
</tr> | ||
<tr> | ||
<th>Click Count</th> | ||
<td><%= @url.click_count %></td> | ||
</tr> | ||
</table> | ||
<% end %> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
class AddClickCountToUrl < ActiveRecord::Migration | ||
def change | ||
add_column :urls, :click_count, :integer, :default => 0 | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
# 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: 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 | ||
|
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the happy path as the positive part of the
if
and the error path underelse
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough.