8000 Bug fix/curb aardvark foxx cthulhu by pluma · Pull Request #9867 · arangodb/arangodb · GitHub
[go: up one dir, main page]
Skip to content

Bug fix/curb aardvark foxx cthulhu #9867

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Aug 30, 2019
Merged

Bug fix/curb aardvark foxx cthulhu #9867

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d894d40
Revert "fixed ui behaviour when replacing a foxx app"
pluma Aug 30, 2019
7fb8d3e
Cleanup on aisle 3
pluma Aug 30, 2019
a62aee6
Typo
pluma Aug 30, 2019
80a0d2b
Fixed param order
pluma Aug 30, 2019
51ca064
Cleanup flags
pluma Aug 30, 2019
17f10e2
Actually use the queryParams
pluma Aug 30, 2019
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
8 changes: 6 additions & 2 deletions js/apps/system/_admin/aardvark/APP/foxxes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,15 @@ foxxRouter.use(installer)
`)
.queryParam('upgrade', joi.boolean().default(false), dd`
Flag to upgrade the service installed at the mount point.
Triggers setup.
`)
.queryParam('replace', joi.boolean().default(false), dd`
Flag to replace the service installed at the mount point.
Triggers teardown and setup.
`)
.queryParam('setup', joi.boolean().default(true), dd`
Flag to run setup after install.
`)
.queryParam('teardown', joi.boolean().default(false), dd`
Flag to run teardown before replace/upgrade.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These apparently weren't actually implemented in aardvark before.

