Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix cv-deep-copy by removing express request #1993

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 15 additions & 18 deletions lib/models/services/build-service.js
Original file line number Diff line number Diff line change
Expand Up @@ -470,27 +470,24 @@ BuildService.createNewContextVersion = function (instance, pushInfo) {
if (!contextOwnerGithubId) {
throw new Error('createNewContextVersion requires the context to have an owner')
}
return Promise.fromCallback(function (callback) {
var user = {
accounts: {
github: {
id: pushInfoGithubId
}
var user = {
accounts: {
github: {
id: pushInfoGithubId
}
}
var opts = {
owner: {
github: contextOwnerGithubId
}
}
var opts = {
owner: {
github: contextOwnerGithubId
}
ContextVersionService.handleVersionDeepCopy(
context,
contextVersion,
user,
opts,
callback
)
})
}
return ContextVersionService.handleVersionDeepCopy(
context,
contextVersion,
user,
opts
)
})
.then(function (newContextVersion) {
return ContextVersion.modifyAppCodeVersionByRepoAsync(
Expand Down
74 changes: 31 additions & 43 deletions lib/models/services/context-version-service.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
'use strict'

const async = require('async')
const Boom = require('dat-middleware').Boom
const isFunction = require('101/is-function')
const keypather = require('keypather')()
const uuid = require('uuid')

Expand Down Expand Up @@ -47,59 +45,49 @@ ContextVersionService.findContextVersion = function (id) {
* the owner with the user provided (session user), creating a new context and
* copying the infracode files as well. Otherwise, it just creates a new context
* version.
* @param {Object} context Context object that 'owns' the contextVersion.
* @param {Object} contextVersion Context Version to copy.
* @param {Object} user User object (most likely sessionUser).
* @param {Object} [opts] Opts to allow overrides for the owner.
* @param {Object} [opts.owner] Owner override
* @param {Number} [opts.owner.github] Github ID of Owner with which to override.
* @param {Function} cb Callback which returns (err, newContextVersion)
* @param {Context} context - Context object that 'owns' the contextVersion.
* @param {ContextVersion} contextVersion - Context Version to copy.
* @param {SessionUser} user - User object (most likely sessionUser).
* @param {Object} opts - Opts to allow overrides for the owner.
* @param {Object} opts.owner - Owner override
* @param {Number} opts.owner.github - Github ID of Owner with which to override.
*
* @resolves {ContextVersion} Newly forked ContextVersion
*
*/
ContextVersionService.handleVersionDeepCopy = function (context, contextVersion, user, opts, cb) {
ContextVersionService.handleVersionDeepCopy = function (context, contextVersion, user, opts) {
const log = this.logger.child({
contextId: context._id,
contextVersionId: contextVersion._id,
method: 'handleVersionDeepCopy'
})
log.info('called')
if (isFunction(opts)) {
cb = opts
if (!opts) {
opts = {}
}
const contextOwnerId = keypather.get(context, 'owner.github')
const userGithubId = keypather.get(user, 'accounts.github.id')
// 1. deep copy contextVersion
ContextVersion.createDeepCopy(user, contextVersion, function (err, newContextVersion) {
if (err) { return cb(err) }
// check if the context version is (owned by hellorunnable AND the user isn't hellorunnable)
if (contextOwnerId !== process.env.HELLO_RUNNABLE_GITHUB_ID || userGithubId === contextOwnerId) {
return cb(null, newContextVersion)
}
// 2. create new context
const ownerGithubId = keypather.get(opts, 'owner.github') || userGithubId
const newContext = new Context({
name: uuid(),
owner: { github: ownerGithubId }
})
// 3. 'move' new contextVerion -> new context
newContextVersion.context = newContext._id

// 4. update the owner of the contextVersion
newContextVersion.owner.github = ownerGithubId

async.series([
// 4.1. save context, version
newContext.save.bind(newContext),
newContextVersion.save.bind(newContextVersion),
function (cb) {
// 5. copy icv to the new cv
InfraCodeVersionService.copyInfraCodeToContextVersion(newContextVersion, contextVersion.infraCodeVersion._id).asCallback(cb)
return ContextVersion.createDeepCopyAsync(user, contextVersion)
.tap(newContextVersion => {
// check if the context version is (owned by hellorunnable AND the user isn't hellorunnable)
if (contextOwnerId !== process.env.HELLO_RUNNABLE_GITHUB_ID || userGithubId === contextOwnerId) {
return newContextVersion
}
], function (err, results) {
// [1]: newContextVersion.save results
// [1][0]: newContextVersion.save document
// [1][1]: newContextVersion.save number affected
cb(err, keypather.get(results, '[1][0]'))
// 2. create new context
const ownerGithubObject = {
github: keypather.get(opts, 'owner.github') || userGithubId
}
newContextVersion.set('owner', ownerGithubObject)
const newContext = new Context({
name: uuid(),
owner: ownerGithubObject
})
return newContext.saveAsync()
.tap(newContext => newContextVersion.set('context', newContext._id))
.then(() => newContextVersion.saveAsync())
.then(() =>
InfraCodeVersionService.copyInfraCodeToContextVersion(newContextVersion, contextVersion.infraCodeVersion)
)
})
})
}
15 changes: 6 additions & 9 deletions lib/models/services/instance-fork-service.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,15 +286,12 @@ InstanceForkService._createNewNonRepoContextVersion = function (contextVersion,
.then(function (context) {
var user = { accounts: { github: { id: createdById } } }
var opts = { owner: { github: ownerId } }
return Promise.fromCallback(function (callback) {
ContextVersionService.handleVersionDeepCopy(
context,
contextVersion,
user,
opts,
callback
)
})
return ContextVersionService.handleVersionDeepCopy(
context,
contextVersion,
user,
opts
)
})
.then(function (newContextVersion) {
// non-repo context versions _must_ have advanced: true set.
Expand Down
71 changes: 5 additions & 66 deletions lib/routes/contexts/versions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@

const assign = require('101/assign')
const express = require('express')
const flow = require('middleware-flow')
const keypather = require('keypather')()
const mw = require('dat-middleware')
const uuid = require('uuid')

const Build = require('models/mongo/build')
const ContextService = require('models/services/context-service')
Expand All @@ -15,7 +13,6 @@ const InfraCodeVersion = require('models/mongo/infra-code-version')
const InfraCodeVersionService = require('models/services/infracode-version-service')
const Instance = require('models/mongo/instance')
const PermissionService = require('models/services/permission-service')
const Runnable = require('models/apis/runnable')
const transformations = require('middlewares/transformations')
const validations = require('middlewares/validations')

Expand Down Expand Up @@ -243,73 +240,15 @@ app.post('/contexts/:contextId/versions/:id/actions/copy',
findContextVersion,
// only supports deep copy for now
mw.query('deep').require().validate(validations.equals('true')),

mw.req('context.owner.github').require()
.validate(validations.equals(process.env.HELLO_RUNNABLE_GITHUB_ID))
.validate(validations.notEqualsKeypath('sessionUser.accounts.github.id'))
.then(
// if the build owner is hello-runnable and user is not hello-runnable
deepCopyCvFromHelloRunnable('contextVersion')
).else(
function (req, res, next) {
ContextVersion.createDeepCopyAsync(req.sessionUser, req.contextVersion)
.tap(function (contextVersion) {
req.contextVersion = contextVersion
})
.asCallback(next)
}
),
mw.res.status(201),
mw.res.json('contextVersion'))

function deepCopyCvFromHelloRunnable (cvKey) {
return flow.series(
// maybe error if the contextVersion has acvs?
// make a new context
mw.req('contextVersion', cvKey),
// create context requires name
function (req, res, next) {
req.body.name = uuid()
ContextService.createNew(req.sessionUser, req.body)
.then(function (context) {
req.newContext = context
next()
})
.catch(next)
},
// make a new context-version
function (req, res, next) {
var runnable = new Runnable(req.headers)
runnable.createContextVersion(
keypather.get(req, 'newContext._id'),
function (err, result) {
if (err) {
return next(err)
}
req.newContextVersion = result
next()
})
},
// use infracodeversion copy route to copy the files
function (req, res, next) {
var runnable = new Runnable(req.headers)
runnable.copyVersionIcvFiles(
keypather.get(req, 'newContext._id'),
keypather.get(req, 'newContextVersion._id'),
keypather.get(req, 'contextVersion.infraCodeVersion'),
next)
},
function (req, res, next) {
ContextVersionService.findContextVersion(keypather.get(req, 'newContextVersion._id'))
function (req, res, next) {
ContextVersionService.handleVersionDeepCopy(req.context, req.contextVersion, req.sessionUser, req.body)
.tap(function (contextVersion) {
req.contextVersion = contextVersion
res.json(201, contextVersion.toJSON())
})
.asCallback(function (err) {
.catch(function (err) {
next(err)
})
}
)
}
})

/**
* We're assuming this is only called when reverting files back to source
Expand Down
7 changes: 3 additions & 4 deletions unit/models/services/build-service.js
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ describe('BuildService', function () {
_id: 21
}
sinon.stub(Context, 'findOne').yieldsAsync(null, mockContext)
sinon.stub(ContextVersionService, 'handleVersionDeepCopy').yieldsAsync(null, mockContextVersion)
sinon.stub(ContextVersionService, 'handleVersionDeepCopy').resolves(mockContextVersion)
sinon.stub(ContextVersion, 'modifyAppCodeVersionByRepo').yieldsAsync(null, mockContextVersion)
done()
})
Expand Down Expand Up @@ -847,7 +847,7 @@ describe('BuildService', function () {
describe('in ContextVersionService.handleVersionDeepCopy', function () {
beforeEach(function (done) {
error = new Error('robot')
ContextVersionService.handleVersionDeepCopy.yieldsAsync(error)
ContextVersionService.handleVersionDeepCopy.rejects(error)
done()
})

Expand Down Expand Up @@ -913,8 +913,7 @@ describe('BuildService', function () {
mockContext, // returned from `findOne`
contextVersion, // from the Instance
{ accounts: { github: { id: 7 } } }, // from pushInfo (like sessionUser)
{ owner: { github: 14 } }, // from mockContext.owner.github (owner object)
sinon.match.func
{ owner: { github: 14 } } // from mockContext.owner.github (owner object)
)
sinon.assert.calledOnce(ContextVersion.modifyAppCodeVersionByRepo)
sinon.assert.calledWithExactly(
Expand Down
Loading