Skip to content

Commit

Permalink
Add OpenAPI spec and validate requests against it
Browse files Browse the repository at this point in the history
Fixes #1216

Includes:
* Add OpenAPI specification (in YAML format)
* Use Committee gem to validate requests (but not yet responses)
* Validate the OpenAPI specification in CI
* Align ObjectsController documentation with implementation
* Adjust specs now that committee is doing some error handling
* Remove dead code paths

This PR could go farther by validating responses in addition to requests, and also by removing some error-checking/validation code paths in the controllers that may now effectively be dead by virtue of the work Committee is doing for us, but I opted to get this PR submitted instead due to its size and complexity.
  • Loading branch information
mjgiarlo committed Feb 26, 2020
1 parent 4cc7329 commit 50b2bab
Show file tree
Hide file tree
Showing 14 changed files with 699 additions and 52 deletions.
5 changes: 5 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,17 @@ rvm:
- 2.5.3
before_install:
- yes | gem update --system
- nvm install node
- npm install -g openapi-enforcer-cli
cache: bundler
services:
- postgresql
- redis-server
before_script:
- bundle exec rake db:reset
script:
- ./scripts/validate-openapi.bash
- bundle exec rake
bundler_args: "--without 'production development deploy'"
notifications:
email: false
Expand Down
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ end

