Skip to content

Commit

Permalink
fetchRawDefinition gets version support at adapter layer
Browse files Browse the repository at this point in the history
  • Loading branch information
philrenaud committed Nov 25, 2024
1 parent 18b2bd3 commit 34a566a
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 12 deletions.
24 changes: 21 additions & 3 deletions ui/app/adapters/job.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,28 @@ export default class JobAdapter extends WatchableNamespaceIDs {
* This method is still important for backwards compatibility with older job versions, as well as a fallback
* for when fetching raw specification fails.
* @param {import('../models/job').default} job
* @param {number} version
*/
fetchRawDefinition(job) {
const url = this.urlForFindRecord(job.get('id'), 'job');
return this.ajax(url, 'GET');
async fetchRawDefinition(job, version) {
if (version == null) {
const url = this.urlForFindRecord(job.get('id'), 'job');
return this.ajax(url, 'GET');
}

// For specific versions, we need to fetch from versions endpoint,
// and then find the specified version info from the response.
const versionsUrl = addToPath(
this.urlForFindRecord(job.get('id'), 'job', null, 'versions')
);

const response = await this.ajax(versionsUrl, 'GET');
const versionInfo = response.Versions.find((v) => v.Version === version);

if (!versionInfo) {
throw new Error(`Version ${version} not found`);
}

return versionInfo;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions ui/app/models/job.js
Original file line number Diff line number Diff line change
Expand Up @@ -537,8 +537,8 @@ export default class Job extends Model {
return undefined;
}

fetchRawDefinition() {
return this.store.adapterFor('job').fetchRawDefinition(this);
fetchRawDefinition(version) {
return this.store.adapterFor('job').fetchRawDefinition(this, version);
}

fetchRawSpecification(version) {
Expand Down
7 changes: 1 addition & 6 deletions ui/app/routes/jobs/job/definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,7 @@ export default class DefinitionRoute extends Route {
if (version !== undefined) {
// Not (!version) because version can be 0
try {
const versionResponse = await job.getVersions();
const versions = versionResponse.Versions;
definition = versions.findBy('Version', version);
if (!definition) {
throw new Error('Version not found');
}
definition = await job.fetchRawDefinition(version);
} catch (e) {
console.error('error fetching job version definition', e);
this.notifications.add({
Expand Down
61 changes: 60 additions & 1 deletion ui/tests/unit/adapters/job-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,64 @@ module('Unit | Adapter | Job', function (hooks) {
assert.equal(request.method, 'GET');
});

// TODO: test that version requests work for fetchRawDefinition
test('fetchRawDefinition handles version requests', async function (assert) {
assert.expect(5);

const adapter = this.owner.lookup('adapter:job');
const job = {
get: sinon.stub(),
};

job.get.withArgs('id').returns('["job-id"]');

const mockVersionResponse = {
Versions: [
{ Version: 1, JobID: 'job-id', JobModifyIndex: 100 },
{ Version: 2, JobID: 'job-id', JobModifyIndex: 200 },
],
};

// Stub ajax to return mock version data
const ajaxStub = sinon.stub(adapter, 'ajax');
ajaxStub
.withArgs('/v1/job/job-id/versions', 'GET')
.resolves(mockVersionResponse);

// Test fetching specific version
const result = await adapter.fetchRawDefinition(job, 2);
assert.equal(result.Version, 2, 'Returns correct version');
assert.equal(result.JobModifyIndex, 200, 'Returns full version info');

// Test version not found
try {
await adapter.fetchRawDefinition(job, 999);
assert.ok(false, 'Should have thrown error');
} catch (e) {
assert.equal(
e.message,
'Version 999 not found',
'Throws appropriate error'
);
}

// Test no version specified (current version)
ajaxStub
.withArgs('/v1/job/job-id', 'GET')
.resolves({ ID: 'job-id', Version: 2 });

const currentResult = await adapter.fetchRawDefinition(job);

assert.equal(
ajaxStub.lastCall.args[0],
'/v1/job/job-id',
'URL has no version query param'
);
assert.equal(
currentResult.Version,
2,
'Returns current version when no version specified'
);
});

test('forcePeriodic requests include the activeRegion', async function (assert) {
const region = 'region-2';
Expand Down Expand Up @@ -688,6 +745,8 @@ module('Unit | Adapter | Job', function (hooks) {
const job = {
get: sinon.stub(),
};
job.get.withArgs('id').returns('["job-id"]');
job.get.withArgs('version').returns(100);

// Stub the ajax method to avoid making real API calls
sinon.stub(adapter, 'ajax').callsFake(() => resolve({}));
Expand Down

0 comments on commit 34a566a

Please sign in to comment.