From 6b52adc661896434d3fea1fd7f83c62bef2456a6 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 10 Nov 2016 21:16:54 +0100 Subject: [PATCH] Refine incremental pipeline serializer --- .../projects/pipelines_controller.rb | 5 +- app/serializers/pipeline_entity.rb | 53 ++++++++++++------- app/serializers/pipeline_serializer.rb | 5 ++ spec/serializers/pipeline_serializer_spec.rb | 36 +++++++++++++ 4 files changed, 78 insertions(+), 21 deletions(-) create mode 100644 spec/serializers/pipeline_serializer_spec.rb diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 0b3503c6848..62cef8d4a4d 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -17,8 +17,9 @@ class Projects::PipelinesController < Projects::ApplicationController format.html format.json do render json: { - pipelines: PipelineSerializer.new(project: @project). - represent(@pipelines, current_user: current_user, last_updated: @last_updated), + pipelines: PipelineSerializer + .new(project: @project, user: @current_user) + .incremental(@pipelines, @last_updated), updated_at: Time.now, count: { all: @pipelines_count, diff --git a/app/serializers/pipeline_entity.rb b/app/serializers/pipeline_entity.rb index 6191d63fd7f..234ec74c2cd 100644 --- a/app/serializers/pipeline_entity.rb +++ b/app/serializers/pipeline_entity.rb @@ -2,7 +2,8 @@ class PipelineEntity < Grape::Entity include RequestAwareEntity expose :id - expose :user, if: -> (pipeline, opts) { created?(pipeline, opts) }, using: UserEntity + expose :user, if: proc { created_exposure? }, using: UserEntity + expose :url do |pipeline| namespace_project_pipeline_path( pipeline.project.namespace, @@ -10,7 +11,7 @@ class PipelineEntity < Grape::Entity pipeline) end - expose :details, if: -> (pipeline, opts) { updated?(pipeline, opts) } do + expose :details, if: proc { updated_exposure? } do expose :status expose :duration expose :finished_at @@ -19,18 +20,20 @@ class PipelineEntity < Grape::Entity expose :manual_actions, using: PipelineActionEntity end - expose :flags, if: -> (pipeline, opts) { created?(pipeline, opts) } do + expose :flags, if: proc { created_exposure? } do expose :latest?, as: :latest expose :triggered?, as: :triggered + expose :yaml_errors?, as: :yaml_errors do |pipeline| pipeline.yaml_errors.present? end + expose :stuck?, as: :stuck do |pipeline| pipeline.builds.any?(&:stuck?) end end - expose :ref, if: -> (pipeline, opts) { created?(pipeline, opts) } do + expose :ref, if: proc { updated_exposure? } do expose :name do |pipeline| pipeline.ref end @@ -45,31 +48,43 @@ class PipelineEntity < Grape::Entity expose :tag? end - expose :commit, if: -> (pipeline, opts) { created?(pipeline, opts) }, using: CommitEntity + expose :commit, if: proc { created_exposure? }, using: CommitEntity - expose :retry_url, if: -> (pipeline, opts) { updated?(pipeline, opts) } do |pipeline| - can?(current_user, :update_pipeline, pipeline.project) && + expose :retry_url, if: proc { updated_exposure? } do |pipeline| + can?(request.user, :update_pipeline, pipeline.project) && pipeline.retryable? && - retry_namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id) + retry_namespace_project_pipeline_path(pipeline.project.namespace, + pipeline.project, pipeline.id) end - expose :cancel_url, if: -> (pipeline, opts) { updated?(pipeline, opts) } do |pipeline| - can?(current_user, :update_pipeline, pipeline.project) && + expose :cancel_url, if: proc { updated_exposure? } do |pipeline| + can?(request.user, :update_pipeline, pipeline.project) && pipeline.cancelable? && - cancel_namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id) + cancel_namespace_project_pipeline_path(pipeline.project.namespace, + pipeline.project, pipeline.id) end - private - - def last_updated(opts) - opts.fetch(:last_updated) + def created_exposure? + !incremental? || created? end - def created?(pipeline, opts) - !last_updated(opts) || pipeline.created_at > last_updated(opts) + def updated_exposure? + !incremental? || updated? end - def updated?(pipeline, opts) - !last_updated(opts) || pipeline.updated_at > last_updated(opts) + def incremental? + options[:incremental] + end + + def last_updated + options.fetch(:last_updated) + end + + def updated? + @object.updated_at > last_updated + end + + def created? + @object.created_at > last_updated end end diff --git a/app/serializers/pipeline_serializer.rb b/app/serializers/pipeline_serializer.rb index f7abbec7d45..8d3182d9766 100644 --- a/app/serializers/pipeline_serializer.rb +++ b/app/serializers/pipeline_serializer.rb @@ -1,3 +1,8 @@ class PipelineSerializer < BaseSerializer entity PipelineEntity + + def incremental(resource, last_updated) + represent(resource, incremental: true, + last_updated: last_updated) + end end diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb new file mode 100644 index 00000000000..c4eba2f2537 --- /dev/null +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe PipelineSerializer do + let(:serializer) do + described_class.new(user: user) + end + + let(:pipelines) do + create_list(:ci_pipeline, 2) + end + + let(:user) { create(:user) } + + context 'when using incremental serializer' do + let(:json) do + serializer.incremental(pipelines, time).as_json + end + + context 'when pipeline has been already updated' do + let(:time) { Time.now } + + it 'exposes only minimal information' do + expect(json.first.keys).to contain_exactly(:id, :url) + expect(json.second.keys).to contain_exactly(:id, :url) + end + end + + context 'when pipeline updated in the meantime' do + let(:time) { Time.now - 10.minutes } + + it 'exposes new data incrementally' do + expect(json.first.keys.count).to eq 9 + end + end + end +end