Skip to content
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

Better Request Parsing Parameters #331

Merged
merged 3 commits into from
Oct 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions spec/amber/amber_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ struct UserSocket < Amber::WebSockets::ClientSocket; end

struct RoomSocket < Amber::WebSockets::ClientSocket; end


describe Amber do
describe Amber do
describe ".env" do
it "should return test" do
Amber.env.test?.should be_truthy
Expand Down Expand Up @@ -54,9 +53,10 @@ describe Amber do
settings.secret_key_base.should eq "mV6kTmG3k1yVFh-fPYpugSn0wbZveDvrvfQuv88DPF8"
end

it "retains environment.yml settings that haven't been overwritten" do # NOTE: Any changes to settings here remain for all specs run afterwards.
# This is a problem.

it "retains environment.yml settings that haven't been overwritten" do
# NOTE: Any changes to settings here remain for all specs run afterwards.
# Added for convenience until settings is change to an instances
# instead of singleton this is still an "a problem".
Amber::Server.configure do |server|
server.name = "Hello World App"
server.port = 8080
Expand Down
6 changes: 4 additions & 2 deletions spec/amber/cli/commands/generator_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ module Amber::CLI
context "scaffold" do
it "generates and compile generated app" do
ENV["AMBER_ENV"] = "test"

MainCommand.run ["new", TESTING_APP]
Dir.cd(TESTING_APP)
MainCommand.run ["generate", "scaffold", "Animal", "name:string"]
Amber::CLI::Spec.prepare_yaml(Dir.current)

`shards build`

File.exists?("bin/#{TESTING_APP.split("/").last}").should be_true
File.exists?("bin/#{TEST_APP_NAME}").should be_true

Amber::CLI::Spec.cleanup
end
end
Expand Down Expand Up @@ -49,6 +50,7 @@ module Amber::CLI
File.read("./config/routes.cr").includes?(routes_get).should be_true
File.read("./config/routes.cr").includes?(routes_delete).should be_true
File.read("./src/controllers/animal_controller.cr").should eq output_class

Amber::CLI::Spec.cleanup
end
end
Expand Down
9 changes: 5 additions & 4 deletions spec/amber/router/context_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ require "../../../spec_helper"

describe HTTP::Server::Context do
describe "#override_request_method!" do
describe "when X-HTTP-Method-Overrid is present" do
context "when X-HTTP-Method-Override is present" do
it "overrides form POST method to PUT, PATCH, DELETE" do
%w(PUT PATCH DELETE).each do |method|
header = HTTP::Headers.new
Expand Down Expand Up @@ -147,23 +147,24 @@ describe HTTP::Server::Context do
it "parses json hash" do
headers = HTTP::Headers.new
headers["Content-Type"] = "application/json"
body = "{\"test\":\"test\"}"
body = %({ "test": "test", "address": { "city": "New York" }})
request = HTTP::Request.new("POST", "/", headers, body)

context = create_context(request)

context.params["test"].should eq "test"
context.params["address"].as(Hash)["city"].should eq "New York"
end

it "parses json array" do
headers = HTTP::Headers.new
headers["Content-Type"] = "application/json"
body = "[\"test\",\"test2\"]"
body = %(["test", "test2"])
request = HTTP::Request.new("POST", "/", headers, body)

context = create_context(request)

context.params["_json"].should eq "[\"test\", \"test2\"]"
context.params["_json"].should eq %w(test test2)
end

it "parses files from multipart forms" do
Expand Down
88 changes: 58 additions & 30 deletions spec/amber/validations/params_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module Amber::Validators
describe BaseRule do
describe "#apply" do
it "raises error when missing field" do
params = HTTP::Params.parse("")
params = params_builder("")
rule = BaseRule.new("field", "") do
false
end
Expand All @@ -15,7 +15,7 @@ module Amber::Validators
end

it "returns true for given block" do
params = HTTP::Params.parse("field=val")
params = params_builder("field=val")
rule = BaseRule.new("field", "") do
true
end
Expand All @@ -24,7 +24,7 @@ module Amber::Validators
end

it "returns false for the given block" do
params = HTTP::Params.parse("field=val")
params = params_builder("field=val")
rule = BaseRule.new("field", "") do
false
end
Expand All @@ -35,7 +35,7 @@ module Amber::Validators

describe "#error" do
it "returns default error message" do
params = HTTP::Params.parse("field=val")
params = params_builder("field=val")
error_message = "Field field is required"
rule = BaseRule.new("field", nil) do
false
Expand All @@ -47,7 +47,7 @@ module Amber::Validators
end

