Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Preserve App, Package and Directory addresses #120

Merged
merged 7 commits into from
Sep 25, 2018
Merged
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
2 changes: 1 addition & 1 deletion examples/lib-complex/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ async function setupApp(txParams) {
// On-chain, single entry point of the entire application.
log.info(`<< Setting up App >> network: ${network}`)
const initialVersion = '0.0.1'
return await AppProject.deploy('complex-example', initialVersion, txParams)
return await AppProject.fetchOrDeploy('complex-example', initialVersion, txParams, {})
}

async function deployVersion1(project, owner) {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/models/dependency/Dependency.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export default class Dependency {

async deploy(txParams) {
const version = semver.coerce(this.version).toString()
const project = await LibProject.deploy(version, txParams)
const project = await LibProject.fetchOrDeploy(version, txParams, {})
await Promise.all(
_.map(this.getPackageFile().contracts, (contractName, contractAlias) => {
const contractClass = Contracts.getFromNodeModules(this.name, contractName)
Expand Down
40 changes: 20 additions & 20 deletions packages/cli/src/models/network/NetworkAppController.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ import { allPromisesOrError } from '../../utils/async';
const log = new Logger('NetworkAppController');

export default class NetworkAppController extends NetworkBaseController {
get isDeployed() {
return !!this.appAddress;
}

get appAddress() {
return this.networkFile.appAddress
}
Expand All @@ -20,22 +16,26 @@ export default class NetworkAppController extends NetworkBaseController {
return this.project.getApp()
}

async deploy() {
this.project = await AppProject.deploy(this.packageFile.name, this.currentVersion, this.txParams);

const app = this.project.getApp()
this.networkFile.app = { address: app.address };

const projectPackage = await this.project.getProjectPackage()
this.networkFile.package = { address: projectPackage.address };
async fetchOrDeploy() {
try {
const { appAddress, packageAddress } = this

this.project = await AppProject.fetchOrDeploy(this.packageFile.name, this.currentVersion, this.txParams, { appAddress, packageAddress })
this._registerApp(this.project.getApp())
this._registerPackage(await this.project.getProjectPackage())
this._registerVersion(this.currentVersion, await this.project.getCurrentDirectory())
} catch (deployError) {
this._tryRegisterPartialDeploy(deployError)
}
}

const directory = await this.project.getCurrentDirectory()
this._registerVersion(this.currentVersion, directory.address);
_tryRegisterPartialDeploy({ thepackage, app, directory }) {
super._tryRegisterPartialDeploy({ thepackage, directory })
if (app) this._registerApp(app)
}

async fetch() {
if (!this.isDeployed) throw Error('Your application must be deployed to interact with it.');
this.project = await AppProject.fetch(this.appAddress, this.packageFile.name, this.txParams);
_registerApp({ address }) {
this.networkFile.app = { address }
}

async push(reupload = false) {
Expand Down Expand Up @@ -66,7 +66,7 @@ export default class NetworkAppController extends NetworkBaseController {
}

async createProxy(packageName, contractAlias, initMethod, initArgs) {
await this.fetch();
await this.fetchOrDeploy()
if (!packageName) packageName = this.packageFile.name;
const contractClass = this.localController.getContractClass(packageName, contractAlias);
this.checkInitialization(contractClass, initMethod, initArgs);
Expand Down Expand Up @@ -96,7 +96,7 @@ export default class NetworkAppController extends NetworkBaseController {
async setProxiesAdmin(packageName, contractAlias, proxyAddress, newAdmin) {
const proxies = this._fetchOwnedProxies(packageName, contractAlias, proxyAddress)
if (proxies.length === 0) return [];
await this.fetch();
await this.fetchOrDeploy()

await allPromisesOrError(
_.map(proxies, async (proxy) => {
Expand All @@ -111,7 +111,7 @@ export default class NetworkAppController extends NetworkBaseController {
async upgradeProxies(packageName, contractAlias, proxyAddress, initMethod, initArgs) {
const proxies = this._fetchOwnedProxies(packageName, contractAlias, proxyAddress)
if (proxies.length === 0) return [];
await this.fetch();
await this.fetchOrDeploy()

// Check if there is any migrate method in the contracts and warn the user to call it
const contracts = _.uniqWith(_.map(proxies, p => [p.package, p.contract]), _.isEqual)
Expand Down
30 changes: 15 additions & 15 deletions packages/cli/src/models/network/NetworkBaseController.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,38 +51,38 @@ export default class NetworkBaseController {
}

async push(reupload = false) {
if (this.isDeployed) {
await this.fetch();
await this.pushVersion();
} else {
await this.deploy();
}
this._checkVersion()
await this.fetchOrDeploy()

await Promise.all([
this.uploadContracts(reupload),
this.uploadContracts(reupload),
this.unsetContracts()
])
}

async pushVersion() {
_checkVersion() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

const requestedVersion = this.packageFile.version;
const currentVersion = this.networkFile.version;

if (requestedVersion !== currentVersion) {
log.info(`Current version ${currentVersion}`);
log.info(`Creating new version ${requestedVersion}`);
const provider = await this.newVersion(requestedVersion);
this.networkFile.contracts = {};
this._registerVersion(requestedVersion, provider.address);
}
}

_registerVersion(version, providerAddress) {
this.networkFile.provider = { address: providerAddress };
this.networkFile.version = version;
_tryRegisterPartialDeploy({ thepackage, directory }) {
if (thepackage) this._registerPackage(thepackage)
if (directory) this._registerVersion(this.currentVersion, directory)
}

_registerPackage({ address }) {
this.networkFile.package = { address }
}

async newVersion(versionName) {
return this.project.newVersion(versionName);
_registerVersion(version, { address }) {
this.networkFile.provider = { address }
this.networkFile.version = version
}

async uploadContracts(reupload) {
Expand Down
39 changes: 21 additions & 18 deletions packages/cli/src/models/network/NetworkLibController.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,37 @@ import { LibProject } from 'zos-lib';
import NetworkBaseController from './NetworkBaseController';

export default class NetworkLibController extends NetworkBaseController {
get isDeployed() {
return !!this.packageAddress;
}

async createProxy() {
throw Error('Cannot create proxy for library project')
}

async deploy() {
this.project = await LibProject.deploy(this.currentVersion, this.txParams)
const thepackage = await this.project.getProjectPackage()
this.networkFile.package = { address: thepackage.address }
const provider = await this.project.getCurrentDirectory();
this._registerVersion(this.currentVersion, provider.address)
}
async fetchOrDeploy() {
try {
const { packageAddress } = this

async fetch() {
if (!this.isDeployed) throw Error('Your application must be deployed to interact with it.');
this.project = await LibProject.fetch(this.packageAddress, this.currentVersion, this.txParams);
this.project = await LibProject.fetchOrDeploy(this.currentVersion, this.txParams, { packageAddress })
this._registerPackage(await this.project.getProjectPackage())
this._registerVersion(this.currentVersion, await this.project.getCurrentDirectory())
} catch(deployError) {
this._tryRegisterPartialDeploy(deployError)
}
}

async newVersion(versionName) {
this.networkFile.frozen = false
return super.newVersion(versionName)
_registerVersion(version, { address }) {
super._registerVersion(version, { address })
}

_checkVersion() {
const requestedVersion = this.packageFile.version
const currentVersion = this.networkFile.version

if (requestedVersion !== currentVersion) {
this.networkFile.frozen = false
}
super._checkVersion()
}
async freeze() {
await this.fetch()
await this.fetchOrDeploy()
await this.project.freeze()
this.networkFile.frozen = true
}
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/scripts/status.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ async function appInfo(controller) {
return false;
}

await controller.fetch();
await controller.fetchOrDeploy();
log.info(`Application is deployed at ${controller.appAddress}`);
log.info(`- Package ${controller.packageFile.name} is at ${controller.networkFile.packageAddress}`);
return true;
Expand All @@ -37,7 +37,7 @@ async function libInfo(controller) {
return false;
}

await controller.fetch();
await controller.fetchOrDeploy();
log.info(`Library package is deployed at ${controller.packageAddress}`);
return true;
}
Expand Down
5 changes: 3 additions & 2 deletions packages/cli/test/models/Dependency.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,18 @@ contract('Dependency', function() {
beforeEach(function() {
this.dependency = new Dependency('mock-stdlib', '1.1.0')
this.txParams = {}
this.addresses = {}
delete this.dependency._packageFile
})

describe('#deploy', function() {
it('deploys a dependency', function() {
const libDeployStub = sinon
.stub(LibProject, 'deploy')
.stub(LibProject, 'fetchOrDeploy')
.returns({ setImplementation: () => {} })
this.dependency.deploy(this.txParams)

libDeployStub.should.have.been.calledWithExactly('1.1.0', this.txParams)
libDeployStub.should.have.been.calledWithExactly('1.1.0', this.txParams, this.addresses)
sinon.restore()
})
})
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/scripts/push.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ contract('push script', function([_, owner]) {
address.should.be.nonzeroAddress;

const app = await App.fetch(address);
const hasPackage = await app.hasPackage(this.networkFile.packageFile.name)
const hasPackage = await app.hasPackage(this.networkFile.packageFile.name, defaultVersion)
hasPackage.should.be.true
});
};
Expand Down
4 changes: 2 additions & 2 deletions packages/lib/src/app/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ export default class App {
return { package: thepackage, version }
}

async hasPackage(name) {
async hasPackage(name, version = undefined) {
const [address, _version] = await this.appContract.getPackage(name)
return !isZeroAddress(address)
return !isZeroAddress(address) && _version === version
}

async setPackage(name, packageAddress, version) {
Expand Down
44 changes: 27 additions & 17 deletions packages/lib/src/project/AppProject.js
Original file line number Diff line number Diff line change
@@ -1,26 +1,36 @@
import BasePackageProject from "./BasePackageProject";
import App from "../app/App";
import Package from "../package/Package";
import { DeployError } from '../utils/errors/DeployError';
import _ from 'lodash';

export default class AppProject extends BasePackageProject {
static async fetch(appAddress, name, txParams) {
const app = await App.fetch(appAddress, txParams)
const packageInfo = await app.getPackage(name)
const project = new this(app, name, packageInfo.version, txParams)
project.package = packageInfo.package
return project
}

static async deploy(name = 'main', version = '0.1.0', txParams = {}) {
const thepackage = await Package.deploy(txParams)
const directory = await thepackage.newVersion(version)
const app = await App.deploy(txParams)
await app.setPackage(name, thepackage.address, version)
const project = new this(app, name, version, txParams)
project.directory = directory
project.package = thepackage
return project
static async fetchOrDeploy(name = 'main', version = '0.1.0', txParams = {}, { appAddress = undefined, packageAddress = undefined }) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need the default values for name?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say so, it simplifies usage of the Project class directly

let thepackage, directory, app
try {
app = appAddress
? await App.fetch(appAddress, txParams)
: await App.deploy(txParams)
if (packageAddress) {
thepackage = await Package.fetch(packageAddress, txParams)
} else if (await app.hasPackage(name, version)) {
thepackage = (await app.getPackage(name)).package
} else {
thepackage = await Package.deploy(txParams)
}
directory = await thepackage.hasVersion(version)
? await thepackage.getDirectory(version)
: await thepackage.newVersion(version)

if (!await app.hasPackage(name, version)) await app.setPackage(name, thepackage.address, version)
const project = new this(app, name, version, txParams)
project.directory = directory
project.package = thepackage

return project
} catch(deployError) {
throw new DeployError(deployError.message, { thepackage, directory, app })
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about splitting this method a bit to sth like:

  static async fetchOrDeploy(name = 'main', version = '0.1.0', txParams = {}, { appAddress = undefined, packageAddress = undefined }) {
    try {
      const app = await this.fetchOrDeployApp(...)
      const thepackage = await this.fetchOrDeployPackage(...)
      const directory = await this.fetchOrDeployDirectory(...)
      return this.buildProject(...)
    } catch(deployError) {
      throw new DeployError(deployError.message, { thepackage, directory, app })
    }
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've discussed this offline. It's not a good idea to merge deploy and fetch methods in Package and App, as it would add complexity.
On the other hand, separating the logic to new static methods inside AppProject/LibProject (or BasePackageProject) just to make the code prettier is not reason enough to do so. In case we would want to provide this methods through zos-lib, we can add them, but this exceeds this PR's scope.

}

constructor(app, name = 'main', version = '0.1.0', txParams = {}) {
Expand Down
24 changes: 18 additions & 6 deletions packages/lib/src/project/LibProject.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,30 @@
import BasePackageProject from "./BasePackageProject";
import Package from "../package/Package";
import { DeployError } from '../utils/errors/DeployError';

export default class LibProject extends BasePackageProject {
static async fetch(packageAddress, version = '0.1.0', txParams) {
const thepackage = await Package.fetch(packageAddress, txParams)
return new this(thepackage, version, txParams)
}

static async deploy(version = '0.1.0', txParams = {}) {
const thepackage = await Package.deploy(txParams)
const directory = await thepackage.newVersion(version)
const project = new this(thepackage, version, txParams)
project.directory = directory
return project
static async fetchOrDeploy(version = '0.1.0', txParams = {}, { packageAddress = undefined }) {
let thepackage, directory
try {
thepackage = packageAddress
? await Package.fetch(packageAddress, txParams)
: await Package.deploy(txParams)
directory = await thepackage.hasVersion(version)
? await thepackage.getDirectory(version)
: await thepackage.newVersion(version)

const project = new this(thepackage, version, txParams)
project.directory = directory

return project
} catch(deployError) {
throw new DeployError(deployError.message, { thepackage, directory })
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may want to split this one too, it is not that large tho

}

constructor(thepackage, version = '0.1.0', txParams = {}) {
Expand Down
9 changes: 9 additions & 0 deletions packages/lib/src/utils/errors/DeployError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
'use strict'

export class DeployError extends Error {
constructor(message, props) {
super(message)
Object.keys(props).forEach(prop => this[prop] = props[prop])
}
}

4 changes: 2 additions & 2 deletions packages/lib/test/src/app/App.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ contract('App', function (accounts) {
beforeEach('setting package', setPackage)

it('returns true if exists', async function () {
const hasPackage = await this.app.hasPackage(packageName)
const hasPackage = await this.app.hasPackage(packageName, version)
hasPackage.should.be.true
})

it('returns false if not exists', async function () {
const hasPackage = await this.app.hasPackage('NOTEXISTS')
const hasPackage = await this.app.hasPackage('NOTEXISTS', version)
hasPackage.should.be.false
})
})
Expand Down
Loading