From fb5c2147a9f2b3acc6ad5297c737da0f5546c247 Mon Sep 17 00:00:00 2001 From: Artem Sidorenko Date: Sat, 27 Feb 2016 09:28:00 +0100 Subject: [PATCH 1/9] Do not serve anything via nginx as we have workhorse Otherwise this might 'hide' problems https://github.com/gitlabhq/gitlabhq/issues/10053#issuecomment-188919319 --- lib/support/nginx/gitlab | 11 ++++++++++- lib/support/nginx/gitlab-ssl | 10 +++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/support/nginx/gitlab b/lib/support/nginx/gitlab index fc5475c4ee..1324e4cd26 100644 --- a/lib/support/nginx/gitlab +++ b/lib/support/nginx/gitlab @@ -30,7 +30,6 @@ server { listen [::]:80 default_server; server_name YOUR_SERVER_FQDN; ## Replace this with something like gitlab.example.com server_tokens off; ## Don't show the nginx version number, a security best practice - root /home/git/gitlab/public; ## See app/controllers/application_controller.rb for headers set @@ -57,4 +56,14 @@ server { proxy_pass http://gitlab-workhorse; } + + error_page 404 /404.html; + error_page 422 /422.html; + error_page 500 /500.html; + error_page 502 /502.html; + location ~ ^/(404|422|500|502)\.html$ { + root /home/git/gitlab/public; + internal; + } + } diff --git a/lib/support/nginx/gitlab-ssl b/lib/support/nginx/gitlab-ssl index 1e5f85413e..af6ea9ed70 100644 --- a/lib/support/nginx/gitlab-ssl +++ b/lib/support/nginx/gitlab-ssl @@ -45,7 +45,6 @@ server { listen [::]:443 ipv6only=on ssl default_server; server_name YOUR_SERVER_FQDN; ## Replace this with something like gitlab.example.com server_tokens off; ## Don't show the nginx version number, a security best practice - root /home/git/gitlab/public; ## Strong SSL Security ## https://raymii.org/s/tutorials/Strong_SSL_Security_On_nginx.html & https://cipherli.st/ @@ -101,4 +100,13 @@ server { proxy_set_header X-Forwarded-Proto $scheme; proxy_pass http://gitlab-workhorse; } + + error_page 404 /404.html; + error_page 422 /422.html; + error_page 500 /500.html; + error_page 502 /502.html; + location ~ ^/(404|422|500|502)\.html$ { + root /home/git/gitlab/public; + internal; + } } From 8fa2469bebf814a821da7883addc7c6c37e4a704 Mon Sep 17 00:00:00 2001 From: Artem Sidorenko Date: Sat, 27 Feb 2016 11:59:29 +0100 Subject: [PATCH 2/9] Lets have CSS and logo inline This would work independenty from location and configuration https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/662#note_3973448 --- CHANGELOG | 1 + public/404.html | 44 ++++++++++++++++++++++++++++++++++++++++++-- public/422.html | 45 ++++++++++++++++++++++++++++++++++++++++++--- public/500.html | 44 ++++++++++++++++++++++++++++++++++++++++++-- public/502.html | 44 ++++++++++++++++++++++++++++++++++++++++++-- public/deploy.html | 41 +++++++++++++++++++++++++++++++++++++++-- public/logo.svg | 9 --------- public/static.css | 36 ------------------------------------ 8 files changed, 208 insertions(+), 56 deletions(-) delete mode 100644 public/logo.svg delete mode 100644 public/static.css diff --git a/CHANGELOG b/CHANGELOG index c1c90903d5..2c67ee244f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -32,6 +32,7 @@ v 8.5.5 - Prevent a 500 error in Todos when author was removed - Fix pagination for filtered dashboard and explore pages - Fix "Show all" link behavior + - HTTP error pages work independently from location and config (Artem Sidorenko) v 8.5.4 - Do not cache requests for badges (including builds badge) diff --git a/public/404.html b/public/404.html index a0106bc760..4862770cc2 100644 --- a/public/404.html +++ b/public/404.html @@ -2,11 +2,51 @@ The page you're looking for could not be found (404) - + -

404

+

+
+ 404 +

The page you're looking for could not be found.