it "returns the given error message" do
params = HTTP::Params.parse("field=val")
params = params_builder("field=val")
error_message = "You must provide this field"
rule = BaseRule.new("field", error_message) do
false
Expand All @@ -63,17 +63,17 @@ module Amber::Validators
describe OptionalRule do
describe "#apply" do
it "does not apply rule when field is missing" do
params = HTTP::Params.parse("")
params = params_builder("")
error_message = "You must provide this field"
rule = OptionalRule.new("field", error_message) do
false
true
end

rule.apply(params).should be_true
end

it "applies rule when field is present" do
params = HTTP::Params.parse("field=val")
params = params_builder("field=val")
error_message = "You must provide this field"
rule = OptionalRule.new("field", error_message) do
true
Expand All @@ -87,12 +87,12 @@ module Amber::Validators
describe Params do
describe "#validation" do
it "validates required field" do
http_params = HTTP::Params.parse("name=elias&last_name=perez&middle=j")
http_params = params_builder("name=elias&last_name=perez&middle=j")
validator = Validators::Params.new(http_params)

result = validator.validation do
required(:name) { |v| v.str? & !v.empty? }
required("last_name") { |v| v.str? & !v.empty? }
required(:name) { |v| !v.nil? }
required("last_name") { |v| !v.nil? }
end

validator.valid?.should be_true
Expand All @@ -102,12 +102,12 @@ module Amber::Validators
context "optional params" do
context "when missing" do
it "does not validate optional field" do
http_params = HTTP::Params.parse("last_name=&middle=j")
http_params = params_builder("last_name=&middle=j")
validator = Validators::Params.new(http_params)

result = validator.validation do
optional(:name) { |v| v.empty? }
required("last_name") { |v| v.empty? }
optional(:name) { |v| !v.nil? }
required("last_name") { |v| !v.nil? }
end

validator.valid?.should be_true
Expand All @@ -117,40 +117,58 @@ module Amber::Validators

context "when present" do
it "validates optional field" do
http_params = HTTP::Params.parse("name=")
http_params = params_builder("name=")
validator = Validators::Params.new(http_params)

result = validator.validation do
optional(:name) { |v| !v.empty? }
optional(:name) { |v| v.nil? }
end

validator.valid?.should be_false
validator.errors.size.should eq 1
end
end

context "casting" do
it "returns false for the given block" do
params = params_builder("name=john&number=1&price=3.45&list=[1,2,3]")
validator = Validators::Params.new(params)
validator.validation do
required(:name) { |f| !f.to_s.empty? }
required(:number) { |f| f.as(String).to_i > 0 }
required(:price) { |f| f.as(String).to_f == 3.45 }
required(:list) do |f|
list = JSON.parse(f.as(String)).as_a
(list == [1, 2, 3] && !list.includes? 6)
end
end

validator.valid?.should be_truthy
end
end
end
end

describe "#valid?" do
it "returns false with invalid fields" do
http_params = HTTP::Params.parse("name=john&last_name=doe&middle=j")
http_params = params_builder("name=john&last_name=doe&middle=j")
validator = Validators::Params.new(http_params)

validator.validation do
required("name") { |v| v.empty? }
required("last_name") { |v| v.str? & !v.empty? }
required("name") { |v| v.nil? }
required("last_name") { |v| !v.nil? }
end

validator.valid?.should be_false
end

it "returns false when key does not exist" do
http_params = HTTP::Params.parse("name=elias")
http_params = params_builder("name=elias")
validator = Validators::Params.new(http_params)
result = {"nonexisting" => {nil, "Param [nonexisting] does not exist."}}

validator.validation do
required("nonexisting") { |v| v.str? & !v.empty? }
required("nonexisting") { |v| !v.nil? }
end

expect_raises Exceptions::Validator::InvalidParam do
Expand All @@ -159,12 +177,12 @@ module Amber::Validators
end

it "returns true with valid fields" do
http_params = HTTP::Params.parse("name=elias&last_name=perez&middle=j")
http_params = params_builder("name=elias&last_name=perez&middle=j")
validator = Validators::Params.new(http_params)

validator.validation do
required("name") { |v| v.str? & !v.empty? }
required("last_name") { |v| v.str? & !v.empty? }
required("name") { |v| !v.nil? }
required("last_name") { |v| !v.nil? }
end

validator.valid?.should be_true
Expand All @@ -173,12 +191,12 @@ module Amber::Validators

