diff options
56 files changed, 886 insertions, 571 deletions
diff --git a/.rubocop_todo/capybara/visibility_matcher.yml b/.rubocop_todo/rspec/capybara/visibility_matcher.yml index f9e8c982903..f9e8c982903 100644 --- a/.rubocop_todo/capybara/visibility_matcher.yml +++ b/.rubocop_todo/rspec/capybara/visibility_matcher.yml diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index beb66fcc2b4..699e4a0c856 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -e4d8f69ffa2efd3f2cb0adff5fa66f367f66f6fb +c13d9d902ef8175a0b1165ef0bc8643fb37b7897 diff --git a/app/assets/javascripts/pages/shared/wikis/components/wiki_form.vue b/app/assets/javascripts/pages/shared/wikis/components/wiki_form.vue index 9acc1cb62a1..bd3929c5f59 100644 --- a/app/assets/javascripts/pages/shared/wikis/components/wiki_form.vue +++ b/app/assets/javascripts/pages/shared/wikis/components/wiki_form.vue @@ -8,21 +8,18 @@ import { GlFormGroup, GlFormInput, GlFormSelect, - GlSegmentedControl, } from '@gitlab/ui'; -import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue'; -import axios from '~/lib/utils/axios_utils'; import csrf from '~/lib/utils/csrf'; import { setUrlFragment } from '~/lib/utils/url_utility'; import { s__, sprintf } from '~/locale'; import Tracking from '~/tracking'; -import MarkdownField from '~/vue_shared/components/markdown/field.vue'; +import MarkdownEditor from '~/vue_shared/components/markdown/markdown_editor.vue'; import { - CONTENT_EDITOR_LOADED_ACTION, SAVED_USING_CONTENT_EDITOR_ACTION, WIKI_CONTENT_EDITOR_TRACKING_LABEL, WIKI_FORMAT_LABEL, WIKI_FORMAT_UPDATED_ACTION, + CONTENT_EDITOR_LOADED_ACTION, } from '../constants'; const trackingMixin = Tracking.mixin({ @@ -74,10 +71,6 @@ export default { }, cancel: s__('WikiPage|Cancel'), }, - switchEditingControlOptions: [ - { text: s__('Wiki Page|Source'), value: 'source' }, - { text: s__('Wiki Page|Rich text'), value: 'richText' }, - ], components: { GlIcon, GlForm, @@ -87,13 +80,7 @@ export default { GlSprintf, GlLink, GlButton, - GlSegmentedControl, - MarkdownField, - LocalStorageSync, - ContentEditor: () => - import( - /* webpackChunkName: 'content_editor' */ '~/content_editor/components/content_editor.vue' - ), + MarkdownEditor, }, mixins: [trackingMixin], inject: ['formatOptions', 'pageInfo'], @@ -106,7 +93,7 @@ export default { commitMessage: '', isDirty: false, contentEditorEmpty: false, - switchEditingControlDisabled: false, + isContentEditorActive: false, }; }, computed: { @@ -162,12 +149,6 @@ export default { disableSubmitButton() { return this.noContent || !this.title; }, - isContentEditorActive() { - return this.isMarkdownFormat && this.useContentEditor; - }, - useContentEditor() { - return this.editingMode === 'richText'; - }, }, mounted() { this.updateCommitMessage(); @@ -178,23 +159,10 @@ export default { window.removeEventListener('beforeunload', this.onPageUnload); }, methods: { - renderMarkdown(content) { - return axios - .post(this.pageInfo.markdownPreviewPath, { text: content }) - .then(({ data }) => data.body); - }, - - setEditingMode(editingMode) { - this.editingMode = editingMode; - }, - async handleFormSubmit(e) { e.preventDefault(); - if (this.useContentEditor) { - this.trackFormSubmit(); - } - + this.trackFormSubmit(); this.trackWikiFormat(); // Wait until form field values are refreshed @@ -205,16 +173,6 @@ export default { this.isDirty = false; }, - handleContentChange() { - this.isDirty = true; - }, - - handleContentEditorChange({ empty, markdown, changed }) { - this.contentEditorEmpty = empty; - this.isDirty = changed; - this.content = markdown; - }, - onPageUnload(event) { if (!this.isDirty) return undefined; @@ -235,8 +193,13 @@ export default { this.commitMessage = newCommitMessage; }, - trackContentEditorLoaded() { - this.track(CONTENT_EDITOR_LOADED_ACTION); + notifyContentEditorActive() { + this.isContentEditorActive = true; + this.trackContentEditorLoaded(); + }, + + notifyContentEditorInactive() { + this.isContentEditorActive = false; }, trackFormSubmit() { @@ -256,12 +219,12 @@ export default { }); }, - enableSwitchEditingControl() { - this.switchEditingControlDisabled = false; + trackContentEditorLoaded() { + this.track(CONTENT_EDITOR_LOADED_ACTION); }, - disableSwitchEditingControl() { - this.switchEditingControlDisabled = true; + checkDirty(markdown) { + this.isDirty = this.pageInfo.content !== markdown; }, }, }; @@ -329,74 +292,22 @@ export default { <div class="row" data-testid="wiki-form-content-fieldset"> <div class="col-sm-12 row-sm-5"> <gl-form-group> - <div v-if="isMarkdownFormat" class="gl-display-flex gl-justify-content-start gl-mb-3"> - <gl-segmented-control - data-testid="toggle-editing-mode-button" - data-qa-selector="editing_mode_button" - class="gl-display-flex" - :checked="editingMode" - :options="$options.switchEditingControlOptions" - :disabled="switchEditingControlDisabled" - @input="setEditingMode" - /> - </div> - <local-storage-sync - storage-key="gl-wiki-content-editor-enabled" - :value="editingMode" - @input="setEditingMode" - /> - <markdown-field - v-if="!isContentEditorActive" - :markdown-preview-path="pageInfo.markdownPreviewPath" - :can-attach-file="true" - :enable-autocomplete="true" - :textarea-value="content" + <markdown-editor + v-model="content" + :render-markdown-path="pageInfo.markdownPreviewPath" :markdown-docs-path="pageInfo.markdownHelpPath" :uploads-path="pageInfo.uploadsPath" + :enable-content-editor="isMarkdownFormat" :enable-preview="isMarkdownFormat" - class="bordered-box" - > - <template #textarea> - <textarea - id="wiki_content" - ref="textarea" - v-model="content" - name="wiki[content]" - class="note-textarea js-gfm-input js-autosize markdown-area" - dir="auto" - data-supports-quick-actions="false" - data-qa-selector="wiki_content_textarea" - :autofocus="pageInfo.persisted" - :aria-label="$options.i18n.content.label" - :placeholder="$options.i18n.content.placeholder" - @input="handleContentChange" - > - </textarea> - </template> - </markdown-field> - <div v-if="isContentEditorActive"> - <content-editor - :render-markdown="renderMarkdown" - :uploads-path="pageInfo.uploadsPath" - :markdown="content" - @initialized="trackContentEditorLoaded" - @change="handleContentEditorChange" - @loading="disableSwitchEditingControl" - @loadingSuccess="enableSwitchEditingControl" - @loadingError="enableSwitchEditingControl" - /> - <input - id="wiki_content" - v-model.trim="content" - type="hidden" - name="wiki[content]" - data-qa-selector="wiki_hidden_content" - /> - </div> - - <div class="clearfix"></div> - <div class="error-alert"></div> - + :autofocus="pageInfo.persisted" + :form-field-placeholder="$options.i18n.content.placeholder" + :form-field-aria-label="$options.i18n.content.label" + form-field-id="wiki_content" + form-field-name="wiki[content]" + @contentEditor="notifyContentEditorActive" + @markdownField="notifyContentEditorInactive" + @input="checkDirty" + /> <div class="form-text gl-text-gray-600"> <gl-sprintf v-if="displayWikiSpecificMarkdownHelp" diff --git a/app/assets/javascripts/set_status_modal/set_status_form.vue b/app/assets/javascripts/set_status_modal/set_status_form.vue index 7f9a30b7ff1..e5f6ce07189 100644 --- a/app/assets/javascripts/set_status_modal/set_status_form.vue +++ b/app/assets/javascripts/set_status_modal/set_status_form.vue @@ -161,11 +161,7 @@ export default { @click="handleEmojiClick" > <template #button-content> - <span - v-if="noEmoji" - class="no-emoji-placeholder position-relative" - data-testid="no-emoji-placeholder" - > + <span v-if="noEmoji" class="gl-relative" data-testid="no-emoji-placeholder"> <gl-icon name="slight-smile" class="award-control-icon-neutral" /> <gl-icon name="smiley" class="award-control-icon-positive" /> <gl-icon name="smile" class="award-control-icon-super-positive" /> diff --git a/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions_widget.vue b/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions_widget.vue index e5bee4df9b8..61e4aa5a8e6 100644 --- a/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions_widget.vue +++ b/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions_widget.vue @@ -1,5 +1,5 @@ <script> -import { GlIcon, GlLoadingIcon, GlToggle, GlTooltipDirective } from '@gitlab/ui'; +import { GlDropdownForm, GlIcon, GlLoadingIcon, GlToggle, GlTooltipDirective } from '@gitlab/ui'; import createFlash from '~/flash'; import { IssuableType } from '~/issues/constants'; import { isLoggedIn } from '~/lib/utils/common_utils'; @@ -22,6 +22,7 @@ export default { GlTooltip: GlTooltipDirective, }, components: { + GlDropdownForm, GlIcon, GlLoadingIcon, GlToggle, @@ -181,7 +182,7 @@ export default { </script> <template> - <div v-if="isMergeRequest" class="gl-new-dropdown-item"> + <gl-dropdown-form v-if="isMergeRequest" class="gl-new-dropdown-item"> <div class="gl-px-5 gl-pb-2 gl-pt-1"> <gl-toggle :value="subscribed" @@ -192,7 +193,7 @@ export default { @change="toggleSubscribed" /> </div> - </div> + </gl-dropdown-form> <sidebar-editable-item v-else ref="editable" diff --git a/app/assets/javascripts/vue_shared/components/markdown/markdown_editor.vue b/app/assets/javascripts/vue_shared/components/markdown/markdown_editor.vue new file mode 100644 index 00000000000..fd8496fa313 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/markdown/markdown_editor.vue @@ -0,0 +1,178 @@ +<script> +import { GlSegmentedControl } from '@gitlab/ui'; +import { __ } from '~/locale'; +import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue'; +import axios from '~/lib/utils/axios_utils'; +import { EDITING_MODE_MARKDOWN_FIELD, EDITING_MODE_CONTENT_EDITOR } from '../../constants'; +import MarkdownField from './field.vue'; + +export default { + components: { + MarkdownField, + LocalStorageSync, + GlSegmentedControl, + ContentEditor: () => + import( + /* webpackChunkName: 'content_editor' */ '~/content_editor/components/content_editor.vue' + ), + }, + props: { + value: { + type: String, + required: true, + }, + renderMarkdownPath: { + type: String, + required: true, + }, + markdownDocsPath: { + type: String, + required: true, + }, + uploadsPath: { + type: String, + required: true, + }, + enableContentEditor: { + type: Boolean, + required: false, + default: true, + }, + formFieldId: { + type: String, + required: true, + }, + formFieldName: { + type: String, + required: true, + }, + enablePreview: { + type: Boolean, + required: false, + default: true, + }, + enableAutocomplete: { + type: Boolean, + required: false, + default: true, + }, + autofocus: { + type: Boolean, + required: false, + default: false, + }, + formFieldPlaceholder: { + type: String, + required: false, + default: '', + }, + formFieldAriaLabel: { + type: String, + required: false, + default: '', + }, + }, + data() { + return { + editingMode: EDITING_MODE_MARKDOWN_FIELD, + switchEditingControlEnabled: true, + }; + }, + computed: { + isContentEditorActive() { + return this.enableContentEditor && this.editingMode === EDITING_MODE_CONTENT_EDITOR; + }, + }, + methods: { + updateMarkdownFromContentEditor({ markdown }) { + this.$emit('input', markdown); + }, + updateMarkdownFromMarkdownField({ target }) { + this.$emit('input', target.value); + }, + enableSwitchEditingControl() { + this.switchEditingControlEnabled = true; + }, + disableSwitchEditingControl() { + this.switchEditingControlEnabled = false; + }, + renderMarkdown(markdown) { + return axios.post(this.renderMarkdownPath, { text: markdown }).then(({ data }) => data.body); + }, + notifyEditingModeChange(editingMode) { + this.$emit(editingMode); + }, + }, + switchEditingControlOptions: [ + { text: __('Source'), value: EDITING_MODE_MARKDOWN_FIELD }, + { text: __('Rich text'), value: EDITING_MODE_CONTENT_EDITOR }, + ], +}; +</script> +<template> + <div> + <div class="gl-display-flex gl-justify-content-start gl-mb-3"> + <gl-segmented-control + v-model="editingMode" + data-testid="toggle-editing-mode-button" + data-qa-selector="editing_mode_button" + class="gl-display-flex" + :options="$options.switchEditingControlOptions" + :disabled="!enableContentEditor || !switchEditingControlEnabled" + @change="notifyEditingModeChange" + /> + </div> + <local-storage-sync + v-model="editingMode" + storage-key="gl-wiki-content-editor-enabled" + @input="notifyEditingModeChange" + /> + <markdown-field + v-if="!isContentEditorActive" + :markdown-preview-path="renderMarkdownPath" + can-attach-file + :enable-autocomplete="enableAutocomplete" + :textarea-value="value" + :markdown-docs-path="markdownDocsPath" + :uploads-path="uploadsPath" + :enable-preview="enablePreview" + class="bordered-box" + > + <template #textarea> + <textarea + :id="formFieldId" + ref="textarea" + :value="value" + :name="formFieldName" + class="note-textarea js-gfm-input js-autosize markdown-area" + dir="auto" + data-supports-quick-actions="false" + data-qa-selector="markdown_editor_form_field" + :autofocus="autofocus" + :aria-label="formFieldAriaLabel" + :placeholder="formFieldPlaceholder" + @input="updateMarkdownFromMarkdownField" + > + </textarea> + </template> + </markdown-field> + <div v-else> + <content-editor + :render-markdown="renderMarkdown" + :uploads-path="uploadsPath" + :markdown="value" + @change="updateMarkdownFromContentEditor" + @loading="disableSwitchEditingControl" + @loadingSuccess="enableSwitchEditingControl" + @loadingError="enableSwitchEditingControl" + /> + <input + :id="formFieldId" + :value="value" + :name="formFieldName" + data-qa-selector="markdown_editor_form_field" + type="hidden" + /> + </div> + </div> +</template> diff --git a/app/assets/javascripts/vue_shared/constants.js b/app/assets/javascripts/vue_shared/constants.js index b6d69faebb5..a851f84ed2f 100644 --- a/app/assets/javascripts/vue_shared/constants.js +++ b/app/assets/javascripts/vue_shared/constants.js @@ -93,3 +93,6 @@ export const confidentialityInfoText = (workspaceType, issuableType) => : __('at least the Reporter role'), }, ); + +export const EDITING_MODE_MARKDOWN_FIELD = 'markdownField'; +export const EDITING_MODE_CONTENT_EDITOR = 'contentEditor'; diff --git a/app/assets/stylesheets/page_bundles/merge_requests.scss b/app/assets/stylesheets/page_bundles/merge_requests.scss index 719e4b5aa6d..4b7ee575d73 100644 --- a/app/assets/stylesheets/page_bundles/merge_requests.scss +++ b/app/assets/stylesheets/page_bundles/merge_requests.scss @@ -832,6 +832,8 @@ $tabs-holder-z-index: 250; .detail-page-header-actions { .gl-toggle { @include gl-ml-auto; + @include gl-rounded-pill; + @include gl-w-9; } } @@ -844,3 +846,7 @@ $tabs-holder-z-index: 250; @include gl-font-weight-normal; } } + +.dropdown-menu li button.gl-toggle:not(.is-checked) { + background: $gray-400; +} diff --git a/app/assets/stylesheets/page_bundles/profile.scss b/app/assets/stylesheets/page_bundles/profile.scss index 59b8823c113..356f57678f3 100644 --- a/app/assets/stylesheets/page_bundles/profile.scss +++ b/app/assets/stylesheets/page_bundles/profile.scss @@ -1,4 +1,46 @@ @import 'mixins_and_variables_and_functions'; +@import 'framework/buttons'; + +.avatar-image { + margin-bottom: $grid-size; + + .avatar { + float: none; + } + + @include media-breakpoint-up(sm) { + float: left; + margin-bottom: 0; + } +} + +.edit-user { + .emoji-menu-toggle-button { + @include emoji-menu-toggle-button; + } + + @include media-breakpoint-down(sm) { + .input-md, + .input-lg { + max-width: 100%; + } + } +} + +.modal-profile-crop { + .modal-dialog { + width: 380px; + + @include media-breakpoint-down(xs) { + width: auto; + } + } + + .profile-crop-image-container { + height: 300px; + margin: 0 auto; + } +} .calendar-block { padding-left: 0; diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index 951e31ef768..55b9d749bbb 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -1,22 +1,3 @@ -.avatar-image { - margin-bottom: $grid-size; - - .avatar { - float: none; - } - - @include media-breakpoint-up(sm) { - float: left; - margin-bottom: 0; - } -} - -.avatar-file-name { - position: relative; - top: 2px; - display: inline-block; -} - .account-well { padding: 10px; background-color: $gray-light; @@ -29,13 +10,6 @@ } } -.user-avatar-button { - .file-name { - display: inline-block; - padding-left: 10px; - } -} - .subkeys-list { @include basic-list; @@ -113,26 +87,6 @@ } } -.modal-profile-crop { - .modal-dialog { - width: 380px; - - @include media-breakpoint-down(xs) { - width: auto; - } - } - - .profile-crop-image-container { - height: 300px; - margin: 0 auto; - } - - .crop-controls { - padding: 10px 0 0; - text-align: center; - } -} - .created-personal-access-token-container { .btn-clipboard { border: 1px solid $border-color; @@ -247,36 +201,6 @@ table.u2f-registrations { } } -.edit-user { - svg { - fill: $gl-text-color-secondary; - } - - .form-group > label { - font-weight: $gl-font-weight-bold; - } - - .form-group > .form-text { - font-size: $gl-font-size; - } - - .emoji-menu-toggle-button { - @include emoji-menu-toggle-button; - padding: 6px 10px; - - .no-emoji-placeholder { - position: relative; - } - } - - @include media-breakpoint-down(sm) { - .input-md, - .input-lg { - max-width: 100%; - } - } -} - .help-block { color: $gl-text-color-secondary; } diff --git a/app/mailers/previews/notify_preview.rb b/app/mailers/previews/notify_preview.rb index 7b7454340e8..60a68ce97b1 100644 --- a/app/mailers/previews/notify_preview.rb +++ b/app/mailers/previews/notify_preview.rb @@ -217,6 +217,10 @@ class NotifyPreview < ActionMailer::Preview Notify.project_was_exported_email(user, project).message end + def request_review_merge_request_email + Notify.request_review_merge_request_email(user.id, merge_request.id, user.id).message + end + private def project diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 1e328c3c573..5293fd36122 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -760,8 +760,14 @@ module Ci # There is no ActiveRecord relation between Ci::Pipeline and notes # as they are related to a commit sha. This method helps importing # them using the +Gitlab::ImportExport::Project::RelationFactory+ class. - def notes=(notes) - notes.each do |note| + def notes=(notes_to_save) + notes_to_save.reject! do |note_to_save| + notes.any? do |note| + [note_to_save.note, note_to_save.created_at.to_i] == [note.note, note.created_at.to_i] + end + end + + notes_to_save.each do |note| note[:id] = nil note[:commit_id] = sha note[:noteable_id] = self['id'] diff --git a/app/views/notify/request_review_merge_request_email.html.haml b/app/views/notify/request_review_merge_request_email.html.haml index d1f72f6529a..a8c7df79ff3 100644 --- a/app/views/notify/request_review_merge_request_email.html.haml +++ b/app/views/notify/request_review_merge_request_email.html.haml @@ -1,2 +1,2 @@ %p - #{sanitize_name(@updated_by.name)} requested a new review on #{merge_request_reference_link(@merge_request)}. + = html_escape(s_('Notify|%{name} requested a new review on %{mr_link}.')) % {name: sanitize_name(@updated_by.name), mr_link: merge_request_reference_link(@merge_request).html_safe} diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index cf0ccc48ad2..8fe9157dce0 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -1,5 +1,6 @@ - breadcrumb_title s_("Profiles|Edit Profile") - page_title s_("Profiles|Edit Profile") +- add_page_specific_style 'page_bundles/profile' - @content_class = "limit-container-width" unless fluid_layout - gravatar_link = link_to Gitlab.config.gravatar.host, 'https://' + Gitlab.config.gravatar.host @@ -27,9 +28,9 @@ = link_to avatar_icon_for_user(@user, 400), target: '_blank', rel: 'noopener noreferrer' do = image_tag avatar_icon_for_user(@user, 96), alt: '', class: 'avatar s96' %h5.gl-mt-0= s_("Profiles|Upload new avatar") - .gl-my-3 + .gl-display-flex.gl-align-items-center.gl-my-3 %button.gl-button.btn.btn-default.js-choose-user-avatar-button{ type: 'button' }= s_("Profiles|Choose file...") - %span.avatar-file-name.gl-ml-3.js-avatar-filename= s_("Profiles|No file chosen.") + %span.gl-ml-3.js-avatar-filename= s_("Profiles|No file chosen.") = f.file_field :avatar, class: 'js-user-avatar-input hidden', accept: 'image/*' .gl-text-gray-500= s_("Profiles|The maximum file size allowed is 200KB.") - if @user.avatar? @@ -152,7 +153,7 @@ .modal-body .profile-crop-image-container %img.modal-profile-crop-image{ alt: s_("Profiles|Avatar cropper") } - .crop-controls + .gl-text-center.gl-mt-4 .btn-group %button.btn.gl-button.btn-default{ data: { method: 'zoom', option: '-0.1' } } %span diff --git a/config/feature_flags/development/ci_rules_changes_compare.yml b/config/feature_flags/development/ci_rules_changes_compare.yml deleted file mode 100644 index 094692def26..00000000000 --- a/config/feature_flags/development/ci_rules_changes_compare.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: ci_rules_changes_compare -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/90968 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/366412 -milestone: '15.3' -type: development -group: group::pipeline authoring -default_enabled: true diff --git a/db/post_migrate/20220920124709_backfill_internal_on_notes.rb b/db/post_migrate/20220920124709_backfill_internal_on_notes.rb new file mode 100644 index 00000000000..0d737195907 --- /dev/null +++ b/db/post_migrate/20220920124709_backfill_internal_on_notes.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class BackfillInternalOnNotes < Gitlab::Database::Migration[2.0] + MIGRATION = 'BackfillInternalOnNotes' + DELAY_INTERVAL = 2.minutes + TABLE = :notes + BATCH_SIZE = 2000 + SUB_BATCH_SIZE = 10 + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + def up + queue_batched_background_migration( + MIGRATION, + TABLE, + :id, + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + end + + def down + delete_batched_background_migration(MIGRATION, TABLE, :id, []) + end +end diff --git a/db/schema_migrations/20220920124709 b/db/schema_migrations/20220920124709 new file mode 100644 index 00000000000..e3b015d44a7 --- /dev/null +++ b/db/schema_migrations/20220920124709 @@ -0,0 +1 @@ +4a975867dc0539049902229521b4d94f940817ffd9196810856c8eb962c57e62
\ No newline at end of file diff --git a/doc/architecture/blueprints/ci_data_decay/pipeline_partitioning.md b/doc/architecture/blueprints/ci_data_decay/pipeline_partitioning.md index d49d9b2600f..c1dd7dc09bc 100644 --- a/doc/architecture/blueprints/ci_data_decay/pipeline_partitioning.md +++ b/doc/architecture/blueprints/ci_data_decay/pipeline_partitioning.md @@ -351,6 +351,22 @@ scope block takes an argument). Preloading instance dependent scopes is not supported. ``` +### Primary key + +Primary key must include the partitioning key column to partition the table. + +We first create a unique index including the `(id, partition_id)`. +Then, we drop the primary key constraint and use the new index created to set +the new primary key constraint. + +We must set the primary key explicitly as `ActiveRecord` does not support composite primary keys. + +```ruby +class Model + self.primary_key = 'id' +end +``` + ### Foreign keys Foreign keys must reference columns that either are a primary key or form a diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index 478818ad361..6075a646532 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -3343,7 +3343,8 @@ In this example, both jobs have the same behavior. ##### `rules:changes:compare_to` -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/293645) in GitLab 15.3 [with a flag](../../administration/feature_flags.md) named `ci_rules_changes_compare`. Enabled by default. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/293645) in GitLab 15.3 [with a flag](../../administration/feature_flags.md) named `ci_rules_changes_compare`. Enabled by default. +> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/366412) in GitLab 15.5. Feature flag `ci_rules_changes_compare` removed. Use `rules:changes:compare_to` to specify which ref to compare against for changes to the files listed under [`rules:changes:paths`](#ruleschangespaths). diff --git a/lib/gitlab/background_migration/backfill_internal_on_notes.rb b/lib/gitlab/background_migration/backfill_internal_on_notes.rb new file mode 100644 index 00000000000..300f2cff6ca --- /dev/null +++ b/lib/gitlab/background_migration/backfill_internal_on_notes.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # This syncs the data to `internal` from `confidential` as we rename the column. + class BackfillInternalOnNotes < BatchedMigrationJob + scope_to -> (relation) { relation.where(confidential: true) } + + def perform + each_sub_batch(operation_name: :update_all) do |sub_batch| + sub_batch.update_all(internal: true) + end + end + end + end +end diff --git a/lib/gitlab/ci/build/rules/rule/clause/changes.rb b/lib/gitlab/ci/build/rules/rule/clause/changes.rb index 1034f5eacef..4069a683ceb 100644 --- a/lib/gitlab/ci/build/rules/rule/clause/changes.rb +++ b/lib/gitlab/ci/build/rules/rule/clause/changes.rb @@ -41,7 +41,6 @@ module Gitlab def find_modified_paths(pipeline) return unless pipeline - return pipeline.modified_paths unless ::Feature.enabled?(:ci_rules_changes_compare, pipeline.project) compare_to_sha = find_compare_to_sha(pipeline) diff --git a/lib/gitlab/ci/templates/Pages/Doxygen.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Doxygen.gitlab-ci.yml index 2f518d667a5..e577a489c55 100644 --- a/lib/gitlab/ci/templates/Pages/Doxygen.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Doxygen.gitlab-ci.yml @@ -11,6 +11,7 @@ pages: - apk update && apk add doxygen - doxygen doxygen/Doxyfile - mv doxygen/documentation/html/ public/ + environment: production artifacts: paths: - public diff --git a/lib/gitlab/ci/templates/Pages/HTML.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/HTML.gitlab-ci.yml index 85f90984045..9f3ba6d5dd4 100644 --- a/lib/gitlab/ci/templates/Pages/HTML.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/HTML.gitlab-ci.yml @@ -6,6 +6,7 @@ # Full project: https://gitlab.com/pages/plain-html pages: stage: deploy + environment: production script: - mkdir .public - cp -r ./* .public diff --git a/lib/gitlab/ci/templates/Pages/Hexo.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Hexo.gitlab-ci.yml index a6f94a4d80e..b1617e9239c 100644 --- a/lib/gitlab/ci/templates/Pages/Hexo.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Hexo.gitlab-ci.yml @@ -11,6 +11,7 @@ pages: - npm install hexo-cli -g - test -e package.json && npm install - hexo generate + environment: production artifacts: paths: - public diff --git a/lib/gitlab/ci/templates/Pages/Hyde.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Hyde.gitlab-ci.yml index 59e55efaee0..fba4afca9ed 100644 --- a/lib/gitlab/ci/templates/Pages/Hyde.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Hyde.gitlab-ci.yml @@ -21,6 +21,7 @@ test: pages: stage: deploy + environment: production script: - pip install hyde - hyde gen -d public diff --git a/lib/gitlab/ci/templates/Pages/JBake.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/JBake.gitlab-ci.yml index 8e15570fd1a..57e3ced4dc2 100644 --- a/lib/gitlab/ci/templates/Pages/JBake.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/JBake.gitlab-ci.yml @@ -29,6 +29,7 @@ before_script: # This build job produced the output directory of your site pages: + environment: production script: - jbake . public artifacts: diff --git a/lib/gitlab/usage/metric.rb b/lib/gitlab/usage/metric.rb index cf48aa49938..30f2efc8638 100644 --- a/lib/gitlab/usage/metric.rb +++ b/lib/gitlab/usage/metric.rb @@ -46,10 +46,7 @@ module Gitlab end def instrumentation_object - @instrumentation_object ||= instrumentation_class.constantize.new( - time_frame: definition.time_frame, - options: definition.attributes[:options] - ) + @instrumentation_object ||= instrumentation_class.constantize.new(definition.attributes) end end end diff --git a/lib/gitlab/usage/metrics/instrumentations/base_metric.rb b/lib/gitlab/usage/metrics/instrumentations/base_metric.rb index 5e20766b1b4..55da2315e45 100644 --- a/lib/gitlab/usage/metrics/instrumentations/base_metric.rb +++ b/lib/gitlab/usage/metrics/instrumentations/base_metric.rb @@ -23,9 +23,9 @@ module Gitlab attr_reader :metric_available end - def initialize(time_frame:, options: {}) - @time_frame = time_frame - @options = options + def initialize(metric_definition) + @time_frame = metric_definition.fetch(:time_frame) + @options = metric_definition.fetch(:options, {}) end def instrumentation diff --git a/lib/gitlab/usage/metrics/instrumentations/count_bulk_imports_entities_metric.rb b/lib/gitlab/usage/metrics/instrumentations/count_bulk_imports_entities_metric.rb index 67dc1455b23..642b67a3b02 100644 --- a/lib/gitlab/usage/metrics/instrumentations/count_bulk_imports_entities_metric.rb +++ b/lib/gitlab/usage/metrics/instrumentations/count_bulk_imports_entities_metric.rb @@ -7,7 +7,7 @@ module Gitlab class CountBulkImportsEntitiesMetric < DatabaseMetric operation :count - def initialize(time_frame:, options: {}) + def initialize(metric_definition) super if source_type.present? && !source_type.in?(allowed_source_types) diff --git a/lib/gitlab/usage/metrics/instrumentations/count_imported_projects_metric.rb b/lib/gitlab/usage/metrics/instrumentations/count_imported_projects_metric.rb index c5498ce530f..d485e8b4f72 100644 --- a/lib/gitlab/usage/metrics/instrumentations/count_imported_projects_metric.rb +++ b/lib/gitlab/usage/metrics/instrumentations/count_imported_projects_metric.rb @@ -7,7 +7,7 @@ module Gitlab class CountImportedProjectsMetric < DatabaseMetric operation :count - def initialize(time_frame:, options: {}) + def initialize(metric_definition) super raise ArgumentError, "import_type options attribute is required" unless import_type.present? diff --git a/lib/gitlab/usage/metrics/instrumentations/generic_metric.rb b/lib/gitlab/usage/metrics/instrumentations/generic_metric.rb index 0f4b903b99c..7c646281598 100644 --- a/lib/gitlab/usage/metrics/instrumentations/generic_metric.rb +++ b/lib/gitlab/usage/metrics/instrumentations/generic_metric.rb @@ -28,9 +28,8 @@ module Gitlab end end - def initialize(time_frame: 'none', options: {}) - @time_frame = time_frame - @options = options + def initialize(metric_definition) + super(metric_definition.reverse_merge(time_frame: 'none')) end def value diff --git a/lib/gitlab/usage/metrics/instrumentations/redis_hll_metric.rb b/lib/gitlab/usage/metrics/instrumentations/redis_hll_metric.rb index bb27cca1bb9..17009f7638e 100644 --- a/lib/gitlab/usage/metrics/instrumentations/redis_hll_metric.rb +++ b/lib/gitlab/usage/metrics/instrumentations/redis_hll_metric.rb @@ -12,7 +12,7 @@ module Gitlab # events: # - g_analytics_valuestream # end - def initialize(time_frame:, options: {}) + def initialize(metric_definition) super raise ArgumentError, "options events are required" unless metric_events.present? diff --git a/lib/gitlab/usage/metrics/instrumentations/redis_metric.rb b/lib/gitlab/usage/metrics/instrumentations/redis_metric.rb index 57a417b4cf3..9cb91676b77 100644 --- a/lib/gitlab/usage/metrics/instrumentations/redis_metric.rb +++ b/lib/gitlab/usage/metrics/instrumentations/redis_metric.rb @@ -19,7 +19,7 @@ module Gitlab USAGE_PREFIX = "USAGE_" OPTIONS_PREFIX_KEY = :prefix - def initialize(time_frame:, options: {}) + def initialize(metric_definition) super raise ArgumentError, "'event' option is required" unless metric_event.present? diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7d3a8a6ec34..d0aa879290c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -27162,6 +27162,9 @@ msgstr "" msgid "Notify|%{mr_highlight}Merge request%{highlight_end} %{mr_link} %{reviewer_highlight}was unapproved by%{highlight_end} %{reviewer_avatar} %{reviewer_link}" msgstr "" +msgid "Notify|%{name} requested a new review on %{mr_link}." +msgstr "" + msgid "Notify|%{paragraph_start}Hi %{name}!%{paragraph_end} %{paragraph_start}A new public key was added to your account:%{paragraph_end} %{paragraph_start}title: %{key_title}%{paragraph_end} %{paragraph_start}If this key was added in error, you can remove it under %{removal_link}%{paragraph_end}" msgstr "" @@ -34176,6 +34179,9 @@ msgstr "" msgid "Revoked personal access token %{personal_access_token_name}!" msgstr "" +msgid "Rich text" +msgstr "" + msgid "RightSidebar|Copy email address" msgstr "" @@ -44969,12 +44975,6 @@ msgstr "" msgid "Wiki" msgstr "" -msgid "Wiki Page|Rich text" -msgstr "" - -msgid "Wiki Page|Source" -msgstr "" - msgid "Wiki page" msgstr "" diff --git a/qa/qa/page/component/content_editor.rb b/qa/qa/page/component/content_editor.rb index f7b055b6052..e9fc575ae39 100644 --- a/qa/qa/page/component/content_editor.rb +++ b/qa/qa/page/component/content_editor.rb @@ -22,8 +22,8 @@ module QA element :file_upload_field end - base.view 'app/assets/javascripts/pages/shared/wikis/components/wiki_form.vue' do - element :wiki_hidden_content + base.view 'app/assets/javascripts/vue_shared/components/markdown/markdown_editor.vue' do + element :markdown_editor_form_field end end @@ -47,7 +47,7 @@ module QA end QA::Support::Retrier.retry_on_exception do - source = find_element(:wiki_hidden_content, visible: false) + source = find_element(:markdown_editor_form_field, visible: false) source.value =~ %r{uploads/.*#{::File.basename(image_path)}} end end diff --git a/qa/qa/page/component/wiki_page_form.rb b/qa/qa/page/component/wiki_page_form.rb index 9e558844469..7a7329e6110 100644 --- a/qa/qa/page/component/wiki_page_form.rb +++ b/qa/qa/page/component/wiki_page_form.rb @@ -11,9 +11,12 @@ module QA base.view 'app/assets/javascripts/pages/shared/wikis/components/wiki_form.vue' do element :wiki_title_textbox - element :wiki_content_textarea element :wiki_message_textbox element :wiki_submit_button + end + + base.view 'app/assets/javascripts/vue_shared/components/markdown/markdown_editor.vue' do + element :markdown_editor_form_field element :editing_mode_button end @@ -27,7 +30,7 @@ module QA end def set_content(content) - fill_element(:wiki_content_textarea, content) + fill_element(:markdown_editor_form_field, content) end def set_message(message) diff --git a/spec/features/merge_request/user_manages_subscription_spec.rb b/spec/features/merge_request/user_manages_subscription_spec.rb index 9fb85957979..ec22201b88f 100644 --- a/spec/features/merge_request/user_manages_subscription_spec.rb +++ b/spec/features/merge_request/user_manages_subscription_spec.rb @@ -50,15 +50,11 @@ RSpec.describe 'User manages subscription', :js do wait_for_requests - click_button 'Toggle dropdown' - expect(page).to have_selector('.gl-toggle.is-checked') find('[data-testid="notifications-toggle"] .gl-toggle').click wait_for_requests - click_button 'Toggle dropdown' - expect(page).to have_selector('.gl-toggle:not(.is-checked)') end end diff --git a/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js b/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js index b37d2f06191..83aced63475 100644 --- a/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js +++ b/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js @@ -1,15 +1,12 @@ import { nextTick } from 'vue'; -import { GlAlert, GlButton, GlFormInput, GlFormGroup, GlSegmentedControl } from '@gitlab/ui'; +import { GlAlert, GlButton, GlFormInput, GlFormGroup } from '@gitlab/ui'; import { mount, shallowMount } from '@vue/test-utils'; import axios from 'axios'; import MockAdapter from 'axios-mock-adapter'; import { mockTracking } from 'helpers/tracking_helper'; -import { stubComponent } from 'helpers/stub_component'; import { extendedWrapper } from 'helpers/vue_test_utils_helper'; -import waitForPromises from 'helpers/wait_for_promises'; -import ContentEditor from '~/content_editor/components/content_editor.vue'; -import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue'; import WikiForm from '~/pages/shared/wikis/components/wiki_form.vue'; +import MarkdownEditor from '~/vue_shared/components/markdown/markdown_editor.vue'; import { CONTENT_EDITOR_LOADED_ACTION, SAVED_USING_CONTENT_EDITOR_ACTION, @@ -18,8 +15,6 @@ import { WIKI_FORMAT_UPDATED_ACTION, } from '~/pages/shared/wikis/constants'; -import MarkdownField from '~/vue_shared/components/markdown/field.vue'; - jest.mock('~/emoji'); describe('WikiForm', () => { @@ -30,16 +25,12 @@ describe('WikiForm', () => { const findForm = () => wrapper.find('form'); const findTitle = () => wrapper.find('#wiki_title'); const findFormat = () => wrapper.find('#wiki_format'); - const findContent = () => wrapper.find('#wiki_content'); const findMessage = () => wrapper.find('#wiki_message'); + const findMarkdownEditor = () => wrapper.findComponent(MarkdownEditor); const findSubmitButton = () => wrapper.findByTestId('wiki-submit-button'); const findCancelButton = () => wrapper.findByTestId('wiki-cancel-button'); - const findToggleEditingModeButton = () => wrapper.findByTestId('toggle-editing-mode-button'); const findTitleHelpLink = () => wrapper.findByText('Learn more.'); const findMarkdownHelpLink = () => wrapper.findByTestId('wiki-markdown-help-link'); - const findContentEditor = () => wrapper.findComponent(ContentEditor); - const findClassicEditor = () => wrapper.findComponent(MarkdownField); - const findLocalStorageSync = () => wrapper.findComponent(LocalStorageSync); const setFormat = (value) => { const format = findFormat(); @@ -103,11 +94,8 @@ describe('WikiForm', () => { }, }, stubs: { - MarkdownField, GlAlert, GlButton, - GlSegmentedControl, - LocalStorageSync: stubComponent(LocalStorageSync), GlFormInput, GlFormGroup, }, @@ -126,6 +114,22 @@ describe('WikiForm', () => { wrapper = null; }); + it('displays markdown editor', () => { + createWrapper({ persisted: true }); + + expect(findMarkdownEditor().props()).toEqual( + expect.objectContaining({ + value: pageInfoPersisted.content, + renderMarkdownPath: pageInfoPersisted.markdownPreviewPath, + markdownDocsPath: pageInfoPersisted.markdownHelpPath, + uploadsPath: pageInfoPersisted.uploadsPath, + autofocus: pageInfoPersisted.persisted, + formFieldId: 'wiki_content', + formFieldName: 'wiki[content]', + }), + ); + }); + it.each` title | persisted | message ${'my page'} | ${false} | ${'Create my page'} @@ -154,7 +158,7 @@ describe('WikiForm', () => { it('does not trim page content by default', () => { createWrapper({ persisted: true }); - expect(findContent().element.value).toBe(' My page content '); + expect(findMarkdownEditor().props().value).toBe(' My page content '); }); it.each` @@ -168,7 +172,9 @@ describe('WikiForm', () => { await setFormat(format); - expect(findClassicEditor().props('enablePreview')).toBe(enabled); + nextTick(); + + expect(findMarkdownEditor().props('enablePreview')).toBe(enabled); }); it.each` @@ -219,9 +225,7 @@ describe('WikiForm', () => { beforeEach(async () => { createWrapper({ mountFn: mount, persisted: true }); - const input = findContent(); - - await input.setValue(' Lorem ipsum dolar sit! '); + await findMarkdownEditor().vm.$emit('input', ' Lorem ipsum dolar sit! '); }); it('sets before unload warning', () => { @@ -245,7 +249,7 @@ describe('WikiForm', () => { }); it('does not trim page content', () => { - expect(findContent().element.value).toBe(' Lorem ipsum dolar sit! '); + expect(findMarkdownEditor().props().value).toBe(' Lorem ipsum dolar sit! '); }); }); }); @@ -264,7 +268,7 @@ describe('WikiForm', () => { createWrapper({ mountFn: mount }); await findTitle().setValue(title); - await findContent().setValue(content); + await findMarkdownEditor().vm.$emit('input', content); expect(findSubmitButton().props().disabled).toBe(disabledAttr); }, @@ -296,208 +300,64 @@ describe('WikiForm', () => { ); }); - describe('toggle editing mode control', () => { - beforeEach(() => { - createWrapper({ mountFn: mount }); - }); + it.each` + format | enabled | action + ${'markdown'} | ${true} | ${'enables'} + ${'rdoc'} | ${false} | ${'disables'} + ${'asciidoc'} | ${false} | ${'disables'} + ${'org'} | ${false} | ${'disables'} + `('$action content editor when format is $format', async ({ format, enabled }) => { + createWrapper({ mountFn: mount }); - it.each` - format | exists | action - ${'markdown'} | ${true} | ${'displays'} - ${'rdoc'} | ${false} | ${'hides'} - ${'asciidoc'} | ${false} | ${'hides'} - ${'org'} | ${false} | ${'hides'} - `('$action toggle editing mode button when format is $format', async ({ format, exists }) => { - await setFormat(format); - - expect(findToggleEditingModeButton().exists()).toBe(exists); - }); + setFormat(format); - describe('when content editor is not active', () => { - it('displays "Source" label in the toggle editing mode button', () => { - expect(findToggleEditingModeButton().props().checked).toBe('source'); - }); + await nextTick(); - describe('when clicking the toggle editing mode button', () => { - beforeEach(async () => { - await findToggleEditingModeButton().vm.$emit('input', 'richText'); - }); + expect(findMarkdownEditor().props().enableContentEditor).toBe(enabled); + }); - it('hides the classic editor', () => { - expect(findClassicEditor().exists()).toBe(false); - }); + describe('when markdown editor activates the content editor', () => { + beforeEach(async () => { + createWrapper({ mountFn: mount, persisted: true }); - it('shows the content editor', () => { - expect(findContentEditor().exists()).toBe(true); - }); - }); + await findMarkdownEditor().vm.$emit('contentEditor'); }); - describe('markdown editor type persistance', () => { - it('loads content editor by default if it is persisted in local storage', async () => { - expect(findClassicEditor().exists()).toBe(true); - expect(findContentEditor().exists()).toBe(false); - - // enable content editor - await findLocalStorageSync().vm.$emit('input', 'richText'); - - expect(findContentEditor().exists()).toBe(true); - expect(findClassicEditor().exists()).toBe(false); - }); + it('disables the format dropdown', () => { + expect(findFormat().element.getAttribute('disabled')).toBeDefined(); }); - describe('when content editor is active', () => { - beforeEach(() => { - createWrapper(); - findToggleEditingModeButton().vm.$emit('input', 'richText'); - }); - - it('displays "Edit Rich" label in the toggle editing mode button', () => { - expect(findToggleEditingModeButton().props().checked).toBe('richText'); - }); - - describe('when clicking the toggle editing mode button', () => { - beforeEach(async () => { - await findToggleEditingModeButton().vm.$emit('input', 'source'); - await nextTick(); - }); - - it('hides the content editor', () => { - expect(findContentEditor().exists()).toBe(false); - }); - - it('displays the classic editor', () => { - expect(findClassicEditor().exists()).toBe(true); - }); - }); - - describe('when content editor is loading', () => { - beforeEach(async () => { - findContentEditor().vm.$emit('loading'); - - await nextTick(); - }); - - it('disables toggle editing mode button', () => { - expect(findToggleEditingModeButton().attributes().disabled).toBe('true'); - }); - - describe('when content editor loads successfully', () => { - it('enables toggle editing mode button', async () => { - findContentEditor().vm.$emit('loadingSuccess'); - - await nextTick(); - - expect(findToggleEditingModeButton().attributes().disabled).not.toBeDefined(); - }); - }); - - describe('when content editor fails to load', () => { - it('enables toggle editing mode button', async () => { - findContentEditor().vm.$emit('loadingError'); - - await nextTick(); - - expect(findToggleEditingModeButton().attributes().disabled).not.toBeDefined(); - }); - }); + it('sends tracking event when editor loads', async () => { + expect(trackingSpy).toHaveBeenCalledWith(undefined, CONTENT_EDITOR_LOADED_ACTION, { + label: WIKI_CONTENT_EDITOR_TRACKING_LABEL, }); }); - }); - describe('wiki content editor', () => { - describe('clicking "Edit rich text": editor fails to load', () => { - beforeEach(async () => { - createWrapper({ mountFn: mount }); - mock.onPost(/preview-markdown/).reply(400); + describe('when triggering form submit', () => { + const updatedMarkdown = 'hello **world**'; - await findToggleEditingModeButton().vm.$emit('input', 'richText'); - - // try waiting for content editor to load (but it will never actually load) - await waitForPromises(); - }); - - it('disables the submit button', () => { - expect(findSubmitButton().props('disabled')).toBe(true); - }); - - describe('toggling editing modes to the classic editor', () => { - beforeEach(() => { - return findToggleEditingModeButton().vm.$emit('input', 'source'); - }); - - it('switches to classic editor', () => { - expect(findContentEditor().exists()).toBe(false); - expect(findClassicEditor().exists()).toBe(true); - }); - }); - }); - - describe('clicking "Edit rich text": editor loads successfully', () => { beforeEach(async () => { - createWrapper({ persisted: true, mountFn: mount }); - - mock.onPost(/preview-markdown/).reply(200, { body: '<p>hello <strong>world</strong></p>' }); - - await findToggleEditingModeButton().vm.$emit('input', 'richText'); - await waitForPromises(); + findMarkdownEditor().vm.$emit('input', updatedMarkdown); + await triggerFormSubmit(); }); - it('shows the rich text editor when loading finishes', async () => { - expect(findContentEditor().exists()).toBe(true); + it('unsets before unload warning on form submit', async () => { + const e = dispatchBeforeUnload(); + expect(e.preventDefault).not.toHaveBeenCalled(); }); - it('sends tracking event when editor loads', async () => { - expect(trackingSpy).toHaveBeenCalledWith(undefined, CONTENT_EDITOR_LOADED_ACTION, { + it('triggers tracking events on form submit', async () => { + expect(trackingSpy).toHaveBeenCalledWith(undefined, SAVED_USING_CONTENT_EDITOR_ACTION, { label: WIKI_CONTENT_EDITOR_TRACKING_LABEL, }); - }); - - it('disables the format dropdown', () => { - expect(findFormat().element.getAttribute('disabled')).toBeDefined(); - }); - - describe('when wiki content is updated', () => { - const updatedMarkdown = 'hello **world**'; - - beforeEach(() => { - findContentEditor().vm.$emit('change', { - empty: false, - changed: true, - markdown: updatedMarkdown, - }); - }); - - it('sets before unload warning', () => { - const e = dispatchBeforeUnload(); - expect(e.preventDefault).toHaveBeenCalledTimes(1); - }); - - it('unsets before unload warning on form submit', async () => { - await triggerFormSubmit(); - - const e = dispatchBeforeUnload(); - expect(e.preventDefault).not.toHaveBeenCalled(); - }); - it('triggers tracking events on form submit', async () => { - await triggerFormSubmit(); - expect(trackingSpy).toHaveBeenCalledWith(undefined, SAVED_USING_CONTENT_EDITOR_ACTION, { - label: WIKI_CONTENT_EDITOR_TRACKING_LABEL, - }); - - expect(trackingSpy).toHaveBeenCalledWith(undefined, WIKI_FORMAT_UPDATED_ACTION, { - label: WIKI_FORMAT_LABEL, - extra: { - value: findFormat().element.value, - old_format: pageInfoPersisted.format, - project_path: pageInfoPersisted.path, - }, - }); - }); - - it('sets content field to the content editor updated markdown', async () => { - expect(findContent().element.value).toBe(updatedMarkdown); + expect(trackingSpy).toHaveBeenCalledWith(undefined, WIKI_FORMAT_UPDATED_ACTION, { + label: WIKI_FORMAT_LABEL, + extra: { + value: findFormat().element.value, + old_format: pageInfoPersisted.format, + project_path: pageInfoPersisted.path, + }, }); }); }); diff --git a/spec/frontend/vue_shared/components/markdown/markdown_editor_spec.js b/spec/frontend/vue_shared/components/markdown/markdown_editor_spec.js new file mode 100644 index 00000000000..da3777de1d0 --- /dev/null +++ b/spec/frontend/vue_shared/components/markdown/markdown_editor_spec.js @@ -0,0 +1,226 @@ +import { GlSegmentedControl } from '@gitlab/ui'; +import axios from 'axios'; +import MockAdapter from 'axios-mock-adapter'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import { EDITING_MODE_MARKDOWN_FIELD, EDITING_MODE_CONTENT_EDITOR } from '~/vue_shared/constants'; +import MarkdownEditor from '~/vue_shared/components/markdown/markdown_editor.vue'; +import ContentEditor from '~/content_editor/components/content_editor.vue'; +import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue'; +import MarkdownField from '~/vue_shared/components/markdown/field.vue'; + +jest.mock('~/emoji'); + +describe('vue_shared/component/markdown/markdown_editor', () => { + let wrapper; + const value = 'test markdown'; + const renderMarkdownPath = '/api/markdown'; + const markdownDocsPath = '/help/markdown'; + const uploadsPath = '/uploads'; + const enableAutocomplete = true; + const enablePreview = false; + const formFieldId = 'markdown_field'; + const formFieldName = 'form[markdown_field]'; + const formFieldPlaceholder = 'Write some markdown'; + const formFieldAriaLabel = 'Edit your content'; + let mock; + + const buildWrapper = (propsData = {}) => { + wrapper = mountExtended(MarkdownEditor, { + propsData: { + value, + renderMarkdownPath, + markdownDocsPath, + uploadsPath, + enableAutocomplete, + enablePreview, + formFieldId, + formFieldName, + formFieldPlaceholder, + formFieldAriaLabel, + ...propsData, + }, + }); + }; + const findSegmentedControl = () => wrapper.findComponent(GlSegmentedControl); + const findMarkdownField = () => wrapper.findComponent(MarkdownField); + const findTextarea = () => wrapper.find('textarea'); + const findLocalStorageSync = () => wrapper.findComponent(LocalStorageSync); + const findContentEditor = () => wrapper.findComponent(ContentEditor); + + beforeEach(() => { + mock = new MockAdapter(axios); + }); + + afterEach(() => { + wrapper.destroy(); + mock.restore(); + }); + + it('displays markdown field by default', () => { + buildWrapper(); + + expect(findMarkdownField().props()).toEqual( + expect.objectContaining({ + markdownPreviewPath: renderMarkdownPath, + canAttachFile: true, + enableAutocomplete, + textareaValue: value, + markdownDocsPath, + uploadsPath, + enablePreview, + }), + ); + }); + + it('renders markdown field textarea', () => { + buildWrapper(); + + expect(findTextarea().attributes()).toEqual( + expect.objectContaining({ + id: formFieldId, + name: formFieldName, + placeholder: formFieldPlaceholder, + 'aria-label': formFieldAriaLabel, + }), + ); + + expect(findTextarea().element.value).toBe(value); + }); + + it('renders switch segmented control', () => { + buildWrapper(); + + expect(findSegmentedControl().props()).toEqual({ + checked: EDITING_MODE_MARKDOWN_FIELD, + options: [ + { + text: expect.any(String), + value: EDITING_MODE_MARKDOWN_FIELD, + }, + { + text: expect.any(String), + value: EDITING_MODE_CONTENT_EDITOR, + }, + ], + }); + }); + + describe.each` + editingMode + ${EDITING_MODE_CONTENT_EDITOR} + ${EDITING_MODE_MARKDOWN_FIELD} + `('when segmented control emits change event with $editingMode value', ({ editingMode }) => { + it(`emits ${editingMode} event`, () => { + buildWrapper(); + + findSegmentedControl().vm.$emit('change', editingMode); + + expect(wrapper.emitted(editingMode)).toHaveLength(1); + }); + }); + + describe(`when editingMode is ${EDITING_MODE_MARKDOWN_FIELD}`, () => { + it('emits input event when markdown field textarea changes', async () => { + buildWrapper(); + const newValue = 'new value'; + + await findTextarea().setValue(newValue); + + expect(wrapper.emitted('input')).toEqual([[newValue]]); + }); + + describe(`when segmented control triggers input event with ${EDITING_MODE_CONTENT_EDITOR} value`, () => { + beforeEach(() => { + buildWrapper(); + findSegmentedControl().vm.$emit('input', EDITING_MODE_CONTENT_EDITOR); + }); + + it('displays the content editor', () => { + expect(findContentEditor().props()).toEqual( + expect.objectContaining({ + renderMarkdown: expect.any(Function), + uploadsPath, + markdown: value, + }), + ); + }); + + it('adds hidden field with current markdown', () => { + const hiddenField = wrapper.find(`#${formFieldId}`); + + expect(hiddenField.attributes()).toEqual( + expect.objectContaining({ + id: formFieldId, + name: formFieldName, + }), + ); + expect(hiddenField.element.value).toBe(value); + }); + + it('hides the markdown field', () => { + expect(findMarkdownField().exists()).toBe(false); + }); + + it('updates localStorage value', () => { + expect(findLocalStorageSync().props().value).toBe(EDITING_MODE_CONTENT_EDITOR); + }); + }); + }); + + describe(`when editingMode is ${EDITING_MODE_CONTENT_EDITOR}`, () => { + beforeEach(() => { + buildWrapper(); + findSegmentedControl().vm.$emit('input', EDITING_MODE_CONTENT_EDITOR); + }); + + it('emits input event when content editor emits change event', async () => { + const newValue = 'new value'; + + await findContentEditor().vm.$emit('change', { markdown: newValue }); + + expect(wrapper.emitted('input')).toEqual([[newValue]]); + }); + + describe(`when segmented control triggers input event with ${EDITING_MODE_MARKDOWN_FIELD} value`, () => { + beforeEach(() => { + findSegmentedControl().vm.$emit('input', EDITING_MODE_MARKDOWN_FIELD); + }); + + it('hides the content editor', () => { + expect(findContentEditor().exists()).toBe(false); + }); + + it('shows the markdown field', () => { + expect(findMarkdownField().exists()).toBe(true); + }); + + it('updates localStorage value', () => { + expect(findLocalStorageSync().props().value).toBe(EDITING_MODE_MARKDOWN_FIELD); + }); + }); + + describe('when content editor emits loading event', () => { + beforeEach(() => { + findContentEditor().vm.$emit('loading'); + }); + + it('disables switch editing mode control', () => { + // This is the only way that I found to check the segmented control is disabled + expect(findSegmentedControl().find('input[disabled]').exists()).toBe(true); + }); + + describe.each` + event + ${'loadingSuccess'} + ${'loadingError'} + `('when content editor emits $event event', ({ event }) => { + beforeEach(() => { + findContentEditor().vm.$emit(event); + }); + it('enables the switch editing mode control', () => { + expect(findSegmentedControl().find('input[disabled]').exists()).toBe(false); + }); + }); + }); + }); +}); diff --git a/spec/lib/gitlab/background_migration/backfill_internal_on_notes_spec.rb b/spec/lib/gitlab/background_migration/backfill_internal_on_notes_spec.rb new file mode 100644 index 00000000000..40a4758ba5f --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_internal_on_notes_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BackfillInternalOnNotes, :migration, schema: 20220920124709 do + let(:notes_table) { table(:notes) } + + let!(:confidential_note) { notes_table.create!(id: 1, confidential: true, internal: false) } + let!(:non_confidential_note) { notes_table.create!(id: 2, confidential: false, internal: false) } + + describe '#perform' do + subject(:perform) do + described_class.new( + start_id: 1, + end_id: 2, + batch_table: :notes, + batch_column: :id, + sub_batch_size: 1, + pause_ms: 0, + connection: ApplicationRecord.connection + ).perform + end + + it 'backfills internal column on notes when confidential' do + expect { perform } + .to change { confidential_note.reload.internal }.from(false).to(true) + .and not_change { non_confidential_note.reload.internal } + end + end +end diff --git a/spec/lib/gitlab/ci/build/rules/rule/clause/changes_spec.rb b/spec/lib/gitlab/ci/build/rules/rule/clause/changes_spec.rb index 234ba68d627..a22aa30304b 100644 --- a/spec/lib/gitlab/ci/build/rules/rule/clause/changes_spec.rb +++ b/spec/lib/gitlab/ci/build/rules/rule/clause/changes_spec.rb @@ -122,19 +122,17 @@ RSpec.describe Gitlab::Ci::Build::Rules::Rule::Clause::Changes do context 'when compare_to is branch or tag' do using RSpec::Parameterized::TableSyntax - where(:pipeline_ref, :compare_to, :paths, :ff, :result) do - 'feature_1' | 'master' | ['file1.txt'] | true | true - 'feature_1' | 'master' | ['README.md'] | true | false - 'feature_1' | 'master' | ['xyz.md'] | true | false - 'feature_2' | 'master' | ['file1.txt'] | true | true - 'feature_2' | 'master' | ['file2.txt'] | true | true - 'feature_2' | 'feature_1' | ['file1.txt'] | true | false - 'feature_2' | 'feature_1' | ['file1.txt'] | false | true - 'feature_2' | 'feature_1' | ['file2.txt'] | true | true - 'feature_1' | 'tag_1' | ['file1.txt'] | true | false - 'feature_1' | 'tag_1' | ['file1.txt'] | false | true - 'feature_1' | 'tag_1' | ['file2.txt'] | true | true - 'feature_2' | 'tag_1' | ['file2.txt'] | true | true + where(:pipeline_ref, :compare_to, :paths, :result) do + 'feature_1' | 'master' | ['file1.txt'] | true + 'feature_1' | 'master' | ['README.md'] | false + 'feature_1' | 'master' | ['xyz.md'] | false + 'feature_2' | 'master' | ['file1.txt'] | true + 'feature_2' | 'master' | ['file2.txt'] | true + 'feature_2' | 'feature_1' | ['file1.txt'] | false + 'feature_2' | 'feature_1' | ['file2.txt'] | true + 'feature_1' | 'tag_1' | ['file1.txt'] | false + 'feature_1' | 'tag_1' | ['file2.txt'] | true + 'feature_2' | 'tag_1' | ['file2.txt'] | true end with_them do @@ -144,10 +142,6 @@ RSpec.describe Gitlab::Ci::Build::Rules::Rule::Clause::Changes do build(:ci_pipeline, project: project, ref: pipeline_ref, sha: project.commit(pipeline_ref).sha) end - before do - stub_feature_flags(ci_rules_changes_compare: ff) - end - it { is_expected.to eq(result) } end end @@ -174,14 +168,6 @@ RSpec.describe Gitlab::Ci::Build::Rules::Rule::Clause::Changes do ::Gitlab::Ci::Build::Rules::Rule::Clause::ParseError, 'rules:changes:compare_to is not a valid ref' ) end - - context 'when the FF ci_rules_changes_compare is disabled' do - before do - stub_feature_flags(ci_rules_changes_compare: false) - end - - it { is_expected.to be_truthy } - end end end end diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/redis_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/redis_metric_spec.rb index 8f9c3573897..9ff3ad1dfc0 100644 --- a/spec/lib/gitlab/usage/metrics/instrumentations/redis_metric_spec.rb +++ b/spec/lib/gitlab/usage/metrics/instrumentations/redis_metric_spec.rb @@ -11,14 +11,21 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::RedisMetric, :clean_git let(:expected_value) { 4 } - it_behaves_like 'a correct instrumented metric value', { options: { event: 'pushes', prefix: 'source_code' } } + it_behaves_like 'a correct instrumented metric value', { + options: { event: 'pushes', prefix: 'source_code' }, + time_frame: 'all' + } it 'raises an exception if event option is not present' do - expect { described_class.new(prefix: 'source_code') }.to raise_error(ArgumentError) + expect do + described_class.new(options: { prefix: 'source_code' }, time_frame: 'all') + end.to raise_error(ArgumentError, /'event' option is required/) end it 'raises an exception if prefix option is not present' do - expect { described_class.new(event: 'pushes') }.to raise_error(ArgumentError) + expect do + described_class.new(options: { event: 'pushes' }, time_frame: 'all') + end.to raise_error(ArgumentError, /'prefix' option is required/) end describe 'children classes' do @@ -55,7 +62,8 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::RedisMetric, :clean_git end it_behaves_like 'a correct instrumented metric value', { - options: { event: 'merge_requests_count', prefix: 'web_ide', include_usage_prefix: false } + options: { event: 'merge_requests_count', prefix: 'web_ide', include_usage_prefix: false }, + time_frame: 'all' } end diff --git a/spec/migrations/20220920124709_backfill_internal_on_notes_spec.rb b/spec/migrations/20220920124709_backfill_internal_on_notes_spec.rb new file mode 100644 index 00000000000..f4ac6e6fc8e --- /dev/null +++ b/spec/migrations/20220920124709_backfill_internal_on_notes_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe BackfillInternalOnNotes, :migration do + let(:migration) { described_class::MIGRATION } + + describe '#up' do + it 'schedules background jobs for each batch of issues' do + migrate! + + expect(migration).to have_scheduled_batched_migration( + table_name: :notes, + column_name: :id, + interval: described_class::DELAY_INTERVAL, + batch_size: described_class::BATCH_SIZE, + sub_batch_size: described_class::SUB_BATCH_SIZE + ) + end + end + + describe '#down' do + it 'deletes all batched migration records' do + migrate! + schema_migrate_down! + + expect(migration).not_to have_scheduled_batched_migration + end + end +end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 2e3d5302a76..79f2565e173 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -5516,4 +5516,34 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end end + + describe '#notes=' do + context 'when notes already exist' do + it 'does not create duplicate notes', :aggregate_failures do + time = Time.zone.now + pipeline = create(:ci_pipeline, user: user, project: project) + note = Note.new( + note: 'note', + noteable_type: 'Commit', + noteable_id: pipeline.id, + commit_id: pipeline.id, + author_id: user.id, + project_id: pipeline.project_id, + created_at: time + ) + another_note = note.dup.tap { |note| note.note = 'another note' } + + expect(project.notes.for_commit_id(pipeline.sha).count).to eq(0) + + pipeline.notes = [note] + + expect(project.notes.for_commit_id(pipeline.sha).count).to eq(1) + + pipeline.notes = [note, note, another_note] + + expect(project.notes.for_commit_id(pipeline.sha).count).to eq(2) + expect(project.notes.for_commit_id(pipeline.sha).pluck(:note)).to contain_exactly(note.note, another_note.note) + end + end + end end diff --git a/spec/services/ci/create_pipeline_service/rules_spec.rb b/spec/services/ci/create_pipeline_service/rules_spec.rb index fc57ca66d3a..87e9b4400e7 100644 --- a/spec/services/ci/create_pipeline_service/rules_spec.rb +++ b/spec/services/ci/create_pipeline_service/rules_spec.rb @@ -544,16 +544,6 @@ RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectnes 'Failed to parse rule for job1: rules:changes:compare_to is not a valid ref' ]) end - - context 'when the FF ci_rules_changes_compare is not enabled' do - before do - stub_feature_flags(ci_rules_changes_compare: false) - end - - it 'ignores compare_to and changes is always true' do - expect(build_names).to contain_exactly('job1', 'job2') - end - end end context 'when the compare_to ref exists' do @@ -563,16 +553,6 @@ RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectnes it 'creates job1 and job2' do expect(build_names).to contain_exactly('job1', 'job2') end - - context 'when the FF ci_rules_changes_compare is not enabled' do - before do - stub_feature_flags(ci_rules_changes_compare: false) - end - - it 'ignores compare_to and changes is always true' do - expect(build_names).to contain_exactly('job1', 'job2') - end - end end context 'when the rule does not match' do @@ -581,16 +561,6 @@ RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectnes it 'does not create job1' do expect(build_names).to contain_exactly('job2') end - - context 'when the FF ci_rules_changes_compare is not enabled' do - before do - stub_feature_flags(ci_rules_changes_compare: false) - end - - it 'ignores compare_to and changes is always true' do - expect(build_names).to contain_exactly('job1', 'job2') - end - end end end end @@ -616,17 +586,6 @@ RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectnes expect(pipeline).to be_created_successfully expect(build_names).to contain_exactly('job1') end - - context 'when the FF ci_rules_changes_compare is not enabled' do - before do - stub_feature_flags(ci_rules_changes_compare: false) - end - - it 'ignores compare_to and changes is always true' do - expect(pipeline).to be_created_successfully - expect(build_names).to contain_exactly('job1') - end - end end context 'when the rule does not match' do diff --git a/workhorse/internal/api/api.go b/workhorse/internal/api/api.go index aa6d7cf1bc7..6a6a51b27bb 100644 --- a/workhorse/internal/api/api.go +++ b/workhorse/internal/api/api.go @@ -19,7 +19,6 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/config" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" "gitlab.com/gitlab-org/gitlab/workhorse/internal/log" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/secret" ) diff --git a/workhorse/internal/dependencyproxy/dependencyproxy.go b/workhorse/internal/dependencyproxy/dependencyproxy.go index 90f3042a342..6651b5aee84 100644 --- a/workhorse/internal/dependencyproxy/dependencyproxy.go +++ b/workhorse/internal/dependencyproxy/dependencyproxy.go @@ -9,12 +9,12 @@ import ( "gitlab.com/gitlab-org/labkit/log" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/httptransport" "gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/transport" ) var httpClient = &http.Client{ - Transport: httptransport.New(), + Transport: transport.NewRestrictedTransport(), } type Injector struct { diff --git a/workhorse/internal/helper/httptransport/http_transport.go b/workhorse/internal/helper/httptransport/http_transport.go deleted file mode 100644 index c7c3c5283f5..00000000000 --- a/workhorse/internal/helper/httptransport/http_transport.go +++ /dev/null @@ -1,37 +0,0 @@ -package httptransport - -import ( - "net/http" - "time" - - "gitlab.com/gitlab-org/labkit/correlation" - "gitlab.com/gitlab-org/labkit/tracing" -) - -type Option func(*http.Transport) - -// Defines a http.Transport with values -// that are more restrictive than for http.DefaultTransport, -// they define shorter TLS Handshake, and more aggressive connection closing -// to prevent the connection hanging and reduce FD usage -func New(options ...Option) http.RoundTripper { - t := http.DefaultTransport.(*http.Transport).Clone() - - // To avoid keep around TCP connections to http servers we're done with - t.MaxIdleConns = 2 - - // A stricter timeout for fetching from external sources that can be slow - t.ResponseHeaderTimeout = 30 * time.Second - - for _, option := range options { - option(t) - } - - return tracing.NewRoundTripper(correlation.NewInstrumentedRoundTripper(t)) -} - -func WithDisabledCompression() Option { - return func(t *http.Transport) { - t.DisableCompression = true - } -} diff --git a/workhorse/internal/imageresizer/image_resizer.go b/workhorse/internal/imageresizer/image_resizer.go index 8c3271b6f11..092369cd2af 100644 --- a/workhorse/internal/imageresizer/image_resizer.go +++ b/workhorse/internal/imageresizer/image_resizer.go @@ -21,9 +21,9 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/config" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/httptransport" "gitlab.com/gitlab-org/gitlab/workhorse/internal/log" "gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/transport" ) type Resizer struct { @@ -69,7 +69,7 @@ const ( var envInjector = tracing.NewEnvInjector() var httpClient = &http.Client{ - Transport: httptransport.New(), + Transport: transport.NewRestrictedTransport(), } const ( diff --git a/workhorse/internal/sendurl/sendurl.go b/workhorse/internal/sendurl/sendurl.go index 205ec8a0e9f..8e679c6b475 100644 --- a/workhorse/internal/sendurl/sendurl.go +++ b/workhorse/internal/sendurl/sendurl.go @@ -11,9 +11,9 @@ import ( "gitlab.com/gitlab-org/labkit/mask" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/httptransport" "gitlab.com/gitlab-org/gitlab/workhorse/internal/log" "gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/transport" ) type entry struct{ senddata.Prefix } @@ -44,7 +44,7 @@ var preserveHeaderKeys = map[string]bool{ "Pragma": true, // Support for HTTP 1.0 proxies } -var httpTransport = httptransport.New() +var httpTransport = transport.NewRestrictedTransport() var httpClient = &http.Client{ Transport: httpTransport, diff --git a/workhorse/internal/transport/transport.go b/workhorse/internal/transport/transport.go new file mode 100644 index 00000000000..f19d332a28a --- /dev/null +++ b/workhorse/internal/transport/transport.go @@ -0,0 +1,58 @@ +package transport + +import ( + "net/http" + "time" + + "gitlab.com/gitlab-org/labkit/correlation" + "gitlab.com/gitlab-org/labkit/tracing" + + "gitlab.com/gitlab-org/gitlab/workhorse/internal/version" +) + +// Creates a new default transport that has Workhorse's User-Agent header set. +func NewDefaultTransport() http.RoundTripper { + return &DefaultTransport{Next: http.DefaultTransport} +} + +// Defines a http.Transport with values that are more restrictive than for +// http.DefaultTransport, they define shorter TLS Handshake, and more +// aggressive connection closing to prevent the connection hanging and reduce +// FD usage +func NewRestrictedTransport(options ...Option) http.RoundTripper { + return &DefaultTransport{Next: newRestrictedTransport(options...)} +} + +type DefaultTransport struct { + Next http.RoundTripper +} + +func (t DefaultTransport) RoundTrip(req *http.Request) (*http.Response, error) { + req.Header.Set("User-Agent", version.GetUserAgent()) + + return t.Next.RoundTrip(req) +} + +type Option func(*http.Transport) + +func WithDisabledCompression() Option { + return func(t *http.Transport) { + t.DisableCompression = true + } +} + +func newRestrictedTransport(options ...Option) http.RoundTripper { + t := http.DefaultTransport.(*http.Transport).Clone() + + // To avoid keep around TCP connections to http servers we're done with + t.MaxIdleConns = 2 + + // A stricter timeout for fetching from external sources that can be slow + t.ResponseHeaderTimeout = 30 * time.Second + + for _, option := range options { + option(t) + } + + return tracing.NewRoundTripper(correlation.NewInstrumentedRoundTripper(t)) +} diff --git a/workhorse/internal/upload/destination/objectstore/object.go b/workhorse/internal/upload/destination/objectstore/object.go index 36ffa0eb12e..1086332312c 100644 --- a/workhorse/internal/upload/destination/objectstore/object.go +++ b/workhorse/internal/upload/destination/objectstore/object.go @@ -8,11 +8,11 @@ import ( "gitlab.com/gitlab-org/labkit/mask" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/httptransport" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/transport" ) var httpClient = &http.Client{ - Transport: httptransport.New(), + Transport: transport.NewRestrictedTransport(), } // Object represents an object on a S3 compatible Object Store service. diff --git a/workhorse/internal/version/version.go b/workhorse/internal/version/version.go new file mode 100644 index 00000000000..790edf8ffca --- /dev/null +++ b/workhorse/internal/version/version.go @@ -0,0 +1,20 @@ +package version + +import "fmt" + +var version = "unknown" +var build = "unknown" +var schema = "gitlab-workhorse (%s)-(%s)" + +func SetVersion(v, b string) { + version = v + build = b +} + +func GetUserAgent() string { + return GetApplicationVersion() +} + +func GetApplicationVersion() string { + return fmt.Sprintf(schema, version, build) +} diff --git a/workhorse/internal/version/version_test.go b/workhorse/internal/version/version_test.go new file mode 100644 index 00000000000..9d0581e093b --- /dev/null +++ b/workhorse/internal/version/version_test.go @@ -0,0 +1,19 @@ +package version + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestVersion(t *testing.T) { + require.Equal(t, GetApplicationVersion(), "gitlab-workhorse (unknown)-(unknown)") + + SetVersion("15.3", "123.123") + + require.Equal(t, GetApplicationVersion(), "gitlab-workhorse (15.3)-(123.123)") + + SetVersion("", "123.123") + + require.Equal(t, GetApplicationVersion(), "gitlab-workhorse ()-(123.123)") +} diff --git a/workhorse/internal/zipartifacts/open_archive.go b/workhorse/internal/zipartifacts/open_archive.go index 881ef915d75..d477725a39f 100644 --- a/workhorse/internal/zipartifacts/open_archive.go +++ b/workhorse/internal/zipartifacts/open_archive.go @@ -8,16 +8,16 @@ import ( "os" "strings" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/httptransport" "gitlab.com/gitlab-org/gitlab/workhorse/internal/httprs" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/transport" zip "gitlab.com/gitlab-org/golang-archive-zip" "gitlab.com/gitlab-org/labkit/mask" ) var httpClient = &http.Client{ - Transport: httptransport.New( - httptransport.WithDisabledCompression(), // To avoid bugs when serving compressed files from object storage + Transport: transport.NewRestrictedTransport( + transport.WithDisabledCompression(), // To avoid bugs when serving compressed files from object storage ), } diff --git a/workhorse/main.go b/workhorse/main.go index b0f9760b0d5..ca9b86de528 100644 --- a/workhorse/main.go +++ b/workhorse/main.go @@ -23,6 +23,7 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/redis" "gitlab.com/gitlab-org/gitlab/workhorse/internal/secret" "gitlab.com/gitlab-org/gitlab/workhorse/internal/upstream" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/version" ) // Version is the current version of GitLab Workhorse @@ -55,8 +56,10 @@ func main() { os.Exit(2) } + version.SetVersion(Version, BuildTime) + if boot.printVersion { - fmt.Printf("gitlab-workhorse %s-%s\n", Version, BuildTime) + fmt.Println(version.GetApplicationVersion()) os.Exit(0) } |