Make sure the address is correct and that the page hasn't moved.

diff --git a/public/422.html b/public/422.html index 026997b48e..055b0bde16 100644 --- a/public/422.html +++ b/public/422.html @@ -2,12 +2,51 @@ The change you requested was rejected (422) - + - -

422

+

+
+ 422 +

The change you requested was rejected.

Make sure you have access to the thing you tried to change.

diff --git a/public/500.html b/public/500.html index 08c11bbd05..3d59d1392f 100644 --- a/public/500.html +++ b/public/500.html @@ -2,10 +2,50 @@ Something went wrong (500) - + -

500

+

+
+ 500 +

Whoops, something went wrong on our end.

Try refreshing the page, or going back and attempting the action again.

diff --git a/public/502.html b/public/502.html index 9480a92843..67dfd8a274 100644 --- a/public/502.html +++ b/public/502.html @@ -2,10 +2,50 @@ GitLab is not responding (502) - + -

502

+

+
+ 502 +

Whoops, GitLab is taking too much time to respond.

Try refreshing the page, or going back and attempting the action again.

diff --git a/public/deploy.html b/public/deploy.html index 3822ed4b64..48976dacf4 100644 --- a/public/deploy.html +++ b/public/deploy.html @@ -2,12 +2,49 @@ Deploy in progress - +

-
+
Deploy in progress

Please try again in a few minutes.

