diff --git a/app/assets/javascripts/merge_requests/constants.js b/app/assets/javascripts/merge_requests/constants.js new file mode 100644 index 0000000000000000000000000000000000000000..83293ad5a364c2d1f2c5a3439a2b067ea02a0830 --- /dev/null +++ b/app/assets/javascripts/merge_requests/constants.js @@ -0,0 +1 @@ +export const MAX_LIST_SIZE = 10; diff --git a/app/assets/javascripts/merge_requests/list/components/merge_requests_list_app.vue b/app/assets/javascripts/merge_requests/list/components/merge_requests_list_app.vue index 09adf5a019f299859e125e1b48272d9a8032e384..a182dd8b75bb78c7c5dc1a5813be9e3462da1f0d 100644 --- a/app/assets/javascripts/merge_requests/list/components/merge_requests_list_app.vue +++ b/app/assets/javascripts/merge_requests/list/components/merge_requests_list_app.vue @@ -2,16 +2,15 @@ import { GlFilteredSearchToken, GlButton, GlLink, GlIcon, GlTooltipDirective } from '@gitlab/ui'; import { isEmpty } from 'lodash'; import ApprovalCount from 'ee_else_ce/merge_requests/components/approval_count.vue'; -import { createAlert } from '~/alert'; import Api from '~/api'; import * as Sentry from '~/sentry/sentry_browser_wrapper'; import { STATUS_ALL, STATUS_CLOSED, STATUS_OPEN, STATUS_MERGED } from '~/issues/constants'; import { fetchPolicies } from '~/lib/graphql'; import { isPositiveInteger } from '~/lib/utils/number_utils'; import { scrollUp } from '~/lib/utils/scroll_utils'; -import { getParameterByName } from '~/lib/utils/url_utility'; +import { getParameterByName, mergeUrlParams } from '~/lib/utils/url_utility'; import { TYPENAME_USER } from '~/graphql_shared/constants'; -import { convertToGraphQLId } from '~/graphql_shared/utils'; +import { convertToGraphQLId, getIdFromGraphQLId } from '~/graphql_shared/utils'; import IssuableList from '~/vue_shared/issuable/list/components/issuable_list_root.vue'; import IssuableMilestone from '~/vue_shared/issuable/list/components/issuable_milestone.vue'; import { DEFAULT_PAGE_SIZE, mergeRequestListTabs } from '~/vue_shared/issuable/list/constants'; @@ -51,7 +50,6 @@ import { TOKEN_TYPE_ENVIRONMENT, TOKEN_TITLE_ENVIRONMENT, } from '~/vue_shared/components/filtered_search_bar/constants'; -import { AutocompleteCache } from '~/issues/dashboard/utils'; import { convertToApiParams, convertToSearchQuery, @@ -73,7 +71,8 @@ import { import CiIcon from '~/vue_shared/components/ci_icon/ci_icon.vue'; import setSortPreferenceMutation from '~/issues/list/queries/set_sort_preference.mutation.graphql'; import issuableEventHub from '~/issues/list/eventhub'; -import { i18n } from '../constants'; +import { AutocompleteCache } from '../../utils/autocomplete_cache'; +import { i18n, BRANCH_LIST_REFRESH_INTERVAL } from '../constants'; import getMergeRequestsQuery from '../queries/get_merge_requests.query.graphql'; import getMergeRequestsCountsQuery from '../queries/get_merge_requests_counts.query.graphql'; import searchLabelsQuery from '../queries/search_labels.query.graphql'; @@ -94,6 +93,10 @@ const EmojiToken = () => import('~/vue_shared/components/filtered_search_bar/tokens/emoji_token.vue'); const DateToken = () => import('~/vue_shared/components/filtered_search_bar/tokens/date_token.vue'); +function cacheIsExpired(cacheAge, compareTo = Date.now()) { + return cacheAge + BRANCH_LIST_REFRESH_INTERVAL <= compareTo; +} + export default { name: 'MergeRequestsListApp', i18n, @@ -129,6 +132,8 @@ export default { }, data() { return { + projectId: null, + branchCacheAges: {}, filterTokens: [], mergeRequests: [], mergeRequestCounts: {}, @@ -157,6 +162,7 @@ export default { if (!data) { return; } + this.projectId = getIdFromGraphQLId(data.project.id); this.pageInfo = data.project.mergeRequests?.pageInfo ?? {}; }, error(error) { @@ -336,7 +342,7 @@ export default { operators: OPERATORS_IS, fullPath: this.fullPath, isProject: true, - fetchBranches: this.fetchBranches, + fetchBranches: this.fetchTargetBranches, }, { type: TOKEN_TYPE_SOURCE_BRANCH, @@ -346,7 +352,7 @@ export default { operators: OPERATORS_IS, fullPath: this.fullPath, isProject: true, - fetchBranches: this.fetchBranches, + fetchBranches: this.fetchSourceBranches, }, { type: TOKEN_TYPE_LABEL, @@ -472,16 +478,62 @@ export default { issuableEventHub.$off('issuables:toggleBulkEdit', this.toggleBulkEditSidebar); }, methods: { - fetchBranches(search) { - return Api.branches(this.fullPath, search) - .then((response) => { - return response; + getBranchPath(branchType = 'other') { + const typeUrls = { + source: '/-/autocomplete/merge_request_source_branches.json', + target: '/-/autocomplete/merge_request_target_branches.json', + other: Api.buildUrl(Api.createBranchPath).replace(':id', encodeURIComponent(this.fullPath)), + }; + const url = typeUrls[branchType]; + + return url && this.projectId + ? mergeUrlParams({ project_id: this.projectId }, url) + : typeUrls.other; + }, + async updateBranchCache(branchType, path) { + const lastCheck = this.branchCacheAges[branchType]; + + if (cacheIsExpired(lastCheck)) { + await this.autocompleteCache.updateLocalCache(path); + } + }, + async fetchBranches(type = 'other', search) { + const branchPath = this.getBranchPath(type); + const cacheAge = this.branchCacheAges[type]; + const runTime = Date.now(); + + await this.updateBranchCache(type, branchPath); + + const fetch = this.autocompleteCache.fetch({ + mutator: (branchList) => + branchList.map((branch, index) => ({ + ...branch, + name: branch.name || branch.title, + id: index, + })), + formatter: (results) => ({ data: results }), + url: branchPath, + searchProperty: 'name', + search, + }); + + fetch + .then(() => { + if (!cacheAge || cacheIsExpired(cacheAge, runTime)) { + this.branchCacheAges[type] = Date.now(); + } }) .catch(() => { - createAlert({ - message: this.$options.i18n.errorFetchingBranches, - }); + // An error has occurred, but there's nothing the user can do about it, so... we're swallowing it. }); + + return fetch; + }, + fetchTargetBranches(search) { + return this.fetchBranches('target', search); + }, + fetchSourceBranches(search) { + return this.fetchBranches('source', search); }, fetchEmojis(search) { return this.autocompleteCache.fetch({ diff --git a/app/assets/javascripts/merge_requests/list/constants.js b/app/assets/javascripts/merge_requests/list/constants.js index 1fb431426dd2ecdc43edeb6c36e22537ffba557c..651152a93cdef00030f46a6d25879d0e9ffc544b 100644 --- a/app/assets/javascripts/merge_requests/list/constants.js +++ b/app/assets/javascripts/merge_requests/list/constants.js @@ -12,3 +12,5 @@ export const i18n = { downvots: __('Downvotes'), newMergeRequest: __('New merge request'), }; + +export const BRANCH_LIST_REFRESH_INTERVAL = 600000; // 10 minutes (* 60 seconds, * 1000 milliseconds) diff --git a/app/assets/javascripts/merge_requests/utils/autocomplete_cache.js b/app/assets/javascripts/merge_requests/utils/autocomplete_cache.js new file mode 100644 index 0000000000000000000000000000000000000000..70733670998c187ac4cf03a3898a914320756840 --- /dev/null +++ b/app/assets/javascripts/merge_requests/utils/autocomplete_cache.js @@ -0,0 +1,57 @@ +import fuzzaldrinPlus from 'fuzzaldrin-plus'; +import axios from '~/lib/utils/axios_utils'; + +import { MAX_LIST_SIZE } from '../constants'; + +export class AutocompleteCache { + constructor() { + this.cache = new Map(); + this.mutators = new Map(); + this.formatters = new Map(); + this.searchProperties = new Map(); + } + + setUpCache({ url, property, mutator, formatter }) { + this.mutators.set(url, mutator); + this.formatters.set(url, formatter); + this.searchProperties.set(url, property); + } + + async fetch({ url, searchProperty, search, mutator, formatter }) { + this.setUpCache({ url, property: searchProperty, mutator, formatter }); + + if (!this.cache.has(url)) { + await this.updateLocalCache(url); + } + + return this.retrieveFromLocalCache(url, search); + } + + async updateLocalCache(url) { + const mutator = this.mutators.get(url); + + return axios.get(url).then(({ data }) => { + let finalData = data; + + if (mutator) { + finalData = mutator(finalData); + } + + this.cache.set(url, finalData); + }); + } + + retrieveFromLocalCache(url, search) { + const searchProperty = this.searchProperties.get(url); + const formatter = this.formatters.get(url); + let result = search + ? fuzzaldrinPlus.filter(this.cache.get(url), search, { key: searchProperty }) + : this.cache.get(url).slice(0, MAX_LIST_SIZE); + + if (formatter) { + result = formatter(result); + } + + return result; + } +} diff --git a/spec/frontend/merge_requests/list/components/merge_requests_list_app_spec.js b/spec/frontend/merge_requests/list/components/merge_requests_list_app_spec.js index 6c9041cb719c516fc772f679c1fb657fc0705098..97416f3b47dc9985fd6bb710d470481512c836ca 100644 --- a/spec/frontend/merge_requests/list/components/merge_requests_list_app_spec.js +++ b/spec/frontend/merge_requests/list/components/merge_requests_list_app_spec.js @@ -1,6 +1,9 @@ +import AxiosMockAdapter from 'axios-mock-adapter'; import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import VueRouter from 'vue-router'; +import axios from '~/lib/utils/axios_utils'; +import { HTTP_STATUS_OK } from '~/lib/utils/http_status'; import { shallowMountExtended, mountExtended } from 'helpers/vue_test_utils_helper'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; @@ -36,6 +39,7 @@ import { import { mergeRequestListTabs } from '~/vue_shared/issuable/list/constants'; import { getSortOptions } from '~/issues/list/utils'; import MergeRequestsListApp from '~/merge_requests/list/components/merge_requests_list_app.vue'; +import { BRANCH_LIST_REFRESH_INTERVAL } from '~/merge_requests/list/constants'; import getMergeRequestsQuery from '~/merge_requests/list/queries/get_merge_requests.query.graphql'; import getMergeRequestsCountQuery from '~/merge_requests/list/queries/get_merge_requests_counts.query.graphql'; import IssuableList from '~/vue_shared/issuable/list/components/issuable_list_root.vue'; @@ -136,6 +140,130 @@ describe('Merge requests list app', () => { }); }); + describe('fetching branches', () => { + const apiVersion = 1; + const projectId = 2; + const fullPath = 'gitlab-org/gitlab'; + const allBranchesPath = `/api/${apiVersion}/projects/${encodeURIComponent(fullPath)}/repository/branches`; + const sourceBranchPath = `/-/autocomplete/merge_request_source_branches.json?project_id=${projectId}`; + const targetBranchPath = `/-/autocomplete/merge_request_target_branches.json?project_id=${projectId}`; + let axiosMock; + + beforeEach(() => { + axiosMock = new AxiosMockAdapter(axios); + + window.gon = { api_version: apiVersion }; + axiosMock.onGet().reply(HTTP_STATUS_OK, []); + }); + + describe('with no projectId', () => { + it('uses the generic "all branches" endpoint', async () => { + const queryResponse = getQueryResponse; + queryResponse.data.project.id = null; + + createComponent({ response: queryResponse }); + + await waitForPromises(); + await wrapper.vm.fetchBranches(); + + expect(axiosMock.history.get[0].url).toBe(allBranchesPath); + }); + }); + + describe('with projectId', () => { + it.each` + branchPath | fetchArgs + ${targetBranchPath} | ${['target']} + ${sourceBranchPath} | ${['source']} + ${allBranchesPath} | ${['']} + `( + 'selects the correct path ($branchPath) given the arguments $fetchArgs', + async ({ branchPath, fetchArgs }) => { + const queryResponse = getQueryResponse; + queryResponse.data.project.id = projectId; + + createComponent({ response: queryResponse }); + await waitForPromises(); + + await wrapper.vm.fetchBranches(...fetchArgs); + + expect(axiosMock.history.get[0].url).toBe(branchPath); + }, + ); + }); + + describe('cache expiration', () => { + const queryResponse = getQueryResponse; + + queryResponse.data.project.id = projectId; + + beforeEach(() => { + axiosMock.resetHistory(); + + createComponent(); + + return waitForPromises(); + }); + + it('has no cache ages normally', () => { + expect(wrapper.vm.branchCacheAges).toEqual({}); + }); + + it('does not try to refresh the cache on the very first attempt for a branch type', async () => { + const updateSpy = jest.spyOn(wrapper.vm.autocompleteCache, 'updateLocalCache'); + + await wrapper.vm.fetchBranches('target'); + // This call happens internally in the AutocompleteCache .fetch only when it is first set up + expect(updateSpy).toHaveBeenCalledTimes(1); + + await jest.advanceTimersByTime(BRANCH_LIST_REFRESH_INTERVAL); + + await wrapper.vm.fetchBranches('target'); + // Now the MR List app attempts to refresh ahead of time, because the cache has expired + // (Note the AutocompleteCache does not, since it doesn't expire caches at all internally) + expect(updateSpy).toHaveBeenCalledTimes(2); + }); + + it.each` + type + ${'source'} + ${'target'} + ${'other'} + `('only sets the cache age for the type of branch request ($type)', async ({ type }) => { + await wrapper.vm.fetchBranches(type); + + expect(wrapper.vm.branchCacheAges).toEqual({ [type]: expect.any(Number) }); + }); + + it('only requests fresh data if the cache has become stale', async () => { + // Prime the target cache + await wrapper.vm.fetchBranches('target'); + expect(axiosMock.history.get.length).toBe(1); + + jest.advanceTimersByTime(1000); + + // Only load from the cache since it has not expired yet + await wrapper.vm.fetchBranches('target'); + expect(axiosMock.history.get.length).toBe(1); + + jest.advanceTimersByTime(BRANCH_LIST_REFRESH_INTERVAL); + + // Refresh the cache since the expiration date has passed + await wrapper.vm.fetchBranches('target'); + expect(axiosMock.history.get.length).toBe(2); + }); + }); + + it('uses the AutocompleteCache', async () => { + createComponent(); + const fetchSpy = jest.spyOn(wrapper.vm.autocompleteCache, 'fetch'); + + await wrapper.vm.fetchBranches(); + + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + }); + describe('tokens', () => { const mockCurrentUser = { id: 1, diff --git a/spec/frontend/merge_requests/utils/autocomplete_cache_spec.js b/spec/frontend/merge_requests/utils/autocomplete_cache_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..1eae23c4cc92649955a767f473b374bba0c84eb4 --- /dev/null +++ b/spec/frontend/merge_requests/utils/autocomplete_cache_spec.js @@ -0,0 +1,170 @@ +import AxiosMockAdapter from 'axios-mock-adapter'; +import fuzzaldrinPlus from 'fuzzaldrin-plus'; +import { AutocompleteCache } from '~/merge_requests/utils/autocomplete_cache'; +import { MAX_LIST_SIZE } from '~/merge_requests/constants'; +import axios from '~/lib/utils/axios_utils'; +import { HTTP_STATUS_OK } from '~/lib/utils/http_status'; + +function mutator(data) { + return data.map((entry, index) => ({ ...entry, id: index })); +} + +function formatter(data) { + return { + data, + }; +} + +describe('AutocompleteCache', () => { + const searchProperty = 'property'; + const url = 'url'; + const data = [ + { [searchProperty]: 'one' }, + { [searchProperty]: 'two' }, + { [searchProperty]: 'three' }, + { [searchProperty]: 'four' }, + { [searchProperty]: 'five' }, + { [searchProperty]: 'six' }, + { [searchProperty]: 'seven' }, + { [searchProperty]: 'eight' }, + { [searchProperty]: 'nine' }, + { [searchProperty]: 'ten' }, + { [searchProperty]: 'eleven' }, + { [searchProperty]: 'twelve' }, + { [searchProperty]: 'thirteen' }, + { [searchProperty]: 'fourteen' }, + { [searchProperty]: 'fifteen' }, + ]; + let autocompleteCache; + let axiosMock; + + beforeEach(() => { + autocompleteCache = new AutocompleteCache(); + axiosMock = new AxiosMockAdapter(axios); + }); + + afterEach(() => { + axiosMock.reset(); + }); + + describe('when there is no cached data', () => { + let response; + + beforeEach(async () => { + axiosMock.onGet(url).replyOnce(HTTP_STATUS_OK, data); + response = await autocompleteCache.fetch({ url, searchProperty }); + }); + + it('fetches items via the API', () => { + expect(axiosMock.history.get[0].url).toBe(url); + }); + + it(`returns a maximum of ${MAX_LIST_SIZE} items`, () => { + expect(response).toHaveLength(MAX_LIST_SIZE); + }); + }); + + describe('when there is cached data', () => { + let response; + + beforeEach(async () => { + axiosMock.onGet(url).replyOnce(HTTP_STATUS_OK, data); + jest.spyOn(fuzzaldrinPlus, 'filter'); + // Populate cache + await autocompleteCache.fetch({ url, searchProperty }); + // Execute filtering on cache data + response = await autocompleteCache.fetch({ url, searchProperty, search: 'een' }); + }); + + it('returns filtered items based on search characters', () => { + expect(response).toEqual([ + { [searchProperty]: 'fifteen' }, + { [searchProperty]: 'thirteen' }, + { [searchProperty]: 'fourteen' }, + { [searchProperty]: 'eleven' }, + { [searchProperty]: 'seven' }, + ]); + }); + + it('filters using fuzzaldrinPlus', () => { + expect(fuzzaldrinPlus.filter).toHaveBeenCalled(); + }); + + it('does not call the API', () => { + expect(axiosMock.history.get[1]).toBeUndefined(); + }); + }); + + describe('refreshing the local cache', () => { + const updatedData = [ + { [searchProperty]: 'one' }, + { [searchProperty]: 'two' }, + { [searchProperty]: 'three' }, + ]; + + beforeEach(async () => { + axiosMock.onGet(url).replyOnce(HTTP_STATUS_OK, data); + // Populate cache + await autocompleteCache.fetch({ url, searchProperty }); + // Reduced entries later... + axiosMock.onGet(url).replyOnce(HTTP_STATUS_OK, updatedData); + }); + + it('overwrites the cache with the new data from the endpoint', async () => { + // Initially confirm the cache was hydrated + expect(autocompleteCache.cache.get(url).length).toBe(data.length); + + await autocompleteCache.updateLocalCache(url); + + expect(autocompleteCache.cache.get(url).length).toBe(updatedData.length); + }); + }); + + describe('mutators', () => { + beforeEach(() => { + axiosMock.onGet(url).replyOnce(HTTP_STATUS_OK, data); + }); + + it('does not touch the data if no mutator is provided', async () => { + await autocompleteCache.updateLocalCache(url); + + expect(autocompleteCache.cache.get(url)).toBe(data); + }); + + it('modifies data before storing it if a mutator is provided', async () => { + autocompleteCache.setUpCache({ url, mutator }); + + await autocompleteCache.updateLocalCache(url); + + expect(autocompleteCache.cache.get(url)).not.toBe(data); + expect(autocompleteCache.cache.get(url)[0]).toEqual({ ...data[0], id: 0 }); + }); + }); + + describe('formatters', () => { + const expectedOutput = data.slice(0, MAX_LIST_SIZE); + + beforeEach(async () => { + axiosMock.onGet(url).replyOnce(HTTP_STATUS_OK, data); + + await autocompleteCache.updateLocalCache(url); + }); + + it('returns the data directly from the cache if no formatter is provided', () => { + expect(autocompleteCache.retrieveFromLocalCache(url)).toEqual(expectedOutput); + }); + + it('modifies the data before returning it if a formatter is provided', () => { + autocompleteCache.setUpCache({ url, formatter }); + + expect(autocompleteCache.retrieveFromLocalCache(url)).toEqual({ data: expectedOutput }); + }); + + it('does not modify the source (cached) data at all if a formatter is provided', () => { + autocompleteCache.setUpCache({ url, formatter }); + autocompleteCache.retrieveFromLocalCache(url); + + expect(autocompleteCache.cache.get(url)).toBe(data); + }); + }); +});