describe "#validate!" do
it "raises error on failed validation" do
http_params = HTTP::Params.parse("name=elias&last_name=perez&middle=j")
http_params = params_builder("name=&last_name=&middle=j")
validator = Validators::Params.new(http_params)

validator.validation do
required("name") { |v| v.str? & v.empty? }
required("last_name") { |v| v.str? & !v.empty? }
required("name") { |v| !v.to_s.empty? }
required("last_name") { |v| !v.to_s.empty? }
end

expect_raises Exceptions::Validator::ValidationFailed do
Expand All @@ -187,17 +205,27 @@ module Amber::Validators
end

it "returns validated params on successful validation" do
http_params = HTTP::Params.parse("name=elias&last_name=perez&middle=j")
http_params = params_builder("name=elias&last_name=perez&middle=j")
validator = Validators::Params.new(http_params)
result : Hash(String, String) = {"name" => "elias", "last_name" => "perez"}

validator.validation do
required("name") { |v| v.str? & !v.empty? }
required("last_name") { |v| v.str? & !v.empty? }
required("name") { |v| !v.nil? }
required("last_name") { |v| !v.nil? }
end

validator.validate!.should eq result
end
end
end
end

def params_builder(body = "")
params = {} of String | Symbol => Amber::Router::ParamsType
return params if body.empty?

body.tr("?", "").split("&").each_with_object(params) do |item, params|
key, value = item.split("=")
params[key] = value
end
end
13 changes: 7 additions & 6 deletions spec/spec_helper.cr
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
# NOTE: Constants should be set before require begins.

ENV["AMBER_ENV"] = "test"
TEST_PATH = "spec/support/sample"
PUBLIC_PATH = TEST_PATH + "/public"
VIEWS_PATH = TEST_PATH + "/views"
TESTING_APP = "./tmp/test_app"
APP_TPL_PATH = "./src/amber/cli/templates/app"
CURRENT_DIR = Dir.current
TEST_PATH = "spec/support/sample"
PUBLIC_PATH = TEST_PATH + "/public"
VIEWS_PATH = TEST_PATH + "/views"
TEST_APP_NAME = "test_app"
TESTING_APP = "./tmp/#{TEST_APP_NAME}"
APP_TPL_PATH = "./src/amber/cli/templates/app"
CURRENT_DIR = Dir.current

require "http"
require "spec"
Expand Down
2 changes: 1 addition & 1 deletion src/amber/cli/commands.cr
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ module Amber::CLI
help desc: "# Describe available commands and usages"
string ["-t", "--template"], desc: "# Preconfigure for selected template engine. Options: slang | ecr", default: "slang"
string ["-d", "--database"], desc: "# Preconfigure for selected database. Options: pg | mysql | sqlite", default: "pg"
string ["-m", "--model"], desc: "# Preconfigure for selected model. Options: granite | crecto", default: "granite"
string ["-m", "--model"], desc: "# Preconfigure for selected model. Options: granite | crecto", default: "granite"
end
end
end
12 changes: 6 additions & 6 deletions src/amber/cli/templates/auth.cr
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,19 @@ module Amber::CLI
@timestamp = Time.now.to_s("%Y%m%d%H%M%S")
@primary_key = primary_key
@visible_fields = @fields.reject(&.hidden).map(&.name)

add_routes :web, <<-ROUTES
get "/signin", SessionController, :new
post "/session", SessionController, :create
get "/signout", SessionController, :delete
get "/signup", RegistrationController, :new
post "/registration", RegistrationController, :create
post "/session", SessionController, :create
get "/signout", SessionController, :delete
get "/signup", RegistrationController, :new
post "/registration", RegistrationController, :create
ROUTES

add_plugs :web, <<-PLUGS
plug Authenticate.new
PLUGS

add_dependencies <<-DEPENDENCY
require "../src/models/**"
require "../src/handlers/**"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ class RegistrationController < ApplicationController
end

def create
<%= @name %> = <%= @name.capitalize %>.new(params.to_h.select(<%= @visible_fields %>))
<%= @name %>.password = params["password"]
<%= @name %> = <%= @name.capitalize %>.new(registration_params.validate!)
<%= @name %>.password = params["password"].to_s

if <%= @name %>.valid? && <%= @name %>.save
session[:<%= @name %>_id] = <%= @name %>.id
Expand All @@ -17,4 +17,11 @@ class RegistrationController < ApplicationController
render("new.<%= @language %>")
end
end

private def registration_params
params.validation do
required(:email) { |f| !f.nil? }
required(:password) { |f| !f.nil? }
end
end
end