# general Ruby/Rails gems
gem 'aws-sdk-s3', '~> 1.17'
gem 'committee' # Validates HTTP requests/responses per OpenAPI specification
gem 'config' # Settings to manage configs on different instances
gem 'dor-workflow-client', '~> 3.8' # audit errors are reported to the workflow service
gem 'honeybadger' # for error reporting / tracking / notifications
Expand Down
7 changes: 7 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ GEM
capistrano-shared_configs (0.2.2)
chronic (0.10.2)
coderay (1.1.2)
committee (3.3.0)
json_schema (~> 0.14, >= 0.14.3)
openapi_parser (>= 0.6.1)
rack (>= 1.5)
concurrent-ruby (1.1.6)
config (2.2.1)
deep_merge (~> 1.2, >= 1.2.1)
Expand Down Expand Up @@ -207,6 +211,7 @@ GEM
activesupport (>= 5.0.0)
jmespath (1.4.0)
json (2.3.0)
json_schema (0.20.8)
jwt (2.2.1)
listen (3.1.5)
rb-fsevent (~> 0.9, >= 0.9.4)
Expand Down Expand Up @@ -243,6 +248,7 @@ GEM
nokogiri-happymapper (0.8.1)
nokogiri (~> 1.5)
okcomputer (1.18.1)
openapi_parser (0.8.0)
parallel (1.19.1)
parser (2.7.0.2)
ast (~> 2.4.0)
Expand Down Expand Up @@ -395,6 +401,7 @@ DEPENDENCIES
capistrano-passenger
capistrano-rails
capistrano-resque-pool
committee
config
coveralls
dlss-capistrano
Expand Down
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
[![Build Status](https://travis-ci.org/sul-dlss/preservation_catalog.svg?branch=master)](https://travis-ci.org/sul-dlss/preservation_catalog)
[![Coverage Status](https://coveralls.io/repos/github/sul-dlss/preservation_catalog/badge.svg?branch=master)](https://coveralls.io/github/sul-dlss/preservation_catalog?branch=master)
[![GitHub version](https://badge.fury.io/gh/sul-dlss%2Fpreservation_catalog.svg)](https://badge.fury.io/gh/sul-dlss%2Fpreservation_catalog)
[![Docker image](https://images.microbadger.com/badges/image/suldlss/preservation_catalog.svg)](https://microbadger.com/images/suldlss/preservation_catalog "Get your own image badge on microbadger.com")
[![OpenAPI Validator](http://validator.swagger.io/validator?url=https://raw.githubusercontent.com/sul-dlss/preservation_catalog/master/openapi.yml)](http://validator.swagger.io/validator/debug?url=https://raw.githubusercontent.com/sul-dlss/preservation_catalog/master/openapi.yml)

# README

Expand Down Expand Up @@ -359,6 +361,16 @@ curl -H 'Authorization: Bearer eyJhbGcxxxxx.eyJzdWIxxxxx.lWMJ66Wxx-xx' http://lo
}
```

Build image:
```
docker build -t suldlss/preservation_catalog:latest .
```

Publish:
```
docker push suldlss/preservation_catalog:latest
```

## Deploying

Capistrano is used to deploy. You will need SSH access to the targeted servers, via `kinit` and VPN.
Expand Down
6 changes: 0 additions & 6 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,6 @@ def strip_druid(id)
id&.split(':', 2)&.last
end

def bad_request(exception)
msg = '400 bad request'
msg = "#{msg}: #{exception.message}" if exception
render plain: msg, status: :bad_request
end

def not_found(exception)
msg = '404 Not Found'
msg = "#{msg}: #{exception.message}" if exception
Expand Down
20 changes: 3 additions & 17 deletions app/controllers/objects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
# (Note: methods will eventually be ported from sdr-services-app)
class ObjectsController < ApplicationController
# return the PreservedObject model for the druid (supplied with druid: prefix)
# GET /v1/objects/:druid
# GET /v1/objects/:id.json
def show
render json: PreservedObject.find_by!(druid: druid).to_json
end

# return a specific file from the Moab
# GET /v1/objects/:druid/file?category=manifest&filepath=signatureCatalog.xml
# GET /v1/objects/:id/file?category=manifest&filepath=signatureCatalog.xml
# useful params:
# - category (content|manifest|metadata)
# - filepath path of file, relative to category directory
Expand All @@ -31,14 +31,12 @@ def file
else
render(plain: "404 File Not Found: #{druid}, #{params[:category]}, #{params[:filepath]}, #{params[:version]}", status: :not_found)
end
rescue ArgumentError => e
render(plain: "400 Bad Request: #{e}", status: :bad_request)
rescue Moab::MoabRuntimeError => e
render(plain: "404 Not Found: #{e}", status: :not_found)
end

# return the checksums and filesize for a single druid (supplied with druid: prefix)
# GET /v1/objects/:druid/checksum
# GET /v1/objects/:id/checksum
def checksum
render json: content_files_checksums(druid).to_json
end
Expand All @@ -47,11 +45,6 @@ def checksum
# note: this is deliberately allowed to be a POST to allow for a large number of druids to be passed in
# GET OR POST /v1/objects/checksums?druids[]=druid1&druids[]=druid2&druids[]=druid3
def checksums
unless normalized_druids.present?
render(plain: "400 Bad Request - druids param must be populated with valid druids", status: :bad_request)
return
end

checksum_list, missing_druids, errored_druids = generate_checksum_list

bad_recs_msg = "\nStorage object(s) not found for #{missing_druids.join(', ')}" if missing_druids.any?
Expand All @@ -70,8 +63,6 @@ def checksums
end
format.any { render status: :not_acceptable, plain: 'Format not acceptable' }
end
rescue Moab::InvalidSuriSyntaxError => e
render(plain: "400 Bad Request: #{e}", status: :bad_request)
end

# Retrieves [Moab::FileInventoryDifference] from comparison of passed contentMetadata.xml
Expand All @@ -90,8 +81,6 @@ def content_diff
obj_version = params[:version].to_i if params[:version]&.match?(/^[1-9]\d*$/)
subset = params[:subset] ||= 'all'
render(xml: MoabStorageService.content_diff(druid, params[:content_metadata], subset, obj_version).to_xml)
rescue ArgumentError => e
render(plain: "400 Bad Request: #{e}", status: :bad_request)
rescue Moab::MoabRuntimeError => e
render(plain: "500 Unable to get content diff: #{e}", status: :internal_server_error)
Honeybadger.notify(e)
Expand Down Expand Up @@ -124,9 +113,6 @@ def generate_checksum_list
checksum_list << { returned_druid(druid) => content_files_checksums(druid) }
rescue Moab::ObjectNotFoundException
missing_druids << druid
rescue Moab::InvalidSuriSyntaxError => e
# this needs to raise 400 error
raise e
rescue StandardError => e
errored_druids << "#{druid} (#{e.inspect})"
end
Expand Down
29 changes: 29 additions & 0 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,40 @@
# you've limited to :test, :development, or :production.
Bundler.require(*Rails.groups)

class JSONAPIError < Committee::ValidationError
def error_body
{
errors: [
{ status: id, detail: message }
]
}
end

def render
[
status,
{ 'Content-Type' => 'application/vnd.api+json' },
[JSON.generate(error_body)]
]
end
end

module PreservationCatalog
class Application < Rails::Application
# Initialize configuration defaults for originally generated Rails version.
config.load_defaults 5.1

# accept_request_filter omits OKComputer
accept_proc = proc { |request| request.path.start_with?('/v1') }
config.middleware.use Committee::Middleware::RequestValidation, schema_path: 'openapi.yml',
strict: true, error_class: JSONAPIError,
accept_request_filter: accept_proc
# TODO: we can uncomment this at a later date to ensure we are passing back
# valid responses. Currently, uncommenting this line causes 24 spec
# failures. See https://github.com/sul-dlss/preservation_catalog/issues/1407
#
# config.middleware.use Committee::Middleware::ResponseValidation, schema_path: 'openapi.yml'

# Settings in config/environments/* take precedence over those specified here.
# Application configuration should go into files in config/initializers
# -- all .rb files in that directory are automatically loaded.
Expand Down
17 changes: 17 additions & 0 deletions docs/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!DOCTYPE html>
<html>
<head>
<title>API documentation</title>
<meta name="viewport" content="width=device-width, initial-scale=1">
<style>
body {
margin: 0;
padding: 0;
}
</style>
</head>
<body>
<redoc spec-url='https://raw.githubusercontent.com/sul-dlss/preservation_catalog/master/openapi.yml'></redoc>
<script src="https://cdn.jsdelivr.net/npm/redoc@2.0.0-alpha.17/bundles/redoc.standalone.js"> </script>
</body>
</html>

0 comments on commit 50b2bab

Please sign in to comment.