diff --git a/public/logo.svg b/public/logo.svg deleted file mode 100644 index fc4553137f..0000000000 --- a/public/logo.svg +++ /dev/null @@ -1,9 +0,0 @@ - - - - - - - - - diff --git a/public/static.css b/public/static.css deleted file mode 100644 index 0a2b6060d4..0000000000 --- a/public/static.css +++ /dev/null @@ -1,36 +0,0 @@ -body { - color: #666; - text-align: center; - font-family: "Helvetica Neue", Helvetica, Arial, sans-serif; - margin: 0; - width: 800px; - margin: auto; - font-size: 14px; -} - -h1 { - font-size: 56px; - line-height: 100px; - font-weight: normal; - color: #456; -} - -h2 { - font-size: 24px; - color: #666; - line-height: 1.5em; -} - -h3 { - color: #456; - font-size: 20px; - font-weight: normal; - line-height: 28px; -} - -hr { - margin: 18px 0; - border: 0; - border-top: 1px solid #EEE; - border-bottom: 1px solid white; -} From b5c8f5727b5261d86ce52b976a355e7f223a563a Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Tue, 15 Mar 2016 11:34:49 +0100 Subject: [PATCH 3/9] Hide builds from Project's settings when the feature is disabled --- CHANGELOG | 1 + app/assets/javascripts/project_new.js.coffee | 13 ++++ app/views/projects/_builds_settings.html.haml | 60 +++++++++++++++++ app/views/projects/edit.html.haml | 65 +------------------ 4 files changed, 76 insertions(+), 63 deletions(-) create mode 100644 app/views/projects/_builds_settings.html.haml diff --git a/CHANGELOG b/CHANGELOG index d38646ece6..473acdaa5e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -22,6 +22,7 @@ v 8.6.0 (unreleased) - Rewrite logo to simplify SVG code (Sean Lang) - Allow to use YAML anchors when parsing the `.gitlab-ci.yml` (Pascal Bach) - Ignore jobs that start with `.` (hidden jobs) + - Hide builds from project's settings when the feature is disabled - Allow to pass name of created artifacts archive in `.gitlab-ci.yml` - Refactor and greatly improve search performance - Add support for cross-project label references diff --git a/app/assets/javascripts/project_new.js.coffee b/app/assets/javascripts/project_new.js.coffee index fecdb9fc2e..63dee4ed5d 100644 --- a/app/assets/javascripts/project_new.js.coffee +++ b/app/assets/javascripts/project_new.js.coffee @@ -3,3 +3,16 @@ class @ProjectNew $('.project-edit-container').on 'ajax:before', => $('.project-edit-container').hide() $('.save-project-loader').show() + @toggleSettings() + @toggleSettingsOnclick() + + + toggleSettings: -> + checked = $("#project_builds_enabled").prop("checked") + if checked + $('.builds-feature').show() + else + $('.builds-feature').hide() + + toggleSettingsOnclick: -> + $("#project_builds_enabled").on 'click', @toggleSettings diff --git a/app/views/projects/_builds_settings.html.haml b/app/views/projects/_builds_settings.html.haml new file mode 100644 index 0000000000..95ab9ecf3e --- /dev/null +++ b/app/views/projects/_builds_settings.html.haml @@ -0,0 +1,60 @@ +%fieldset.builds-feature + %legend + Builds: + .form-group + .col-sm-offset-2.col-sm-10 + %p Get recent application code using the following command: + .radio + = f.label :build_allow_git_fetch_false do + = f.radio_button :build_allow_git_fetch, 'false' + %strong git clone + %br + %span.descr Slower but makes sure you have a clean dir before every build + .radio + = f.label :build_allow_git_fetch_true do + = f.radio_button :build_allow_git_fetch, 'true' + %strong git fetch + %br + %span.descr Faster + + .form-group + = f.label :build_timeout_in_minutes, 'Timeout', class: 'control-label' + .col-sm-10 + = f.number_field :build_timeout_in_minutes, class: 'form-control', min: '0' + %p.help-block per build in minutes + .form-group + = f.label :build_coverage_regex, "Test coverage parsing", class: 'control-label' + .col-sm-10 + .input-group + %span.input-group-addon / + = f.text_field :build_coverage_regex, class: 'form-control', placeholder: '\(\d+.\d+\%\) covered' + %span.input-group-addon / + %p.help-block + We will use this regular expression to find test coverage output in build trace. + Leave blank if you want to disable this feature + .bs-callout.bs-callout-info + %p Below are examples of regex for existing tools: + %ul + %li + Simplecov (Ruby) - + %code \(\d+.\d+\%\) covered + %li + pytest-cov (Python) - + %code \d+\%\s*$ + %li + phpunit --coverage-text --colors=never (PHP) - + %code ^\s*Lines:\s*\d+.\d+\% + + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = f.label :public_builds do + = f.check_box :public_builds + %strong Public builds + .help-block Allow everyone to access builds for Public and Internal projects + + .form-group + = f.label :runners_token, "Runners token", class: 'control-label' + .col-sm-10 + = f.text_field :runners_token, class: "form-control", placeholder: 'xEeFCaDAB89' + %p.help-block The secure token used to checkout project. diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index f2e56081af..6d872cd0b2 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -84,6 +84,8 @@ %br %span.descr Share code pastes with others out of git repository + = render 'builds_settings', f: f + %fieldset.features %legend Project avatar: @@ -110,69 +112,6 @@ %hr = link_to 'Remove avatar', namespace_project_avatar_path(@project.namespace, @project), data: { confirm: "Project avatar will be removed. Are you sure?"}, method: :delete, class: "btn btn-remove btn-sm remove-avatar" - %fieldset.features - %legend - Continuous Integration - .form-group - .col-sm-offset-2.col-sm-10 - %p Get recent application code using the following command: - .radio - = f.label :build_allow_git_fetch_false do - = f.radio_button :build_allow_git_fetch, 'false' - %strong git clone - %br - %span.descr Slower but makes sure you have a clean dir before every build - .radio - = f.label :build_allow_git_fetch_true do - = f.radio_button :build_allow_git_fetch, 'true' - %strong git fetch - %br - %span.descr Faster - - .form-group - = f.label :build_timeout_in_minutes, 'Timeout', class: 'control-label' - .col-sm-10 - = f.number_field :build_timeout_in_minutes, class: 'form-control', min: '0' - %p.help-block per build in minutes - .form-group - = f.label :build_coverage_regex, "Test coverage parsing", class: 'control-label' - .col-sm-10 - .input-group - %span.input-group-addon / - = f.text_field :build_coverage_regex, class: 'form-control', placeholder: '\(\d+.\d+\%\) covered' - %span.input-group-addon / - %p.help-block - We will use this regular expression to find test coverage output in build trace. - Leave blank if you want to disable this feature - .bs-callout.bs-callout-info - %p Below are examples of regex for existing tools: - %ul - %li - Simplecov (Ruby) - - %code \(\d+.\d+\%\) covered - %li - pytest-cov (Python) - - %code \d+\%\s*$ - %li - phpunit --coverage-text --colors=never (PHP) - - %code ^\s*Lines:\s*\d+.\d+\% - - .form-group - .col-sm-offset-2.col-sm-10 - .checkbox - = f.label :public_builds do - = f.check_box :public_builds - %strong Public builds - .help-block Allow everyone to access builds for Public and Internal projects - - %fieldset.features - %legend - Advanced settings - .form-group - = f.label :runners_token, "CI token", class: 'control-label' - .col-sm-10 - = f.text_field :runners_token, class: "form-control", placeholder: 'xEeFCaDAB89' - %p.help-block The secure token used to checkout project. .form-actions = f.submit 'Save changes', class: "btn btn-save" From dce43b4bea88c753c6aeb43d2d283bef4d11eff1 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 15 Mar 2016 17:17:05 +0000 Subject: [PATCH 4/9] Pre-selects values on issue and merge request dropdown Closes #14290 --- app/helpers/issuables_helper.rb | 28 +++++++++++++++++++++ app/views/shared/issuable/_filter.html.haml | 8 +++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 2dfeddf736..29b78ac910 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -20,6 +20,34 @@ module IssuablesHelper base_issuable_scope(issuable).where('iid < ?', issuable.iid).first end + def user_dropdown_label(user_id, default_label) + return "Unassigned" if user_id == "0" + + user = @project.team.users.find_by(id: user_id) + + if user + user.name + else + default_label + end + end + + def labels_dropdown_label(label_name) + if !label_name + "Label" + else + label_name + end + end + + def milestone_dropdown_label(milestone_name) + if !milestone_name + "Milestone" + else + milestone_name + end + end + private def sidebar_gutter_collapsed? diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index 3eb0db276b..74ccaa53da 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -9,19 +9,19 @@ .filter-item.inline - if params[:author_id] = hidden_field_tag(:author_id, params[:author_id]) - = dropdown_tag("Author", options: { toggle_class: "js-user-search js-filter-submit js-author-search", title: "Filter by author", filter: true, dropdown_class: "dropdown-menu-user dropdown-menu-selectable dropdown-menu-author", + = dropdown_tag(user_dropdown_label(params[:author_id], "Author"), options: { toggle_class: "js-user-search js-filter-submit js-author-search", title: "Filter by author", filter: true, dropdown_class: "dropdown-menu-user dropdown-menu-selectable dropdown-menu-author", placeholder: "Search authors", data: { any_user: "Any Author", first_user: (current_user.username if current_user), current_user: true, project_id: (@project.id if @project), selected: params[:author_id], field_name: "author_id" } }) .filter-item.inline - if params[:assignee_id] = hidden_field_tag(:assignee_id, params[:assignee_id]) - = dropdown_tag("Assignee", options: { toggle_class: "js-user-search js-filter-submit js-assignee-search", title: "Filter by assignee", filter: true, dropdown_class: "dropdown-menu-user dropdown-menu-selectable dropdown-menu-assignee", + = dropdown_tag(user_dropdown_label(params[:assignee_id], "Assignee"), options: { toggle_class: "js-user-search js-filter-submit js-assignee-search", title: "Filter by assignee", filter: true, dropdown_class: "dropdown-menu-user dropdown-menu-selectable dropdown-menu-assignee", placeholder: "Search assignee", data: { any_user: "Any Assignee", first_user: (current_user.username if current_user), null_user: true, current_user: true, project_id: (@project.id if @project), selected: params[:assignee_id], field_name: "assignee_id" } }) .filter-item.inline.milestone-filter - if params[:milestone_title] = hidden_field_tag(:milestone_title, params[:milestone_title]) - = dropdown_tag("Milestone", options: { title: "Filter by milestone", toggle_class: 'js-milestone-select js-filter-submit', filter: true, dropdown_class: "dropdown-menu-selectable", + = dropdown_tag(milestone_dropdown_label(params[:milestone_title]), options: { title: "Filter by milestone", toggle_class: 'js-milestone-select js-filter-submit', filter: true, dropdown_class: "dropdown-menu-selectable", placeholder: "Search milestones", footer_content: true, data: { show_no: true, show_any: true, field_name: "milestone_title", selected: params[:milestone_title], project_id: (@project.id if @project), milestones: (namespace_project_milestones_path(@project.namespace, @project, :js) if @project) } }) do - if @project %ul.dropdown-footer-list @@ -42,7 +42,7 @@ .dropdown %button.dropdown-menu-toggle.js-label-select.js-filter-submit{type: "button", data: {toggle: "dropdown", field_name: "label_name", show_no: "true", show_any: "true", selected: params[:label_name], project_id: (@project.id if @project), labels: (namespace_project_labels_path(@project.namespace, @project, :js) if @project)}} %span.dropdown-toggle-text - Label + = labels_dropdown_label(params[:label_name]) = icon('chevron-down') .dropdown-menu.dropdown-select.dropdown-menu-paging.dropdown-menu-labels.dropdown-menu-selectable .dropdown-page-one From 2e07ee7f01e9bd525c98ae3f84025dd8699fdf8a Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 15 Mar 2016 18:28:25 +0000 Subject: [PATCH 5/9] Fixed failing tests --- app/helpers/issuables_helper.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 29b78ac910..b254c62da2 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -23,7 +23,8 @@ module IssuablesHelper def user_dropdown_label(user_id, default_label) return "Unassigned" if user_id == "0" - user = @project.team.users.find_by(id: user_id) + user = @project.team.users.find_by(id: user_id) if @project + user = User.find_by_id(user_id) if !@project if user user.name From 438d614b2780e280256af81f1e3f2f1847b3405e Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 16 Mar 2016 08:48:27 +0000 Subject: [PATCH 6/9] Removed label/milestone helper methods --- app/helpers/issuables_helper.rb | 26 +++++++-------------- app/views/shared/issuable/_filter.html.haml | 4 ++-- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index b254c62da2..23f389b8c5 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -23,8 +23,14 @@ module IssuablesHelper def user_dropdown_label(user_id, default_label) return "Unassigned" if user_id == "0" - user = @project.team.users.find_by(id: user_id) if @project - user = User.find_by_id(user_id) if !@project + if @project + member = @project.team.find_member(user_id) + if member + user = member.user + end + else + user = User.find(user_id) + end if user user.name @@ -33,22 +39,6 @@ module IssuablesHelper end end - def labels_dropdown_label(label_name) - if !label_name - "Label" - else - label_name - end - end - - def milestone_dropdown_label(milestone_name) - if !milestone_name - "Milestone" - else - milestone_name - end - end - private def sidebar_gutter_collapsed? diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index 74ccaa53da..dfdc84ba4c 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -21,7 +21,7 @@ .filter-item.inline.milestone-filter - if params[:milestone_title] = hidden_field_tag(:milestone_title, params[:milestone_title]) - = dropdown_tag(milestone_dropdown_label(params[:milestone_title]), options: { title: "Filter by milestone", toggle_class: 'js-milestone-select js-filter-submit', filter: true, dropdown_class: "dropdown-menu-selectable", + = dropdown_tag(h(params[:milestone_name] || "Milestone"), options: { title: "Filter by milestone", toggle_class: 'js-milestone-select js-filter-submit', filter: true, dropdown_class: "dropdown-menu-selectable", placeholder: "Search milestones", footer_content: true, data: { show_no: true, show_any: true, field_name: "milestone_title", selected: params[:milestone_title], project_id: (@project.id if @project), milestones: (namespace_project_milestones_path(@project.namespace, @project, :js) if @project) } }) do - if @project %ul.dropdown-footer-list @@ -42,7 +42,7 @@ .dropdown %button.dropdown-menu-toggle.js-label-select.js-filter-submit{type: "button", data: {toggle: "dropdown", field_name: "label_name", show_no: "true", show_any: "true", selected: params[:label_name], project_id: (@project.id if @project), labels: (namespace_project_labels_path(@project.namespace, @project, :js) if @project)}} %span.dropdown-toggle-text - = labels_dropdown_label(params[:label_name]) + = h(params[:label_name] || "Label") = icon('chevron-down') .dropdown-menu.dropdown-select.dropdown-menu-paging.dropdown-menu-labels.dropdown-menu-selectable .dropdown-page-one From 81439c9093707d8964258db9481715b3ca0e7a3a Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 17 Mar 2016 09:13:20 +0000 Subject: [PATCH 7/9] Moved code to single line if --- app/helpers/issuables_helper.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 23f389b8c5..81df209439 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -25,11 +25,9 @@ module IssuablesHelper if @project member = @project.team.find_member(user_id) - if member - user = member.user - end + user = member.user if member else - user = User.find(user_id) + user = User.find_by(id: user_id) end if user From 028c751040b6acc29f3127d5f2d119b899a85205 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 17 Mar 2016 12:20:24 -0500 Subject: [PATCH 8/9] Revert link color from blue --- app/assets/stylesheets/framework/blocks.scss | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/assets/stylesheets/framework/blocks.scss b/app/assets/stylesheets/framework/blocks.scss index 90c3ce0e84..c36f29dda0 100644 --- a/app/assets/stylesheets/framework/blocks.scss +++ b/app/assets/stylesheets/framework/blocks.scss @@ -28,10 +28,6 @@ border-bottom: 1px solid $border-color; color: $gl-gray; - a { - color: $md-link-color; - } - &.oneline-block { line-height: 42px; } From cd05d3f78d2093baab39f6c3c114d1ab7138309f Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 17 Mar 2016 16:53:05 +0100 Subject: [PATCH 9/9] Cache project avatars stored in Git The avatar logic has been moved from Project to Repository as this makes caching easier. The logic itself in turn has been changed so that the logo file names are cached in Redis. This cache is flushed upon pushing a commit but _only_ if: 1. The commit was pushed to the default branch 2. The commit actually changes any of the logo files If no branch or commit is given the cache is flushed anyway, this ensures that calling Repository#expire_cache without any arguments still flushes the avatar cache (e.g. this is used when removing a project). Fixes gitlab-org/gitlab-ce#14363 --- app/models/project.rb | 5 +- app/models/repository.rb | 36 +++++++++++- app/services/git_push_service.rb | 2 +- spec/models/repository_spec.rb | 81 +++++++++++++++++++++++++- spec/services/git_push_service_spec.rb | 9 ++- 5 files changed, 120 insertions(+), 13 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index ab4913e99a..412c6c6732 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -571,10 +571,7 @@ class Project < ActiveRecord::Base end def avatar_in_git - @avatar_file ||= 'logo.png' if repository.blob_at_branch('master', 'logo.png') - @avatar_file ||= 'logo.jpg' if repository.blob_at_branch('master', 'logo.jpg') - @avatar_file ||= 'logo.gif' if repository.blob_at_branch('master', 'logo.gif') - @avatar_file + repository.avatar end def avatar_url diff --git a/app/models/repository.rb b/app/models/repository.rb index e555e97689..036919c27b 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -3,6 +3,10 @@ require 'securerandom' class Repository class CommitError < StandardError; end + # Files to use as a project avatar in case no avatar was uploaded via the web + # UI. + AVATAR_FILES = %w{logo.png logo.jpg logo.gif} + include Gitlab::ShellAdapter attr_accessor :path_with_namespace, :project @@ -241,12 +245,13 @@ class Repository @branches = nil end - def expire_cache(branch_name = nil) + def expire_cache(branch_name = nil, revision = nil) cache_keys.each do |key| cache.expire(key) end expire_branch_cache(branch_name) + expire_avatar_cache(branch_name, revision) # This ensures this particular cache is flushed after the first commit to a # new repository. @@ -316,6 +321,23 @@ class Repository cache.expire(:branch_names) end + def expire_avatar_cache(branch_name = nil, revision = nil) + # Avatars are pulled from the default branch, thus if somebody pushes to a + # different branch there's no need to expire anything. + return if branch_name && branch_name != root_ref + + # We don't want to flush the cache if the commit didn't actually make any + # changes to any of the possible avatar files. + if revision && commit = self.commit(revision) + return unless commit.diffs. + any? { |diff| AVATAR_FILES.include?(diff.new_path) } + end + + cache.expire(:avatar) + + @avatar = nil + end + # Runs code just before a repository is deleted. def before_delete expire_cache if exists? @@ -350,8 +372,8 @@ class Repository end # Runs code after a new commit has been pushed. - def after_push_commit(branch_name) - expire_cache(branch_name) + def after_push_commit(branch_name, revision) + expire_cache(branch_name, revision) end # Runs code after a new branch has been created. @@ -857,6 +879,14 @@ class Repository end end + def avatar + @avatar ||= cache.fetch(:avatar) do + AVATAR_FILES.find do |file| + blob_at_branch('master', file) + end + end + end + private def cache diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index d840ab5e34..14e2a2c069 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -17,7 +17,7 @@ class GitPushService < BaseService # 6. Checks if the project's main language has changed # def execute - @project.repository.after_push_commit(branch_name) + @project.repository.after_push_commit(branch_name, params[:newrev]) if push_remove_branch? @project.repository.after_remove_branch diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index fc2ab2d993..536fe66b21 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -597,9 +597,9 @@ describe Repository, models: true do describe '#after_push_commit' do it 'flushes the cache' do - expect(repository).to receive(:expire_cache).with('master') + expect(repository).to receive(:expire_cache).with('master', '123') - repository.after_push_commit('master') + repository.after_push_commit('master', '123') end end @@ -703,4 +703,81 @@ describe Repository, models: true do repository.rm_tag('8.5') end end + + describe '#avatar' do + it 'returns the first avatar file found in the repository' do + expect(repository).to receive(:blob_at_branch). + with('master', 'logo.png'). + and_return(true) + + expect(repository.avatar).to eq('logo.png') + end + + it 'caches the output' do + allow(repository).to receive(:blob_at_branch). + with('master', 'logo.png'). + and_return(true) + + expect(repository.avatar).to eq('logo.png') + + expect(repository).to_not receive(:blob_at_branch) + expect(repository.avatar).to eq('logo.png') + end + end + + describe '#expire_avatar_cache' do + let(:cache) { repository.send(:cache) } + + before do + allow(repository).to receive(:cache).and_return(cache) + end + + context 'without a branch or revision' do + it 'flushes the cache' do + expect(cache).to receive(:expire).with(:avatar) + + repository.expire_avatar_cache + end + end + + context 'with a branch' do + it 'does not flush the cache if the branch is not the default branch' do + expect(cache).not_to receive(:expire) + + repository.expire_avatar_cache('cats') + end + + it 'flushes the cache if the branch equals the default branch' do + expect(cache).to receive(:expire).with(:avatar) + + repository.expire_avatar_cache(repository.root_ref) + end + end + + context 'with a branch and revision' do + let(:commit) { double(:commit) } + + before do + allow(repository).to receive(:commit).and_return(commit) + end + + it 'does not flush the cache if the commit does not change any logos' do + diff = double(:diff, new_path: 'test.txt') + + expect(commit).to receive(:diffs).and_return([diff]) + expect(cache).not_to receive(:expire) + + repository.expire_avatar_cache(repository.root_ref, '123') + end + + it 'flushes the cache if the commit changes any of the logos' do + diff = double(:diff, new_path: Repository::AVATAR_FILES[0]) + + expect(commit).to receive(:diffs).and_return([diff]) + expect(cache).to receive(:expire).with(:avatar) + + repository.expire_avatar_cache(repository.root_ref, '123') + end + end + end end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 145bc93756..b49ca96e8e 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -29,7 +29,8 @@ describe GitPushService, services: true do it { is_expected.to be_truthy } it 'flushes general cached data' do - expect(project.repository).to receive(:expire_cache).with('master') + expect(project.repository).to receive(:expire_cache). + with('master', newrev) subject end @@ -46,7 +47,8 @@ describe GitPushService, services: true do it { is_expected.to be_truthy } it 'flushes general cached data' do - expect(project.repository).to receive(:expire_cache).with('master') + expect(project.repository).to receive(:expire_cache). + with('master', newrev) subject end @@ -65,7 +67,8 @@ describe GitPushService, services: true do end it 'flushes general cached data' do - expect(project.repository).to receive(:expire_cache).with('master') + expect(project.repository).to receive(:expire_cache). + with('master', newrev) subject end