Commit 91a808af authored by Frédéric Caplette's avatar Frédéric Caplette Committed by Miguel Rincon

Fix sending BlobContent query with empty variables

In the pipeline editor, we make an initial query to get
the BlobContent, and then we parse this content through
the configData query. However, to make that initial query,
we rely on 2 client side queries for the branchName and the
ref. We have to wait for both of these query to be resolved
before we can fetch the blob, otherwise we send malformed
requests.

Changelog: fixed
parent 6b33b67f
...@@ -69,9 +69,10 @@ export default { ...@@ -69,9 +69,10 @@ export default {
// If it's a brand new file, we don't want to fetch the content. // If it's a brand new file, we don't want to fetch the content.
// Then when the user commits the first time, the query would run // Then when the user commits the first time, the query would run
// to get the initial file content, but we already have it in `lastCommitedContent` // to get the initial file content, but we already have it in `lastCommitedContent`
// so we skip the loading altogether. // so we skip the loading altogether. We also wait for the currentBranch
skip({ isNewCiConfigFile, lastCommittedContent }) { // to have been fetched
return isNewCiConfigFile || lastCommittedContent; skip() {
return this.shouldSkipBlobContentQuery;
}, },
variables() { variables() {
return { return {
...@@ -128,8 +129,8 @@ export default { ...@@ -128,8 +129,8 @@ export default {
}, },
ciConfigData: { ciConfigData: {
query: getCiConfigData, query: getCiConfigData,
skip({ currentCiFileContent }) { skip() {
return !currentCiFileContent; return this.shouldSkipCiConfigQuery;
}, },
variables() { variables() {
return { return {
...@@ -174,6 +175,9 @@ export default { ...@@ -174,6 +175,9 @@ export default {
}, },
commitSha: { commitSha: {
query: getLatestCommitShaQuery, query: getLatestCommitShaQuery,
skip({ currentBranch }) {
return !currentBranch;
},
variables() { variables() {
return { return {
projectPath: this.projectFullPath, projectPath: this.projectFullPath,
...@@ -181,7 +185,7 @@ export default { ...@@ -181,7 +185,7 @@ export default {
}; };
}, },
update(data) { update(data) {
const latestCommitSha = data.project?.repository?.tree?.lastCommit?.sha; const latestCommitSha = data?.project?.repository?.tree?.lastCommit?.sha;
if (this.isFetchingCommitSha && latestCommitSha === this.commitSha) { if (this.isFetchingCommitSha && latestCommitSha === this.commitSha) {
this.$apollo.queries.commitSha.startPolling(COMMIT_SHA_POLL_INTERVAL); this.$apollo.queries.commitSha.startPolling(COMMIT_SHA_POLL_INTERVAL);
...@@ -192,6 +196,9 @@ export default { ...@@ -192,6 +196,9 @@ export default {
this.$apollo.queries.commitSha.stopPolling(); this.$apollo.queries.commitSha.stopPolling();
return latestCommitSha; return latestCommitSha;
}, },
error() {
this.reportFailure(LOAD_FAILURE_UNKNOWN);
},
}, },
currentBranch: { currentBranch: {
query: getCurrentBranch, query: getCurrentBranch,
...@@ -234,6 +241,12 @@ export default { ...@@ -234,6 +241,12 @@ export default {
isEmpty() { isEmpty() {
return this.currentCiFileContent === ''; return this.currentCiFileContent === '';
}, },
shouldSkipBlobContentQuery() {
return this.isNewCiConfigFile || this.lastCommittedContent || !this.currentBranch;
},
shouldSkipCiConfigQuery() {
return !this.currentCiFileContent || !this.commitSha;
},
}, },
i18n: { i18n: {
resetModal: { resetModal: {
......
...@@ -18,12 +18,15 @@ import { ...@@ -18,12 +18,15 @@ import {
COMMIT_SUCCESS, COMMIT_SUCCESS,
COMMIT_SUCCESS_WITH_REDIRECT, COMMIT_SUCCESS_WITH_REDIRECT,
COMMIT_FAILURE, COMMIT_FAILURE,
EDITOR_APP_STATUS_LOADING,
} from '~/pipeline_editor/constants'; } from '~/pipeline_editor/constants';
import getBlobContent from '~/pipeline_editor/graphql/queries/blob_content.query.graphql'; import getBlobContent from '~/pipeline_editor/graphql/queries/blob_content.query.graphql';
import getCiConfigData from '~/pipeline_editor/graphql/queries/ci_config.query.graphql'; import getCiConfigData from '~/pipeline_editor/graphql/queries/ci_config.query.graphql';
import getTemplate from '~/pipeline_editor/graphql/queries/get_starter_template.query.graphql'; import getTemplate from '~/pipeline_editor/graphql/queries/get_starter_template.query.graphql';
import getLatestCommitShaQuery from '~/pipeline_editor/graphql/queries/latest_commit_sha.query.graphql'; import getLatestCommitShaQuery from '~/pipeline_editor/graphql/queries/latest_commit_sha.query.graphql';
import getPipelineQuery from '~/pipeline_editor/graphql/queries/pipeline.query.graphql'; import getPipelineQuery from '~/pipeline_editor/graphql/queries/pipeline.query.graphql';
import getCurrentBranch from '~/pipeline_editor/graphql/queries/client/current_branch.query.graphql';
import getAppStatus from '~/pipeline_editor/graphql/queries/client/app_status.query.graphql';
import PipelineEditorApp from '~/pipeline_editor/pipeline_editor_app.vue'; import PipelineEditorApp from '~/pipeline_editor/pipeline_editor_app.vue';
import PipelineEditorHome from '~/pipeline_editor/pipeline_editor_home.vue'; import PipelineEditorHome from '~/pipeline_editor/pipeline_editor_home.vue';
...@@ -84,9 +87,6 @@ describe('Pipeline editor app component', () => { ...@@ -84,9 +87,6 @@ describe('Pipeline editor app component', () => {
initialCiFileContent: { initialCiFileContent: {
loading: blobLoading, loading: blobLoading,
}, },
ciConfigData: {
loading: false,
},
}, },
}, },
}, },
...@@ -94,7 +94,11 @@ describe('Pipeline editor app component', () => { ...@@ -94,7 +94,11 @@ describe('Pipeline editor app component', () => {
}); });
}; };
const createComponentWithApollo = async ({ provide = {}, stubs = {} } = {}) => { const createComponentWithApollo = async ({
provide = {},
stubs = {},
withUndefinedBranch = false,
} = {}) => {
const handlers = [ const handlers = [
[getBlobContent, mockBlobContentData], [getBlobContent, mockBlobContentData],
[getCiConfigData, mockCiConfigData], [getCiConfigData, mockCiConfigData],
...@@ -105,6 +109,31 @@ describe('Pipeline editor app component', () => { ...@@ -105,6 +109,31 @@ describe('Pipeline editor app component', () => {
mockApollo = createMockApollo(handlers, resolvers); mockApollo = createMockApollo(handlers, resolvers);
if (!withUndefinedBranch) {
mockApollo.clients.defaultClient.cache.writeQuery({
query: getCurrentBranch,
data: {
workBranches: {
__typename: 'BranchList',
current: {
__typename: 'WorkBranch',
name: mockDefaultBranch,
},
},
},
});
}
mockApollo.clients.defaultClient.cache.writeQuery({
query: getAppStatus,
data: {
app: {
__typename: 'AppData',
status: EDITOR_APP_STATUS_LOADING,
},
},
});
const options = { const options = {
localVue, localVue,
mocks: {}, mocks: {},
...@@ -145,6 +174,55 @@ describe('Pipeline editor app component', () => { ...@@ -145,6 +174,55 @@ describe('Pipeline editor app component', () => {
}); });
}); });
describe('skipping queries', () => {
describe('when branchName is undefined', () => {
beforeEach(async () => {
await createComponentWithApollo({ withUndefinedBranch: true });
});
it('does not calls getBlobContent', () => {
expect(mockBlobContentData).not.toHaveBeenCalled();
});
});
describe('when branchName is defined', () => {
beforeEach(async () => {
await createComponentWithApollo();
});
it('calls getBlobContent', () => {
expect(mockBlobContentData).toHaveBeenCalled();
});
});
describe('when commit sha is undefined', () => {
beforeEach(async () => {
mockLatestCommitShaQuery.mockResolvedValue(undefined);
await createComponentWithApollo();
});
it('calls getBlobContent', () => {
expect(mockBlobContentData).toHaveBeenCalled();
});
it('does not call ciConfigData', () => {
expect(mockCiConfigData).not.toHaveBeenCalled();
});
});
describe('when commit sha is defined', () => {
beforeEach(async () => {
mockBlobContentData.mockResolvedValue(mockBlobContentQueryResponse);
mockLatestCommitShaQuery.mockResolvedValue(mockCommitShaResults);
await createComponentWithApollo();
});
it('calls ciConfigData', () => {
expect(mockCiConfigData).toHaveBeenCalled();
});
});
});
describe('when queries are called', () => { describe('when queries are called', () => {
beforeEach(() => { beforeEach(() => {
mockBlobContentData.mockResolvedValue(mockBlobContentQueryResponse); mockBlobContentData.mockResolvedValue(mockBlobContentQueryResponse);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment