diff --git a/Gemfile b/Gemfile index 19d6c60a738..bda9fc07d7c 100644 --- a/Gemfile +++ b/Gemfile @@ -200,6 +200,8 @@ end # API gems gem 'grape', '~> 0.10.1' +gem 'grape-cache_control', '~> 1.0.1' + gem 'roar', '~> 1.0.0' gem 'reform', '~> 1.2.6', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 62b8d4aa1da..1609f5407fc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -286,6 +286,8 @@ GEM rack-accept rack-mount virtus (>= 1.0.0) + grape-cache_control (1.0.1) + grape (~> 0.3) gravatar_image_tag (1.2.0) hashie (3.4.1) health_check (1.5.1) @@ -595,6 +597,7 @@ DEPENDENCIES globalize (~> 5.0.1) gon (~> 4.0) grape (~> 0.10.1) + grape-cache_control (~> 1.0.1) gravatar_image_tag (~> 1.2.0) health_check htmldiff diff --git a/lib/api/caching/helpers.rb b/lib/api/caching/helpers.rb new file mode 100644 index 00000000000..ac76fd10ba3 --- /dev/null +++ b/lib/api/caching/helpers.rb @@ -0,0 +1,26 @@ +module API + module Caching + module Helpers + def with_etag!(key) + etag = %(W/"#{::Digest::SHA1.hexdigest(key.to_s)}") + error!('Not Modified'.freeze, 304) if headers['If-None-Match'.freeze] == etag + + header 'ETag'.freeze, etag + end + + ## + # Store a represented object in its JSON representation + def cache(key, args = {}) + # Save serialization since we're only dealing with strings here + args[:raw] = true + + json = Rails.cache.fetch(key, args) { + result = yield + result.to_json + } + + ::API::Caching::StoredRepresenter.new json + end + end + end +end diff --git a/lib/api/caching/stored_representer.rb b/lib/api/caching/stored_representer.rb new file mode 100644 index 00000000000..99e24622946 --- /dev/null +++ b/lib/api/caching/stored_representer.rb @@ -0,0 +1,13 @@ +module API + module Caching + class StoredRepresenter + def initialize(json) + @json = json + end + + def to_json + @json + end + end + end +end diff --git a/lib/api/root.rb b/lib/api/root.rb index 31e8a315c90..7ea8a2a26cb 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -66,6 +66,7 @@ module API use OpenProject::Authentication::Manager + helpers API::Caching::Helpers helpers do def current_user User.current diff --git a/lib/api/v3/work_packages/schema/work_package_schema_representer.rb b/lib/api/v3/work_packages/schema/work_package_schema_representer.rb index e0ddd80a569..f5df50b4852 100644 --- a/lib/api/v3/work_packages/schema/work_package_schema_representer.rb +++ b/lib/api/v3/work_packages/schema/work_package_schema_representer.rb @@ -126,9 +126,6 @@ module API schema :spent_time, type: 'Duration', - show_if: -> (_) do - current_user_allowed_to(:view_time_entries, context: represented.project) - end, required: false schema :percentage_done, diff --git a/lib/api/v3/work_packages/schema/work_package_schemas_api.rb b/lib/api/v3/work_packages/schema/work_package_schemas_api.rb index 2669e6d1c72..403e38ebeb8 100644 --- a/lib/api/v3/work_packages/schema/work_package_schemas_api.rb +++ b/lib/api/v3/work_packages/schema/work_package_schemas_api.rb @@ -46,6 +46,10 @@ module API def raise404 raise ::API::Errors::NotFound.new end + + def cache_key(project_id, type_id) + "api/v3/work_packages/schema/#{project_id}-#{type_id}" + end end # The schema identifier is an artificial identifier that is composed of a work packages @@ -56,25 +60,30 @@ module API namespace ':project-:type' do before do begin - project = Project.find(params[:project]) - type = Type.find(params[:type]) + @project = Project.find(params[:project]) + @type = Type.find(params[:type]) rescue ActiveRecord::RecordNotFound raise404 end - authorize(:view_work_packages, context: project) do + authorize(:view_work_packages, context: @project) do raise404 end - schema = TypedWorkPackageSchema.new(project: project, type: type) - self_link = api_v3_paths.work_package_schema(project.id, type.id) - @representer = WorkPackageSchemaRepresenter.create(schema, - self_link: self_link, - current_user: current_user) + # Compare with ETag composed of project and customizations + # to avoid evaluating the server request + @custom_fields = @project.all_work_package_custom_fields + with_etag! "#{@project.id}/#{@custom_fields.count}/#{@custom_fields.to_param}" end get do - @representer + cache(key: [cache_key(@project.id, @type.id), @custom_fields]) do + schema = TypedWorkPackageSchema.new(project: @project, type: @type) + self_link = api_v3_paths.work_package_schema(@project.id, @type.id) + WorkPackageSchemaRepresenter.create(schema, + self_link: self_link, + current_user: nil) + end end end diff --git a/spec/lib/api/v3/work_packages/schema/work_package_schema_representer_spec.rb b/spec/lib/api/v3/work_packages/schema/work_package_schema_representer_spec.rb index 295d7f13e5c..867f480689e 100644 --- a/spec/lib/api/v3/work_packages/schema/work_package_schema_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/schema/work_package_schema_representer_spec.rb @@ -280,18 +280,6 @@ describe ::API::V3::WorkPackages::Schema::WorkPackageSchemaRepresenter do let(:required) { false } let(:writable) { false } end - - context 'not allowed to view time entries' do - before do - allow(current_user).to receive(:allowed_to?).with(:view_time_entries, - work_package.project) - .and_return false - end - - it 'does not show spentTime' do - is_expected.not_to have_json_path('spentTime') - end - end end describe 'percentageDone' do diff --git a/spec/requests/api/v3/work_packages/work_packages_schemas_resource_spec.rb b/spec/requests/api/v3/work_packages/work_packages_schemas_resource_spec.rb index 524285322c2..557889ecd4c 100644 --- a/spec/requests/api/v3/work_packages/work_packages_schemas_resource_spec.rb +++ b/spec/requests/api/v3/work_packages/work_packages_schemas_resource_spec.rb @@ -50,6 +50,10 @@ describe API::V3::WorkPackages::Schema::WorkPackageSchemasAPI, type: :request do it 'should return HTTP 200' do expect(last_response.status).to eql(200) end + + it 'should set a weak ETag' do + expect(last_response.headers['ETag']).to match(/W\/\"\w+\"/) + end end context 'id is too long' do