diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-11-28 15:06:57 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-11-28 15:06:57 +0000 |
commit | 7cdd70dcec27402e89e65451b4b1feb75b5eb267 (patch) | |
tree | 1691c8e1afd469fa426ecf5bc127de8df16d4855 /app | |
parent | 79348faced5e7e62103ad27f6a6594dfdca463e2 (diff) | |
download | gitlab-ce-7cdd70dcec27402e89e65451b4b1feb75b5eb267.tar.gz |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'app')
27 files changed, 313 insertions, 215 deletions
diff --git a/app/assets/javascripts/monitoring/components/charts/anomaly.vue b/app/assets/javascripts/monitoring/components/charts/anomaly.vue index b8767f48c5f..1df7ca37a98 100644 --- a/app/assets/javascripts/monitoring/components/charts/anomaly.vue +++ b/app/assets/javascripts/monitoring/components/charts/anomaly.vue @@ -29,7 +29,7 @@ const AREA_COLOR_RGBA = `rgba(${hexToRgb(AREA_COLOR).join(',')},${AREA_OPACITY}) * time series chart, the boundary band shows the normal * range of values the metric should take. * - * This component accepts 3 queries, which contain the + * This component accepts 3 metrics, which contain the * "metric", "upper" limit and "lower" limit. * * The upper and lower series are "stacked areas" visually @@ -62,10 +62,10 @@ export default { }, computed: { series() { - return this.graphData.queries.map(query => { - const values = query.result[0] ? query.result[0].values : []; + return this.graphData.metrics.map(metric => { + const values = metric.result && metric.result[0] ? metric.result[0].values : []; return { - label: query.label, + label: metric.label, // NaN values may disrupt avg., max. & min. calculations in the legend, filter them out data: values.filter(([, value]) => !Number.isNaN(value)), }; @@ -83,7 +83,7 @@ export default { return min < 0 ? -min : 0; }, metricData() { - const originalMetricQuery = this.graphData.queries[0]; + const originalMetricQuery = this.graphData.metrics[0]; const metricQuery = { ...originalMetricQuery }; metricQuery.result[0].values = metricQuery.result[0].values.map(([x, y]) => [ @@ -93,7 +93,7 @@ export default { return { ...this.graphData, type: 'line-chart', - queries: [metricQuery], + metrics: [metricQuery], }; }, metricSeriesConfig() { diff --git a/app/assets/javascripts/monitoring/components/charts/column.vue b/app/assets/javascripts/monitoring/components/charts/column.vue index ee6aaeb7dde..eb407ad1d7f 100644 --- a/app/assets/javascripts/monitoring/components/charts/column.vue +++ b/app/assets/javascripts/monitoring/components/charts/column.vue @@ -32,8 +32,8 @@ export default { }, computed: { chartData() { - const queryData = this.graphData.queries.reduce((acc, query) => { - const series = makeDataSeries(query.result, { + const queryData = this.graphData.metrics.reduce((acc, query) => { + const series = makeDataSeries(query.result || [], { name: this.formatLegendLabel(query), }); @@ -45,13 +45,13 @@ export default { }; }, xAxisTitle() { - return this.graphData.queries[0].result[0].x_label !== undefined - ? this.graphData.queries[0].result[0].x_label + return this.graphData.metrics[0].result[0].x_label !== undefined + ? this.graphData.metrics[0].result[0].x_label : ''; }, yAxisTitle() { - return this.graphData.queries[0].result[0].y_label !== undefined - ? this.graphData.queries[0].result[0].y_label + return this.graphData.metrics[0].result[0].y_label !== undefined + ? this.graphData.metrics[0].result[0].y_label : ''; }, xAxisType() { diff --git a/app/assets/javascripts/monitoring/components/charts/heatmap.vue b/app/assets/javascripts/monitoring/components/charts/heatmap.vue index b8158247e49..6ab5aaeba1d 100644 --- a/app/assets/javascripts/monitoring/components/charts/heatmap.vue +++ b/app/assets/javascripts/monitoring/components/charts/heatmap.vue @@ -24,7 +24,7 @@ export default { }, computed: { chartData() { - return this.queries.result.reduce( + return this.metrics.result.reduce( (acc, result, i) => [...acc, ...result.values.map((value, j) => [i, j, value[1]])], [], ); @@ -36,7 +36,7 @@ export default { return this.graphData.y_label || ''; }, xAxisLabels() { - return this.queries.result.map(res => Object.values(res.metric)[0]); + return this.metrics.result.map(res => Object.values(res.metric)[0]); }, yAxisLabels() { return this.result.values.map(val => { @@ -46,10 +46,10 @@ export default { }); }, result() { - return this.queries.result[0]; + return this.metrics.result[0]; }, - queries() { - return this.graphData.queries[0]; + metrics() { + return this.graphData.metrics[0]; }, }, }; diff --git a/app/assets/javascripts/monitoring/components/charts/single_stat.vue b/app/assets/javascripts/monitoring/components/charts/single_stat.vue index 076682820e6..e75ddb05808 100644 --- a/app/assets/javascripts/monitoring/components/charts/single_stat.vue +++ b/app/assets/javascripts/monitoring/components/charts/single_stat.vue @@ -17,7 +17,7 @@ export default { }, computed: { queryInfo() { - return this.graphData.queries[0]; + return this.graphData.metrics[0]; }, engineeringNotation() { return `${roundOffFloat(this.queryInfo.result[0].value[1], 1)}${this.queryInfo.unit}`; diff --git a/app/assets/javascripts/monitoring/components/charts/time_series.vue b/app/assets/javascripts/monitoring/components/charts/time_series.vue index 067a727fe16..ed01d0ee553 100644 --- a/app/assets/javascripts/monitoring/components/charts/time_series.vue +++ b/app/assets/javascripts/monitoring/components/charts/time_series.vue @@ -105,7 +105,7 @@ export default { // Transforms & supplements query data to render appropriate labels & styles // Input: [{ queryAttributes1 }, { queryAttributes2 }] // Output: [{ seriesAttributes1 }, { seriesAttributes2 }] - return this.graphData.queries.reduce((acc, query) => { + return this.graphData.metrics.reduce((acc, query) => { const { appearance } = query; const lineType = appearance && appearance.line && appearance.line.type @@ -121,7 +121,7 @@ export default { ? appearance.area.opacity : undefined, }; - const series = makeDataSeries(query.result, { + const series = makeDataSeries(query.result || [], { name: this.formatLegendLabel(query), lineStyle: { type: lineType, diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index 91aa5cf0e7b..823ea9e1641 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -250,14 +250,9 @@ export default { 'setEndpoints', 'setPanelGroupMetrics', ]), - chartsWithData(charts) { - return charts.filter(chart => - chart.metrics.some(metric => this.metricsWithData.includes(metric.metric_id)), - ); - }, - updateMetrics(key, metrics) { + updateMetrics(key, panels) { this.setPanelGroupMetrics({ - metrics, + panels, key, }); }, @@ -292,8 +287,13 @@ export default { submitCustomMetricsForm() { this.$refs.customMetricsForm.submit(); }, + chartsWithData(panels) { + return panels.filter(panel => + panel.metrics.some(metric => this.metricsWithData.includes(metric.metric_id)), + ); + }, groupHasData(group) { - return this.chartsWithData(group.metrics).length > 0; + return this.chartsWithData(group.panels).length > 0; }, onDateTimePickerApply(timeWindowUrlParams) { return redirectTo(mergeUrlParams(timeWindowUrlParams, window.location.href)); @@ -453,14 +453,14 @@ export default { :collapse-group="groupHasData(groupData)" > <vue-draggable - :value="groupData.metrics" + :value="groupData.panels" group="metrics-dashboard" :component-data="{ attrs: { class: 'row mx-0 w-100' } }" :disabled="!isRearrangingPanels" @input="updateMetrics(groupData.key, $event)" > <div - v-for="(graphData, graphIndex) in groupData.metrics" + v-for="(graphData, graphIndex) in groupData.panels" :key="`panel-type-${graphIndex}`" class="col-12 col-lg-6 px-2 mb-2 draggable" :class="{ 'draggable-enabled': isRearrangingPanels }" @@ -469,7 +469,7 @@ export default { <div v-if="isRearrangingPanels" class="draggable-remove js-draggable-remove p-2 w-100 position-absolute d-flex justify-content-end" - @click="removeGraph(groupData.metrics, graphIndex)" + @click="removeGraph(groupData.panels, graphIndex)" > <a class="mx-2 p-2 draggable-remove-link" :aria-label="__('Remove')" ><icon name="close" diff --git a/app/assets/javascripts/monitoring/components/embed.vue b/app/assets/javascripts/monitoring/components/embed.vue index f75839c7c6b..581b2093d44 100644 --- a/app/assets/javascripts/monitoring/components/embed.vue +++ b/app/assets/javascripts/monitoring/components/embed.vue @@ -37,11 +37,14 @@ export default { computed: { ...mapState('monitoringDashboard', ['dashboard', 'metricsWithData']), charts() { + if (!this.dashboard || !this.dashboard.panel_groups) { + return []; + } const groupWithMetrics = this.dashboard.panel_groups.find(group => - group.metrics.find(chart => this.chartHasData(chart)), - ) || { metrics: [] }; + group.panels.find(chart => this.chartHasData(chart)), + ) || { panels: [] }; - return groupWithMetrics.metrics.filter(chart => this.chartHasData(chart)); + return groupWithMetrics.panels.filter(chart => this.chartHasData(chart)); }, isSingleChart() { return this.charts.length === 1; diff --git a/app/assets/javascripts/monitoring/components/panel_type.vue b/app/assets/javascripts/monitoring/components/panel_type.vue index effcf334cbc..fab1dd0f981 100644 --- a/app/assets/javascripts/monitoring/components/panel_type.vue +++ b/app/assets/javascripts/monitoring/components/panel_type.vue @@ -54,10 +54,14 @@ export default { return IS_EE && this.prometheusAlertsAvailable && this.alertsEndpoint && this.graphData; }, graphDataHasMetrics() { - return this.graphData.queries[0].result.length > 0; + return ( + this.graphData.metrics && + this.graphData.metrics[0].result && + this.graphData.metrics[0].result.length > 0 + ); }, csvText() { - const chartData = this.graphData.queries[0].result[0].values; + const chartData = this.graphData.metrics[0].result[0].values; const yLabel = this.graphData.y_label; const header = `timestamp,${yLabel}\r\n`; // eslint-disable-line @gitlab/i18n/no-non-i18n-strings return chartData.reduce((csv, data) => { @@ -112,7 +116,7 @@ export default { :graph-data="graphData" :deployment-data="deploymentData" :project-path="projectPath" - :thresholds="getGraphAlertValues(graphData.queries)" + :thresholds="getGraphAlertValues(graphData.metrics)" group-id="panel-type-chart" > <div class="d-flex align-items-center"> @@ -120,8 +124,8 @@ export default { v-if="alertWidgetAvailable && graphData" :modal-id="`alert-modal-${index}`" :alerts-endpoint="alertsEndpoint" - :relevant-queries="graphData.queries" - :alerts-to-manage="getGraphAlerts(graphData.queries)" + :relevant-queries="graphData.metrics" + :alerts-to-manage="getGraphAlerts(graphData.metrics)" @setAlerts="setAlerts" /> <gl-dropdown diff --git a/app/assets/javascripts/monitoring/stores/mutations.js b/app/assets/javascripts/monitoring/stores/mutations.js index 696af5aed75..bfa76aa7cea 100644 --- a/app/assets/javascripts/monitoring/stores/mutations.js +++ b/app/assets/javascripts/monitoring/stores/mutations.js @@ -1,9 +1,13 @@ import Vue from 'vue'; import { slugify } from '~/lib/utils/text_utility'; import * as types from './mutation_types'; -import { normalizeMetrics, normalizeMetric, normalizeQueryResult } from './utils'; +import { normalizeMetric, normalizeQueryResult } from './utils'; -const normalizePanel = panel => panel.metrics.map(normalizeMetric); +const normalizePanelMetrics = (metrics, defaultLabel) => + metrics.map(metric => ({ + ...normalizeMetric(metric), + label: metric.label || defaultLabel, + })); export default { [types.REQUEST_METRICS_DATA](state) { @@ -13,28 +17,18 @@ export default { [types.RECEIVE_METRICS_DATA_SUCCESS](state, groupData) { state.dashboard.panel_groups = groupData.map((group, i) => { const key = `${slugify(group.group || 'default')}-${i}`; - let { metrics = [], panels = [] } = group; + let { panels = [] } = group; // each panel has metric information that needs to be normalized - panels = panels.map(panel => ({ ...panel, - metrics: normalizePanel(panel), - })); - - // for backwards compatibility, and to limit Vue template changes: - // for each group alias panels to metrics - // for each panel alias metrics to queries - metrics = panels.map(panel => ({ - ...panel, - queries: panel.metrics, + metrics: normalizePanelMetrics(panel.metrics, panel.y_label), })); return { ...group, panels, key, - metrics: normalizeMetrics(metrics), }; }); @@ -58,6 +52,7 @@ export default { [types.RECEIVE_ENVIRONMENTS_DATA_FAILURE](state) { state.environments = []; }, + [types.SET_QUERY_RESULT](state, { metricId, result }) { if (!metricId || !result || result.length === 0) { return; @@ -65,14 +60,17 @@ export default { state.showEmptyState = false; + /** + * Search the dashboard state for a matching id + */ state.dashboard.panel_groups.forEach(group => { - group.metrics.forEach(metric => { - metric.queries.forEach(query => { - if (query.metric_id === metricId) { + group.panels.forEach(panel => { + panel.metrics.forEach(metric => { + if (metric.metric_id === metricId) { state.metricsWithData.push(metricId); // ensure dates/numbers are correctly formatted for charts const normalizedResults = result.map(normalizeQueryResult); - Vue.set(query, 'result', Object.freeze(normalizedResults)); + Vue.set(metric, 'result', Object.freeze(normalizedResults)); } }); }); @@ -101,6 +99,6 @@ export default { }, [types.SET_PANEL_GROUP_METRICS](state, payload) { const panelGroup = state.dashboard.panel_groups.find(pg => payload.key === pg.key); - panelGroup.metrics = payload.metrics; + panelGroup.panels = payload.panels; }, }; diff --git a/app/assets/javascripts/monitoring/stores/utils.js b/app/assets/javascripts/monitoring/stores/utils.js index 8a396b15a31..3300d2032d0 100644 --- a/app/assets/javascripts/monitoring/stores/utils.js +++ b/app/assets/javascripts/monitoring/stores/utils.js @@ -1,83 +1,21 @@ import _ from 'underscore'; -function checkQueryEmptyData(query) { - return { - ...query, - result: query.result.filter(timeSeries => { - const newTimeSeries = timeSeries; - const hasValue = series => - !Number.isNaN(series[1]) && (series[1] !== null || series[1] !== undefined); - const hasNonNullValue = timeSeries.values.find(hasValue); - - newTimeSeries.values = hasNonNullValue ? newTimeSeries.values : []; - - return newTimeSeries.values.length > 0; - }), - }; -} - -function removeTimeSeriesNoData(queries) { - return queries.reduce((series, query) => series.concat(checkQueryEmptyData(query)), []); -} - -// Metrics and queries are currently stored 1:1, so `queries` is an array of length one. -// We want to group queries onto a single chart by title & y-axis label. -// This function will no longer be required when metrics:queries are 1:many, -// though there is no consequence if the function stays in use. -// @param metrics [Array<Object>] -// Ex) [ -// { id: 1, title: 'title', y_label: 'MB', queries: [{ ...query1Attrs }] }, -// { id: 2, title: 'title', y_label: 'MB', queries: [{ ...query2Attrs }] }, -// { id: 3, title: 'new title', y_label: 'MB', queries: [{ ...query3Attrs }] } -// ] -// @return [Array<Object>] -// Ex) [ -// { title: 'title', y_label: 'MB', queries: [{ metricId: 1, ...query1Attrs }, -// { metricId: 2, ...query2Attrs }] }, -// { title: 'new title', y_label: 'MB', queries: [{ metricId: 3, ...query3Attrs }]} -// ] -export function groupQueriesByChartInfo(metrics) { - const metricsByChart = metrics.reduce((accumulator, metric) => { - const { queries, ...chart } = metric; - - const chartKey = `${chart.title}|${chart.y_label}`; - accumulator[chartKey] = accumulator[chartKey] || { ...chart, queries: [] }; - - queries.forEach(queryAttrs => { - let metricId; - - if (chart.id) { - metricId = chart.id.toString(); - } else if (queryAttrs.metric_id) { - metricId = queryAttrs.metric_id.toString(); - } else { - metricId = null; - } - - accumulator[chartKey].queries.push({ metricId, ...queryAttrs }); - }); - - return accumulator; - }, {}); - - return Object.values(metricsByChart); -} - export const uniqMetricsId = metric => `${metric.metric_id}_${metric.id}`; /** - * Not to confuse with normalizeMetrics (plural) * Metrics loaded from project-defined dashboards do not have a metric_id. * This method creates a unique ID combining metric_id and id, if either is present. * This is hopefully a temporary solution until BE processes metrics before passing to fE * @param {Object} metric - metric * @returns {Object} - normalized metric with a uniqueID */ + export const normalizeMetric = (metric = {}) => _.omit( { ...metric, metric_id: uniqMetricsId(metric), + metricId: uniqMetricsId(metric), }, 'id', ); @@ -93,6 +31,11 @@ export const normalizeQueryResult = timeSeries => { Number(value), ]), }; + // Check result for empty data + normalizedResult.values = normalizedResult.values.filter(series => { + const hasValue = d => !Number.isNaN(d[1]) && (d[1] !== null || d[1] !== undefined); + return series.find(hasValue); + }); } else if (timeSeries.value) { normalizedResult = { ...timeSeries, @@ -102,21 +45,3 @@ export const normalizeQueryResult = timeSeries => { return normalizedResult; }; - -export const normalizeMetrics = metrics => { - const groupedMetrics = groupQueriesByChartInfo(metrics); - - return groupedMetrics.map(metric => { - const queries = metric.queries.map(query => ({ - ...query, - // custom metrics do not require a label, so we should ensure this attribute is defined - label: query.label || metric.y_label, - result: (query.result || []).map(normalizeQueryResult), - })); - - return { - ...metric, - queries: removeTimeSeriesNoData(queries), - }; - }); -}; diff --git a/app/assets/javascripts/monitoring/utils.js b/app/assets/javascripts/monitoring/utils.js index 2ae1647011d..336f81aeeff 100644 --- a/app/assets/javascripts/monitoring/utils.js +++ b/app/assets/javascripts/monitoring/utils.js @@ -72,10 +72,9 @@ export const ISODateToString = date => dateformat(date, dateFormats.dateTimePick */ export const graphDataValidatorForValues = (isValues, graphData) => { const responseValueKeyName = isValues ? 'value' : 'values'; - return ( - Array.isArray(graphData.queries) && - graphData.queries.filter(query => { + Array.isArray(graphData.metrics) && + graphData.metrics.filter(query => { if (Array.isArray(query.result)) { return ( query.result.filter(res => Array.isArray(res[responseValueKeyName])).length === @@ -83,7 +82,7 @@ export const graphDataValidatorForValues = (isValues, graphData) => { ); } return false; - }).length === graphData.queries.length + }).length === graphData.metrics.filter(query => query.result).length ); }; @@ -131,7 +130,7 @@ export const downloadCSVOptions = title => { }; /** - * This function validates the graph data contains exactly 3 queries plus + * This function validates the graph data contains exactly 3 metrics plus * value validations from graphDataValidatorForValues. * @param {Object} isValues * @param {Object} graphData the graph data response from a prometheus request @@ -140,8 +139,8 @@ export const downloadCSVOptions = title => { export const graphDataValidatorForAnomalyValues = graphData => { const anomalySeriesCount = 3; // metric, upper, lower return ( - graphData.queries && - graphData.queries.length === anomalySeriesCount && + graphData.metrics && + graphData.metrics.length === anomalySeriesCount && graphDataValidatorForValues(false, graphData) ); }; diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb index bd6b6190fb5..5819f279eaa 100644 --- a/app/finders/snippets_finder.rb +++ b/app/finders/snippets_finder.rb @@ -40,15 +40,14 @@ # Any other value will be ignored. class SnippetsFinder < UnionFinder include FinderMethods + include Gitlab::Utils::StrongMemoize - attr_accessor :current_user, :project, :author, :scope, :explore + attr_accessor :current_user, :params + delegate :explore, :only_personal, :only_project, :scope, to: :params def initialize(current_user = nil, params = {}) @current_user = current_user - @project = params[:project] - @author = params[:author] - @scope = params[:scope].to_s - @explore = params[:explore] + @params = OpenStruct.new(params) if project && author raise( @@ -60,8 +59,15 @@ class SnippetsFinder < UnionFinder end def execute - base = init_collection - base.with_optional_visibility(visibility_from_scope).fresh + # The snippet query can be expensive, therefore if the + # author or project params have been passed and they don't + # exist, it's better to return + return Snippet.none if author.nil? && params[:author].present? + return Snippet.none if project.nil? && params[:project].present? + + items = init_collection + items = by_ids(items) + items.with_optional_visibility(visibility_from_scope).fresh end private @@ -69,10 +75,12 @@ class SnippetsFinder < UnionFinder def init_collection if explore snippets_for_explore + elsif only_personal + personal_snippets elsif project snippets_for_a_single_project else - snippets_for_multiple_projects + snippets_for_personal_and_multiple_projects end end @@ -96,8 +104,9 @@ class SnippetsFinder < UnionFinder # # Each collection is constructed in isolation, allowing for greater control # over the resulting SQL query. - def snippets_for_multiple_projects - queries = [personal_snippets] + def snippets_for_personal_and_multiple_projects + queries = [] + queries << personal_snippets unless only_project if Ability.allowed?(current_user, :read_cross_project) queries << snippets_of_visible_projects @@ -158,7 +167,7 @@ class SnippetsFinder < UnionFinder end def visibility_from_scope - case scope + case scope.to_s when 'are_private' Snippet::PRIVATE when 'are_internal' @@ -169,6 +178,28 @@ class SnippetsFinder < UnionFinder nil end end + + def by_ids(items) + return items unless params[:ids].present? + + items.id_in(params[:ids]) + end + + def author + strong_memoize(:author) do + next unless params[:author].present? + + params[:author].is_a?(User) ? params[:author] : User.find_by_id(params[:author]) + end + end + + def project + strong_memoize(:project) do + next unless params[:project].present? + + params[:project].is_a?(Project) ? params[:project] : Project.find_by_id(params[:project]) + end + end end SnippetsFinder.prepend_if_ee('EE::SnippetsFinder') diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 912f0b61978..659f9778892 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -141,7 +141,7 @@ module BlobHelper if @build && @entry raw_project_job_artifacts_url(@project, @build, path: @entry.path, **kwargs) elsif @snippet - reliable_raw_snippet_url(@snippet) + raw_snippet_url(@snippet) elsif @blob project_raw_url(@project, @id, **kwargs) end diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb index 404ea7b00d4..38ca12e6f90 100644 --- a/app/helpers/gitlab_routing_helper.rb +++ b/app/helpers/gitlab_routing_helper.rb @@ -193,6 +193,101 @@ module GitlabRoutingHelper project = schedule.project take_ownership_project_pipeline_schedule_path(project, schedule, *args) end + + def snippet_path(snippet, *args) + if snippet.is_a?(ProjectSnippet) + application_url_helpers.project_snippet_path(snippet.project, snippet, *args) + else + new_args = snippet_query_params(snippet, *args) + application_url_helpers.snippet_path(snippet, *new_args) + end + end + + def snippet_url(snippet, *args) + if snippet.is_a?(ProjectSnippet) + application_url_helpers.project_snippet_url(snippet.project, snippet, *args) + else + new_args = snippet_query_params(snippet, *args) + application_url_helpers.snippet_url(snippet, *new_args) + end + end + + def raw_snippet_path(snippet, *args) + if snippet.is_a?(ProjectSnippet) + application_url_helpers.raw_project_snippet_path(snippet.project, snippet, *args) + else + new_args = snippet_query_params(snippet, *args) + application_url_helpers.raw_snippet_path(snippet, *new_args) + end + end + + def raw_snippet_url(snippet, *args) + if snippet.is_a?(ProjectSnippet) + application_url_helpers.raw_project_snippet_url(snippet.project, snippet, *args) + else + new_args = snippet_query_params(snippet, *args) + application_url_helpers.raw_snippet_url(snippet, *new_args) + end + end + + def snippet_notes_path(snippet, *args) + new_args = snippet_query_params(snippet, *args) + application_url_helpers.snippet_notes_path(snippet, *new_args) + end + + def snippet_notes_url(snippet, *args) + new_args = snippet_query_params(snippet, *args) + application_url_helpers.snippet_notes_url(snippet, *new_args) + end + + def snippet_note_path(snippet, note, *args) + new_args = snippet_query_params(snippet, *args) + application_url_helpers.snippet_note_path(snippet, note, *new_args) + end + + def snippet_note_url(snippet, note, *args) + new_args = snippet_query_params(snippet, *args) + application_url_helpers.snippet_note_url(snippet, note, *new_args) + end + + def toggle_award_emoji_snippet_note_path(snippet, note, *args) + new_args = snippet_query_params(snippet, *args) + application_url_helpers.toggle_award_emoji_snippet_note_path(snippet, note, *new_args) + end + + def toggle_award_emoji_snippet_note_url(snippet, note, *args) + new_args = snippet_query_params(snippet, *args) + application_url_helpers.toggle_award_emoji_snippet_note_url(snippet, note, *new_args) + end + + def toggle_award_emoji_snippet_path(snippet, *args) + new_args = snippet_query_params(snippet, *args) + application_url_helpers.toggle_award_emoji_snippet_path(snippet, *new_args) + end + + def toggle_award_emoji_snippet_url(snippet, *args) + new_args = snippet_query_params(snippet, *args) + application_url_helpers.toggle_award_emoji_snippet_url(snippet, *new_args) + end + + private + + def application_url_helpers + Gitlab::Routing.url_helpers + end + + def snippet_query_params(snippet, *args) + opts = case args.last + when Hash + args.pop + when ActionController::Parameters + args.pop.to_h + else + {} + end + + args << opts + end end GitlabRoutingHelper.include_if_ee('EE::GitlabRoutingHelper') diff --git a/app/helpers/snippets_helper.rb b/app/helpers/snippets_helper.rb index 10e31fb8888..7d7d8646c25 100644 --- a/app/helpers/snippets_helper.rb +++ b/app/helpers/snippets_helper.rb @@ -11,33 +11,9 @@ module SnippetsHelper end end - def reliable_snippet_path(snippet, opts = {}) - reliable_snippet_url(snippet, opts.merge(only_path: true)) - end - - def reliable_raw_snippet_path(snippet, opts = {}) - reliable_raw_snippet_url(snippet, opts.merge(only_path: true)) - end - - def reliable_snippet_url(snippet, opts = {}) - if snippet.project_id? - project_snippet_url(snippet.project, snippet, nil, opts) - else - snippet_url(snippet, nil, opts) - end - end - - def reliable_raw_snippet_url(snippet, opts = {}) - if snippet.project_id? - raw_project_snippet_url(snippet.project, snippet, nil, opts) - else - raw_snippet_url(snippet, nil, opts) - end - end - def download_raw_snippet_button(snippet) link_to(icon('download'), - reliable_raw_snippet_path(snippet, inline: false), + raw_snippet_path(snippet, inline: false), target: '_blank', rel: 'noopener noreferrer', class: "btn btn-sm has-tooltip", @@ -133,7 +109,7 @@ module SnippetsHelper end def snippet_embed_tag(snippet) - content_tag(:script, nil, src: reliable_snippet_url(snippet, format: :js, only_path: false)) + content_tag(:script, nil, src: snippet_url(snippet, format: :js)) end def snippet_badge(snippet) @@ -158,7 +134,7 @@ module SnippetsHelper return if blob.empty? || blob.binary? || blob.stored_externally? link_to(external_snippet_icon('doc-code'), - reliable_raw_snippet_url(@snippet), + raw_snippet_url(@snippet), class: 'btn', target: '_blank', rel: 'noopener noreferrer', @@ -167,7 +143,7 @@ module SnippetsHelper def embedded_snippet_download_button link_to(external_snippet_icon('download'), - reliable_raw_snippet_url(@snippet, inline: false), + raw_snippet_url(@snippet, inline: false), class: 'btn', target: '_blank', title: 'Download', diff --git a/app/models/commit.rb b/app/models/commit.rb index aae49c36899..95993089426 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -464,8 +464,20 @@ class Commit "commit:#{sha}" end + def expire_note_etag_cache + super + + expire_note_etag_cache_for_related_mrs + end + private + def expire_note_etag_cache_for_related_mrs + MergeRequest.includes(target_project: :namespace).by_commit_sha(id).find_each do |mr| + mr.expire_note_etag_cache + end + end + def commit_reference(from, referable_commit_id, full: false) reference = project.to_reference(from, full: full) diff --git a/app/models/import_failure.rb b/app/models/import_failure.rb new file mode 100644 index 00000000000..998572853d3 --- /dev/null +++ b/app/models/import_failure.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class ImportFailure < ApplicationRecord + belongs_to :project + + validates :project, presence: true +end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 16d36d42eac..eb98bf3da7f 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -199,6 +199,9 @@ class MergeRequest < ApplicationRecord scope :by_milestone, ->(milestone) { where(milestone_id: milestone) } scope :of_projects, ->(ids) { where(target_project_id: ids) } scope :from_project, ->(project) { where(source_project_id: project.id) } + scope :from_and_to_forks, ->(project) do + where('source_project_id <> target_project_id AND (source_project_id = ? OR target_project_id = ?)', project.id, project.id) + end scope :merged, -> { with_state(:merged) } scope :closed_and_merged, -> { with_states(:closed, :merged) } scope :open_and_closed, -> { with_states(:opened, :closed) } @@ -1468,6 +1471,10 @@ class MergeRequest < ApplicationRecord all_pipelines.for_sha_or_source_sha(diff_head_sha).first end + def etag_caching_enabled? + true + end + private def with_rebase_lock diff --git a/app/models/project.rb b/app/models/project.rb index 4cfbb963ddf..26392964ced 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -179,6 +179,7 @@ class Project < ApplicationRecord has_one :forked_from_project, through: :fork_network_member has_many :forked_to_members, class_name: 'ForkNetworkMember', foreign_key: 'forked_from_project_id' has_many :forks, through: :forked_to_members, source: :project, inverse_of: :forked_from_project + has_many :fork_network_projects, through: :fork_network, source: :projects has_one :import_state, autosave: true, class_name: 'ProjectImportState', inverse_of: :project has_one :import_export_upload, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent @@ -295,6 +296,8 @@ class Project < ApplicationRecord has_one :pages_metadatum, class_name: 'ProjectPagesMetadatum', inverse_of: :project + has_many :import_failures, inverse_of: :project + accepts_nested_attributes_for :variables, allow_destroy: true accepts_nested_attributes_for :project_feature, update_only: true accepts_nested_attributes_for :import_data @@ -715,6 +718,10 @@ class Project < ApplicationRecord Feature.enabled?(:project_daily_statistics, self, default_enabled: true) end + def unlink_forks_upon_visibility_decrease_enabled? + Feature.enabled?(:unlink_fork_network_upon_visibility_decrease, self) + end + def empty_repo? repository.empty? end @@ -1533,6 +1540,7 @@ class Project < ApplicationRecord # update visibility_level of forks def update_forks_visibility_level + return if unlink_forks_upon_visibility_decrease_enabled? return unless visibility_level < visibility_level_before_last_save forks.each do |forked_project| diff --git a/app/services/metrics/dashboard/grafana_metric_embed_service.rb b/app/services/metrics/dashboard/grafana_metric_embed_service.rb index 60591e9a6f3..44b58ad9729 100644 --- a/app/services/metrics/dashboard/grafana_metric_embed_service.rb +++ b/app/services/metrics/dashboard/grafana_metric_embed_service.rb @@ -133,7 +133,7 @@ module Metrics def uid_regex base_url = @project.grafana_integration.grafana_url.chomp('/') - %r{(#{Regexp.escape(base_url)}\/d\/(?<uid>\w+)\/)}x + %r{^(#{Regexp.escape(base_url)}\/d\/(?<uid>.+)\/)}x end end diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 90e703e7050..cbed794f92e 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -31,13 +31,6 @@ module Projects Projects::UnlinkForkService.new(project, current_user).execute - # The project is not necessarily a fork, so update the fork network originating - # from this project - if fork_network = project.root_of_fork_network - fork_network.update(root_project: nil, - deleted_root_project_name: project.full_name) - end - attempt_destroy_transaction(project) system_hook_service.execute_hooks_for(project, :destroy) diff --git a/app/services/projects/overwrite_project_service.rb b/app/services/projects/overwrite_project_service.rb index 696e1b665b2..c5e38f166da 100644 --- a/app/services/projects/overwrite_project_service.rb +++ b/app/services/projects/overwrite_project_service.rb @@ -7,7 +7,9 @@ module Projects Project.transaction do move_before_destroy_relationships(source_project) - destroy_old_project(source_project) + # Reset is required in order to get the proper + # uncached fork network method calls value. + destroy_old_project(source_project.reset) rename_project(source_project.name, source_project.path) @project diff --git a/app/services/projects/unlink_fork_service.rb b/app/services/projects/unlink_fork_service.rb index 1b8a920268f..e7e0141099e 100644 --- a/app/services/projects/unlink_fork_service.rb +++ b/app/services/projects/unlink_fork_service.rb @@ -2,34 +2,67 @@ module Projects class UnlinkForkService < BaseService - # rubocop: disable CodeReuse/ActiveRecord + # If a fork is given, it: + # + # - Saves LFS objects to the root project + # - Close existing MRs coming from it + # - Is removed from the fork network + # + # If a root of fork(s) is given, it does the same, + # but not updating LFS objects (there'll be no related root to cache it). def execute - return unless @project.forked? + fork_network = @project.fork_network - if fork_source = @project.fork_source - fork_source.lfs_objects.find_each do |lfs_object| - lfs_object.projects << @project unless lfs_object.projects.include?(@project) - end + return unless fork_network - refresh_forks_count(fork_source) - end + save_lfs_objects - merge_requests = @project.fork_network + merge_requests = fork_network .merge_requests .opened - .where.not(target_project: @project) - .from_project(@project) + .from_and_to_forks(@project) - merge_requests.each do |mr| + merge_requests.find_each do |mr| ::MergeRequests::CloseService.new(@project, @current_user).execute(mr) end - @project.fork_network_member.destroy + Project.transaction do + # Get out of the fork network as a member and + # remove references from all its direct forks. + @project.fork_network_member.destroy + @project.forked_to_members.update_all(forked_from_project_id: nil) + + # The project is not necessarily a fork, so update the fork network originating + # from this project + if fork_network = @project.root_of_fork_network + fork_network.update(root_project: nil, deleted_root_project_name: @project.full_name) + end + end + + # When the project getting out of the network is a node with parent + # and children, both the parent and the node needs a cache refresh. + [@project.forked_from_project, @project].compact.each do |project| + refresh_forks_count(project) + end end - # rubocop: enable CodeReuse/ActiveRecord + + private def refresh_forks_count(project) Projects::ForksCountService.new(project).refresh_cache end + + def save_lfs_objects + return unless @project.forked? + + lfs_storage_project = @project.lfs_storage_project + + return unless lfs_storage_project + return if lfs_storage_project == @project # that project is being unlinked + + lfs_storage_project.lfs_objects.find_each do |lfs_object| + lfs_object.projects << @project unless lfs_object.projects.include?(@project) + end + end end end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 2dad1d05a2c..aedd7252f63 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -65,7 +65,7 @@ module Projects ) project_changed_feature_keys = project.project_feature.previous_changes.keys - if project.previous_changes.include?(:visibility_level) && project.private? + if project.visibility_level_previous_changes && project.private? # don't enqueue immediately to prevent todos removal in case of a mistake TodosDestroyer::ConfidentialIssueWorker.perform_in(Todo::WAIT_FOR_DELETE, nil, project.id) TodosDestroyer::ProjectPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, project.id) @@ -79,6 +79,11 @@ module Projects system_hook_service.execute_hooks_for(project, :update) end + if project.visibility_level_decreased? && project.unlink_forks_upon_visibility_decrease_enabled? + # It's a system-bounded operation, so no extra authorization check is required. + Projects::UnlinkForkService.new(project, current_user).execute + end + update_pages_config if changing_pages_related_config? end diff --git a/app/views/search/results/_snippet_blob.html.haml b/app/views/search/results/_snippet_blob.html.haml index 37f4efee9d2..2dbd2a74602 100644 --- a/app/views/search/results/_snippet_blob.html.haml +++ b/app/views/search/results/_snippet_blob.html.haml @@ -1,7 +1,7 @@ - snippet_blob = chunk_snippet(snippet_blob, @search_term) - snippet = snippet_blob[:snippet_object] - snippet_chunks = snippet_blob[:snippet_chunks] -- snippet_path = reliable_snippet_path(snippet) +- snippet_path = snippet_path(snippet) .search-result-row %span diff --git a/app/views/search/results/_snippet_title.html.haml b/app/views/search/results/_snippet_title.html.haml index 7280146720e..a544e59c405 100644 --- a/app/views/search/results/_snippet_title.html.haml +++ b/app/views/search/results/_snippet_title.html.haml @@ -1,6 +1,6 @@ .search-result-row %h4.snippet-title.term - = link_to reliable_snippet_path(snippet_title) do + = link_to snippet_path(snippet_title) do = truncate(snippet_title.title, length: 60) = snippet_badge(snippet_title) %span.cgray.monospace.tiny.float-right.term diff --git a/app/views/shared/snippets/_snippet.html.haml b/app/views/shared/snippets/_snippet.html.haml index 5602ea37b5c..7d102a1b280 100644 --- a/app/views/shared/snippets/_snippet.html.haml +++ b/app/views/shared/snippets/_snippet.html.haml @@ -5,7 +5,7 @@ = image_tag avatar_icon_for_user(snippet.author), class: "avatar s40 d-none d-sm-block", alt: '' .title - = link_to reliable_snippet_path(snippet) do + = link_to snippet_path(snippet) do = snippet.title - if snippet.file_name.present? %span.snippet-filename.d-none.d-sm-inline-block.ml-2 @@ -14,7 +14,7 @@ %ul.controls %li - = link_to reliable_snippet_path(snippet, anchor: 'notes'), class: ('no-comments' if notes_count.zero?) do + = link_to snippet_path(snippet, anchor: 'notes'), class: ('no-comments' if notes_count.zero?) do = icon('comments') = notes_count %li |