`);

installer.use(function (req, res, next) {
Expand Down
51 changes: 38 additions & 13 deletions js/apps/system/_admin/aardvark/APP/frontend/js/arango/arango.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1159,18 +1159,25 @@
});
},

getFoxxFlag: function () {
var flag;
getFoxxFlags: function () {
var flags = {};

if ($('#new-app-replace').prop('checked')) {
flag = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously the flag was apparently some kind of weird enum. We're now using an object with actual boolean flags.

} else {
if ($('#new-app-teardown').prop('checked')) {
flag = false;
}
const $replace = $('#new-app-flag-replace')[0];
if ($replace) {
flags.replace = Boolean($replace.checked);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The DOM APIs have been stable for years, this even works in IE now.

}

return flag;
const $teardown = $('#new-app-flag-teardown')[0];
if ($teardown) {
flags.teardown = Boolean($teardown.checked);
}

const $setup = $('#new-app-flag-setup')[0];
if ($setup) {
flags.setup = Boolean($setup.checked);
}

return flags;
},

createMountPointModal: function (callback, mode, mountpoint) {
Expand Down Expand Up @@ -1200,20 +1207,32 @@
)
);

if (window.App.replaceApp) {
tableContent.push(
window.modalView.createCheckboxEntry(
'new-app-flag-teardown',
'Run teardown?',
true,
"Should the existing service's teardown script be executed before replacing the service?",
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 didn't actually ask about the teardown when replacing an app. This used to be implied by the replace/upgrade flag.

false
)
);
}

tableContent.push(
window.modalView.createCheckboxEntry(
'new-app-teardown',
'new-app-flag-setup',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems cleaner to prefix these consistently with "-flag"

'Run setup?',
true,
"Should this app's setup script be executed after installing the app?",
"Should this service's setup script be executed after installing the service?",
Copy link
Contributor Author
@pluma pluma Aug 30, 2019 < 8000 /div>

Choose a reason for hiding this comment

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

This is just for consistency. We renamed "applications" to "services" in 3.0.

true
)
);

if (window.App.replaceApp) {
tableContent.push(
window.modalView.createCheckboxEntry(
'new-app-replace',
'new-app-flag-replace',
'Discard configuration and dependency files?',
true,
"Should this service's existing configuration and settings be removed completely before replacing the service?",
Expand Down Expand Up @@ -1273,7 +1292,13 @@
} else {
$('#new-app-mount').attr('disabled', 'true');
$('#new-app-replace').attr('checked', false);
$('#new-app-teardown').attr('disabled', true);
$('#new-app-replace').on('click', function () {
if ($('#new-app-replace').prop('checked')) {
$('#new-app-teardown').attr('disabled', true);
} else {
$('#new-app-teardown').attr('disabled', false);
}
});
}
}
};
Expand Down
130 changes: 11 additions & 119 deletions js/apps/system/_admin/aardvark/APP/frontend/js/collections/foxxCollection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,76 +28,21 @@
this.sortOptions.desc = val;
},

// Install Foxx from github repo
// info is expected to contain: "url" and "version"
installFromGithub: function (info, mount, callback, isLegacy, flag) {
var url = arangoHelper.databaseUrl('/_admin/aardvark/foxxes/git?mount=' + encodeURIComponent(mount));
if (isLegacy) {
install: function (info, mount, callback, mode, options) {
var url = arangoHelper.databaseUrl('/_admin/aardvark/foxxes/' + mode + '?mount=' + encodeURIComponent(mount));
if (options.legacy) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The legacy flag has now been merged into the other option flags.

url += '&legacy=true';
}
if (flag !== undefined) {
if (flag) {
url += '&replace=true';
} else {
url += '&upgrade=true';
}
if (options.setup === false) {
url += '&setup=false';
}
$.ajax({
cache: false,
type: 'PUT',
url: url,
data: JSON.stringify(info),
contentType: 'application/json',
processData: false,
success: function (data) {
callback(data);
},
error: function (err) {
callback(err);
}
});
},

// Install Foxx from public url
// info is expected to contain: "url" and "version"
installFromUrl: function (info, mount, callback, isLegacy, flag) {
var url = arangoHelper.databaseUrl('/_admin/aardvark/foxxes/url?mount=' + encodeURIComponent(mount));
if (isLegacy) {
url += '&legacy=true';
if (options.teradown === false) {
url += '&teardown=false';
}
if (flag !== undefined) {
if (flag) {
url += '&replace=true';
} else {
url += '&upgrade=true';
}
}
$.ajax({
cache: false,
type: 'PUT',
url: url,
data: JSON.stringify(info),
contentType: 'application/json',
processData: false,
success: function (data) {
callback(data);
},
error: function (err) {
callback(err);
}
});
},

// Install Foxx from arango store
// info is expected to contain: "name" and "version"
installFromStore: function (info, mount, callback, flag) {
var url = arangoHelper.databaseUrl('/_admin/aardvark/foxxes/store?mount=' + encodeURIComponent(mount));
if (flag !== undefined) {
if (flag) {
url += '&replace=true';
} else {
url += '&upgrade=true';
}
if (options.replace === true) {
url += '&replace=true';
} else if (options.replace === false) {
url += '&upgrade=true';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This previously used to be the magic enum flag.

}
$.ajax({
cache: false,
Expand All @@ -115,58 +60,5 @@
});
},

installFromZip: function (fileName, mount, callback, isLegacy, flag) {
var url = arangoHelper.databaseUrl('/_admin/aardvark/foxxes/zip?mount=' + encodeURIComponent(mount));
if (isLegacy) {
url += '&legacy=true';
}
if (flag !== undefined) {
if (flag) {
url += '&replace=true';
} else {
url += '&upgrade=true';
}
}
$.ajax({
cache: false,
type: 'PUT',
url: url,
data: JSON.stringify({zipFile: fileName}),
contentType: 'application/json',
processData: false,
success: function (data) {
callback(data);
},
error: function (err) {
callback(err);
}
});
},

generate: function (info, mount, callback, flag) {
var url = arangoHelper.databaseUrl('/_admin/aardvark/foxxes/generate?mount=' + encodeURIComponent(mount));
if (flag !== undefined) {
if (flag) {
url += '&replace=true';
} else {
url += '&upgrade=true';
}
}
$.ajax({
cache: false,
type: 'PUT',
url: url,
data: JSON.stringify(info),
contentType: 'application/json',
processData: false,
success: function (data) {
callback(data);
},
error: function (err) {
callback(err);
}
});
}

});
}());
16 changes: 9 additions & 7 deletions js/apps/system/_admin/aardvark/APP/frontend/js/views/foxxRepoView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,21 +201,23 @@

installFoxxFromStore: function (e) {
if (window.modalView.modalTestAll()) {
var mount, flag;
var mount, info, options;
if (this._upgrade) {
mount = window.App.replaceAppData.mount;
flag = arangoHelper.getFoxxFlag();
} else {
mount = window.arangoHelper.escapeHtml($('#new-app-mount').val());
if (mount.charAt(0) !== '/') {
mount = '/' + mount;
}
}
if (flag !== undefined) {
this.collection.installFromStore({name: this.toInstall, version: this.version}, mount, this.installCallback.bind(this), flag);
} else {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These two are the same except once without the argument if it's undefined. That's silly because the function doesn't check the argument count and undefined is undefined.

this.collection.installFromStore({name: this.toInstall, version: this.version}, mount, this.installCallback.bind(this));
}

info = {
name: this.toInstall,
version: this.version
};

options = arangoHelper.getFoxxFlags();
this.collection.install('store', info, mount, options, this.installCallback.bind(this));
window.modalView.hide();

if (this._upgrade) {
Expand Down
21 changes: 11 additions & 10 deletions js/apps/system/_admin/aardvark/APP/frontend/js/views/installGitHubServiceView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,35 +73,36 @@

installFoxxFromGithub: function () {
if (window.modalView.modalTestAll()) {
var url, version, mount, flag, isLegacy;
var mount, info, options, url, version;
if (this._upgrade) {
mount = window.App.replaceAppData.mount;
flag = arangoHelper.getFoxxFlag();
} else {
mount = window.arangoHelper.escapeHtml($('#new-app-mount').val());
if (mount.charAt(0) !== '/') {
mount = '/' + mount;
}
}

url = window.arangoHelper.escapeHtml($('#repository').val());
version = window.arangoHelper.escapeHtml($('#tag').val());

if (version === '') {
version = 'master';
}
var info = {
url: window.arangoHelper.escapeHtml($('#repository').val()),
version: window.arangoHelper.escapeHtml($('#tag').val())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is literally the same code as a few lines earlier except it ignores the default for "master". This is likely a bug.

};

try {
Joi.assert(url, Joi.string().regex(/^[a-zA-Z0-9_-]+\/[a-zA-Z0-9_-]+$/));
} catch (e) {
return;
}
// send server req through collection
isLegacy = Boolean($('#github-app-islegacy').prop('checked'));
this.collection.installFromGithub(info, mount, this.installCallback.bind(this), isLegacy, flag);

info = {
url: url,
version: version
};

options = arangoHelper.getFoxxFlags();
options.legacy = Boolean($('#github-app-islegacy')[0].checked);
this.collection.install('git', info, mount, options, this.installCallback.bind(this));
}
},

Expand Down
10 changes: 6 additions & 4 deletions js/apps/system/_admin/aardvark/APP/frontend/js/views/installNewServiceView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,17 @@

generateNewFoxxApp: function () {
if (window.modalView.modalTestAll()) {
var mount, flag;
var mount, info, options;
if (this._upgrade) {
mount = window.App.replaceAppData.mount;
flag = arangoHelper.getFoxxFlag();
} else {
mount = window.arangoHelper.escapeHtml($('#new-app-mount').val());
if (mount.charAt(0) !== '/') {
mount = '/' + mount;
}
}
var info = {

info = {
name: window.arangoHelper.escapeHtml($('#new-app-name').val()),
documentCollections: _.map($('#new-app-document-collections').select2('data'), function (d) {
return window.arangoHelper.escapeHtml(d.text);
Expand All @@ -79,7 +79,9 @@
license: window.arangoHelper.escapeHtml($('#new-app-license').val()),
description: window.arangoHelper.escapeHtml($('#new-app-description').val())
};
this.collection.generate(info, mount, this.installCallback.bind(this), flag);

options = arangoHelper.getFoxxFlags();
this.collection.install('generate', info, mount, options, this.installCallback.bind(this));
}
window.modalView.hide();
},
Expand Down
13 changes: 9 additions & 4 deletions js/apps/system/_admin/aardvark/APP/frontend/js/views/installUploadServiceView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,23 @@
this._uploadData = window.foxxData.data;
}
if (window.foxxData.data && window.modalView.modalTestAll()) {
var mount, flag, isLegacy;
var mount, info, options;
if (this._upgrade) {
mount = window.App.replaceAppData.mount;
flag = arangoHelper.getFoxxFlag();
} else {
mount = window.arangoHelper.escapeHtml($('#new-app-mount').val());
if (mount.charAt(0) !== '/') {
mount = '/' + mount;
}
}
isLegacy = Boolean($('#zip-app-islegacy').prop('checked'));
this.collection.installFromZip(window.foxxData.data.filename, mount, this.installCallback.bind(this), isLegacy, flag);

info = {
zipFile: window.foxxData.data.filename
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wrapping previously happened in installFromZip.

};

options = arangoHelper.getFoxxFlags();
options.legacy = Boolean($('#zip-app-islegacy')[0].checked);
this.collection.install('zip', info, mount, options, this.installCallback.bind(this));
}
window.modalView.hide();
},
Expand Down
Loading
0