From 1c7f84b044c7efeb4565aff2788ac80a942e96e6 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Fri, 21 Feb 2020 12:00:43 -0500
Subject: [PATCH 01/71] Remove workflow node style dependencies to base.css

---
 .../components/Workflow/Editor/Node.vue       | 11 ++++----
 .../scripts/mvc/workflow/workflow-manager.js  |  2 +-
 .../scripts/mvc/workflow/workflow-node.js     |  6 ++---
 .../mvc/workflow/workflow-view-node.js        |  4 +--
 client/galaxy/style/scss/workflow.scss        | 27 ++++++++++---------
 5 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/client/galaxy/scripts/components/Workflow/Editor/Node.vue b/client/galaxy/scripts/components/Workflow/Editor/Node.vue
index 444443f16c2..5de4895e817 100644
--- a/client/galaxy/scripts/components/Workflow/Editor/Node.vue
+++ b/client/galaxy/scripts/components/Workflow/Editor/Node.vue
@@ -1,9 +1,10 @@
 <template>
     <div>
-        <div class="toolFormTitle unselectable clearfix">
-            <i :class="iconClass" />
-            <span class="node-title">{{ title }}</span>
-            <span class="sr-only">&nbsp;Node</span>
+        <div class="node-header unselectable clearfix">
+            <div class="header float-left">
+                <i :class="iconClass" />
+                <span class="node-title">{{ title }}</span>
+            </div>
             <div class="buttons float-right">
                 <a
                     v-if="canClone"
@@ -22,7 +23,7 @@
                 />
             </div>
         </div>
-        <div class="toolFormBody">
+        <div class="node-body">
             <div>
                 <loading-span message="Loading details" />
             </div>
diff --git a/client/galaxy/scripts/mvc/workflow/workflow-manager.js b/client/galaxy/scripts/mvc/workflow/workflow-manager.js
index 16ffd2c7156..cc8f7cf83ac 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-manager.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-manager.js
@@ -134,7 +134,7 @@ class Workflow extends EventEmitter {
 
         // Create node wrapper
         const container = document.createElement("div");
-        container.className = "toolForm toolFormInCanvas";
+        container.className = "workflow-node";
         document.getElementById("canvas-container").appendChild(container);
         var $f = $(container);
 
diff --git a/client/galaxy/scripts/mvc/workflow/workflow-node.js b/client/galaxy/scripts/mvc/workflow/workflow-node.js
index 2952b9b04be..72ca51e2754 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-node.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-node.js
@@ -191,7 +191,7 @@ export class Node {
         $(this.element).remove();
     }
     make_active() {
-        $(this.element).addClass("toolForm-active");
+        $(this.element).addClass("node-active");
     }
     make_inactive() {
         // Keep inactive nodes stacked from most to least recently active
@@ -202,7 +202,7 @@ export class Node {
             p.appendChild(element);
         })(element.parentNode);
         // Remove active class
-        $(element).removeClass("toolForm-active");
+        $(element).removeClass("node-active");
     }
     set_tool_version() {
         if (this.type === "tool" && this.config_form) {
@@ -341,7 +341,7 @@ export class Node {
         this.redraw();
     }
     error(text) {
-        var b = $(this.element).find(".toolFormBody");
+        var b = $(this.element).find(".node-body");
         b.find("div").remove();
         var tmp = `<div style='color: red; text-style: italic;'>${text}</div>`;
         this.config_form = tmp;
diff --git a/client/galaxy/scripts/mvc/workflow/workflow-view-node.js b/client/galaxy/scripts/mvc/workflow/workflow-view-node.js
index c18207dd113..2b16458a0ff 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-view-node.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-view-node.js
@@ -9,7 +9,7 @@ export class NodeView {
         this.$el = options.$el;
         this.node = options.node;
         this.output_width = Math.max(150, this.$el.width());
-        this.tool_body = this.$el.find(".toolFormBody");
+        this.tool_body = this.$el.find(".node-body");
         this.tool_body.find("div").remove();
         this.newInputsDiv().appendTo(this.tool_body);
         this.terminalViews = {};
@@ -28,7 +28,7 @@ export class NodeView {
     }
 
     renderToolErrors() {
-        this.node.errors ? this.$el.addClass("tool-node-error") : this.$el.removeClass("tool-node-error");
+        this.node.errors ? this.$el.addClass("node-error") : this.$el.removeClass("node-error");
     }
 
     newInputsDiv() {
diff --git a/client/galaxy/style/scss/workflow.scss b/client/galaxy/style/scss/workflow.scss
index 30e5e950d05..5061dc8b904 100644
--- a/client/galaxy/style/scss/workflow.scss
+++ b/client/galaxy/style/scss/workflow.scss
@@ -7,40 +7,43 @@
         display: flex;
         flex-direction: column;
         flex-grow: 1;
-        .toolForm.toolFormInCanvas {
+        .workflow-node {
             @extend .card;
+            @extend .position-absolute;
             z-index: 100;
             width: $worfklow-node-width;
-            position: absolute;
-            border-color: $brand-primary;
-            &.toolForm-active {
+            border: solid $brand-primary 1px;
+            &.node-active {
                 z-index: 1001;
                 box-shadow: 0 0 0 2px $brand-primary;
             }
-            .toolFormTitle {
+            .node-header {
                 @extend .card-header;
                 @extend .py-1;
                 @extend .px-2;
                 cursor: move;
                 background: $brand-secondary;
+                .header {
+                    width: 7rem;
+                }
             }
-            .toolFormBody {
+            .node-body {
                 @extend .card-body;
                 @extend .p-0;
                 @extend .mx-2;
                 .rule {
                     height: 0;
                     border: none;
-                    border-bottom: dotted black 1px;
+                    border-bottom: dotted $brand-primary 1px;
                     margin: 0 5px;
                 }
             }
         }
-        .toolForm.toolFormInCanvas.tool-node-error {
-            border-color: $state-danger-border;
-            .toolFormTitle {
-                background: $state-danger-bg;
-                border-color: $state-danger-border;
+        .workflow-node.node-error {
+            border-color: $brand-danger;
+            .node-header {
+                background: $brand-danger;
+                border-color: $brand-danger;
             }
         }
         .workflow-canvas {

From f344dd85d38761f87bfb02fadb543e4ec5d6923f Mon Sep 17 00:00:00 2001
From: Alexander OSTROVSKY <alexanderostrovsky@Caroline.local>
Date: Wed, 7 Aug 2019 14:46:41 -0400
Subject: [PATCH 02/71] parent 207f4c5fb0423c13a2afcf34c78b9629a0c0ca75 author
 Alexander OSTROVSKY <alexanderostrovsky@Caroline.local> 1565203601 -0400
 committer Alexander OSTROVSKY <alexanderostrovsky@p-body.win.ad.jhu.edu>
 1582640692 -0500

Notifications on job completion

JS lint for var notification

Altered to accomodate job failures and user cancellations

Added an icon to easily differentiate job success from failure

Tab updates when window is hidden.

Better completion message, removed console.log

Notification grammar, and linking tab icon change with notification permissions

tab fix

Tab resets on view

js lint
---
 .../scripts/mvc/dataset/dataset-model.js      |  28 ++++++++++
 client/galaxy/scripts/mvc/user/tab-updates.js |  52 ++++++++++++++++++
 static/erricon.ico                            | Bin 0 -> 2253 bytes
 3 files changed, 80 insertions(+)
 create mode 100644 client/galaxy/scripts/mvc/user/tab-updates.js
 create mode 100644 static/erricon.ico

diff --git a/client/galaxy/scripts/mvc/dataset/dataset-model.js b/client/galaxy/scripts/mvc/dataset/dataset-model.js
index 69f18aa6ce0..b16aae7a0bd 100644
--- a/client/galaxy/scripts/mvc/dataset/dataset-model.js
+++ b/client/galaxy/scripts/mvc/dataset/dataset-model.js
@@ -5,8 +5,10 @@ import { getAppRoot } from "onload/loadConfig";
 import STATES from "mvc/dataset/states";
 import BASE_MVC from "mvc/base-mvc";
 import _l from "utils/localization";
+import TAB_UPDATES from "mvc/user/tab-updates";
 
 var logNamespace = "dataset";
+var hiddenupdates = 1;
 //==============================================================================
 var searchableMixin = BASE_MVC.SearchableModelMixin;
 /** @class base model for any DatasetAssociation (HDAs, LDDAs, DatasetCollectionDAs).
@@ -99,6 +101,25 @@ var DatasetAssociation = Backbone.Model.extend(BASE_MVC.LoggableMixin).extend(
                     this.log(`${this} has changed state:`, currModel, newState);
                     if (this.inReadyState()) {
                         this.trigger("state:ready", currModel, newState, this.previous("state"));
+                        if (newState != "discarded"){
+                            if(newState === "ok"){
+                                new Notification(`Job complete: ${this.get("name")}`, {
+                                icon: "static/favicon.ico"
+                                      });
+                                if(TAB_UPDATES.is_hidden()){
+                                    TAB_UPDATES.hidden_count(hiddenupdates);
+                                    hiddenupdates++;
+                                }
+                            } else if(newState == "error"){
+                                new Notification(`Job failure: ${this.get("name")}`, {
+                                icon: "static/erricon.ico"
+                                      });
+                                if(TAB_UPDATES.is_hidden() && Notification.permission == "granted"){
+                                    TAB_UPDATES.change_favicon("static/erricon.ico")
+                                }
+                            }
+
+                          }
                     }
                 });
                 // the download url (currently) relies on having a correct file extension
@@ -349,6 +370,13 @@ var DatasetAssociationCollection = Backbone.Collection.extend(BASE_MVC.LoggableM
     }
 );
 
+window.addEventListener("focus", function(){
+    if(document.getElementById('tabicon').href.search("erricon") != -1){
+        TAB_UPDATES.change_favicon("static/favicon.ico");
+    }
+    document.title = "Galaxy";
+    hiddenupdates = 1;
+});
 //==============================================================================
 export default {
     DatasetAssociation: DatasetAssociation,
diff --git a/client/galaxy/scripts/mvc/user/tab-updates.js b/client/galaxy/scripts/mvc/user/tab-updates.js
new file mode 100644
index 00000000000..b098eae4fa9
--- /dev/null
+++ b/client/galaxy/scripts/mvc/user/tab-updates.js
@@ -0,0 +1,52 @@
+//Allow change tab on event=====================================================
+function change_favicon(img){
+    var favicon = document.createElement("link");
+    favicon.id = "tabicon";
+    favicon.setAttribute("rel", "shortcut icon");
+    var head = document.querySelector("head");
+    head.appendChild(favicon);
+    favicon.setAttribute("type", "image/png");
+    favicon.setAttribute("href", img);
+}
+
+//Check if browser is hidden-aware==============================================
+function hidden_permit(){
+    var browsers = ["webkit","moz","ms","o"];
+    if ("hidden" in document){
+        return "hidden";
+    }
+    for (var i=0; i < browsers.length; i++){
+        if ((browsers[i] + "Hidden") in document){
+            return browsers[i] + "Hidden";
+        }
+    }
+    return null;
+}
+
+//Find out if browser tab is hidden=============================================
+function is_hidden(){
+    var state = hidden_permit();
+    if (!state){
+        return false;
+    }
+    return document[state];
+}
+
+//Generate new tab message to display on update in window=======================
+function hidden_count(current){
+    if(is_hidden()){
+        if(current === 1){
+            document.title = "Galaxy (1 job completed)";
+        }else{
+        document.title = "Galaxy (" + current + " jobs completed)";
+        }
+    }
+}
+
+
+export default{
+    change_favicon: change_favicon,
+    hidden_permit: hidden_permit,
+    is_hidden: is_hidden,
+    hidden_count: hidden_count
+};
diff --git a/static/erricon.ico b/static/erricon.ico
new file mode 100644
index 0000000000000000000000000000000000000000..c5ee4d32e54a720ba16fd2a49149cface7122767
GIT binary patch
literal 2253
zcmaJ@2{fBo8~$PmVynuitpu5|OTyUFP$f!6?KxwqmLSSOzEncur?%G6R&3L1`7!zz
zZEaIachp`*wU%1i(W){+$6ni!@+X*%>FLaO&imba-}^l8`@HwO=bn4s$-zb(CJzGu
zK%8KUcj1m`o(V&_e>cAMB<=vAx!70&72S$c+=U3$)`JEBqS8Er06Dp`0Kg9f5#8zT
z_Q$Y(V5lC+AM_>bF+-_bGyve3Snf2GOeZ0jp&=9+mT9KCjlgo}JU2=eu??XIo2k0n
zJ0YyVFfzhW?|VJ8syPgSK;Xjs1F$amqq}tO%1kwoPN!l~C<cR}$1u<X!%m?vrlzJS
zv_48-Uzdx}rA1QcB&IHfrnVF0&p3E8%`Ys7N)G}l2wq&0FBncYQ&r_9`uy$aqz483
zm5D;zEsI+qinoKp=%G<xqj8J<tnY6uCW-owBx)c?2Wf%eKTtn!?*!OBmL0V?o))Z!
z3z-Ing!8I0H`d2(3;dh&ukv;X?q4J9b?zZ>3&L83kx6tio@?g%XpE^Y+E~{BL&TV1
zF{W6IF%I<w<_oqx=pPgi`S0vKm@nAdxxp|Z2!@zDZJ!f@+Z%gw?(XfOeLgQNFX@hW
z|F0|mI&?P{4#n${Jw3D2klS8vN1DU9D*o(lbJ*<M@(=(B1`zN^h=GDL?scG|H9WRD
z>Yn+=vD}vZKO!&K!bOw`()$Dj<Dn1xYdC@p-qac*yx*2sn3Qju40TSo4a($0TD04?
z-A$PDn~V7AEf{c$(mlj@G#Q@IQa8T-tUfz(seYwySlXkpgI`;+3^%W1Rxs8)H)p#_
zbu;pP(<kr!Tjo<%+JBNt2K&Oh#h^I&aM$a_2UG1I0+62Hg|~Ehyxvb3u!i5P+3zeT
zCx^}(DLmBtl(M-{{%(^|Exq)M#z2#F2IWU2*lz3X=Je)8&Hr+}vMg*qTqBI>1ePCh
z5yer(nV7=1n`QU4tZtDTFq#=H)mo0umwJduwU=Y0<T!FgkCI<fN#Vj+QJZEht3Lz&
zu3@W~)E0-dJK-N(cT|*^Fsl{RER0NTva<ZizD!V|)ZS5??Q12vo}K;@zsVFFjnF8U
z71Vl*J|<q^qXe8HG@hN7$Jk4et8WtrJ{%kVzQrNmMj#NdsKg?>u00gFc&5KIYS^{o
z<J`~od8Qo))^iEMScs*-1^dF-LxaWa+}mY*ol!Y<<v09Jj4e!6^_!F{M8GHZ4@7`7
zlT(hrd^bIxlU<OZ%P*$%ZG<dKrtM`5TSPy&+8~|7-XN!*n2F3%5`H>tC3VJeTta_z
zqH-?4&~WM3GaRfFzA7G0=sR4c-a6uj->lG#m|dKgs8tKQRjq4wz1q#Sp6_L^vG6_n
zl97mIrBCB)7ei8p74ELYq@Ri0XJp8|X6Iuxex}%we4^qc8gE62br#$FK`o+b@h>0l
zx&G=-<h$WUfTZ3%go%`9Wj4qdae7i<{k{HpsVck5=SKB-F-ktnjqJxdy{c7e{VZ9p
zzt#(F2#8(ANW5CPj<5*Q5G;D)`7ux@z$f~6m0huUTZ}ya`K(3X_81GR97ePg*aqa6
zF{4r%HeRIkC>G!jPDRFe48;>JAF7b=mFk{RDVW@s=)qrm^vRS!hq4Oas6l*SoN<%3
za$hXVuGM)&KIsRjtbE+{I!Iw9gt&IUH-B=S6?_ur6#~>4KIn`B1<d3W8g$g#eo$7v
zGHK|L0Fg|qSoVIsIaBXElG>|d3S_x3-wxT0^(UJo{-MuVfGwFqD_Rxju08hUxZws{
zu1-SNoc-;*vqa+?kI=^vIkH*rwHR(34HX;Jv-hl5&>tHw7Jf=gGLX7CE#xJ7iBm>F
zEO=jC&_G$$Cf$Bjtl=JPk$OB!CaX&3>SDGvYbm-rZdR^V(s|MRy%%Y#5;*xgCv>#V
z(mca*xq)BjRD)mP@Hgw8ZHvWbmokMb=F=>M&~b8uLYdfprbEYRNY2=BPdpM_Zv80a
z`%u<fXlB*=a8phxe-C5iq0vf8`~iy9>^-&C4Uuz$Ns}{4FJy4%4)I4%IyX-$E_Hfd
zVYlhXtTjQj9u2m3++o9U?9;#-J+{RB0UwEI5$1UZXsgx(4=m>-6{ns9xiBu?9Sgko
zBs!wK!#Bfh?z_*9Xish2=)nFL;xHAY3Vm#1IQ?njxyo}0p|p8e>}HHZ;^i)V<#<?U
z=eJub{OrhIQ@8FO53yypT)dLscVR$6z?J15xvAO}csu!-(Bt6El9KA^#1RE4*Vz^D
ziO40^)@mlGEs-zTF*%bNGqLJCAf|I5o^!UcEOtR1DPAD2kS>(dAZgQ2K?K*|Nbue?
zL^syn4PKjwTX?S+6$fLqfK1188*`(4D@DN}ixrQp#`IU>?9;x{Bo?{qF3IN|eHLFU
zOs;5Y>bQezIyn7E=aHoZlFTR)$v#+3gSW$VKQyQzqUNA%6{@sJr6LPj+_S2ACC`1s
jAvz~Udvo{BqvPf!6Q@h_ieiZFyf+5H+5ul->67q(P{pnO

literal 0
HcmV?d00001


From c673eb4a3fc0866d48cbd8ace6bbc577a14c7960 Mon Sep 17 00:00:00 2001
From: Alexander OSTROVSKY <alexanderostrovsky@p-body.win.ad.jhu.edu>
Date: Tue, 25 Feb 2020 11:03:00 -0500
Subject: [PATCH 03/71] incremental

---
 .../components/User/UserPreferences.vue       | 19 ++++++++++++
 .../scripts/mvc/dataset/dataset-model.js      | 31 ++++++++++---------
 client/galaxy/scripts/mvc/user/tab-updates.js | 29 +++++++++--------
 3 files changed, 50 insertions(+), 29 deletions(-)

diff --git a/client/galaxy/scripts/components/User/UserPreferences.vue b/client/galaxy/scripts/components/User/UserPreferences.vue
index 6bc0e467dd5..dc86e9b09f1 100644
--- a/client/galaxy/scripts/components/User/UserPreferences.vue
+++ b/client/galaxy/scripts/components/User/UserPreferences.vue
@@ -22,6 +22,13 @@
                 </div>
             </div>
         </b-row>
+        <b-row class="ml-3 mb-1">
+            <i class="fa-plus-square-o" />
+            <div class="pref-content pr-1">
+                <a @click="toggleNotifications" href="javascript:void(0)"><b>Enable notifications</b></a>
+                <div class="form-text text-muted">Allow push and tab notifcations on job completion/failure</div>
+            </div>
+        </b-row>
         <p class="mt-2">
             You are using <strong>{{ diskUsage }}</strong> of disk space in this Galaxy instance.
             <span v-html="quotaUsageString"></span>
@@ -107,6 +114,18 @@ export default {
         }
     },
     methods: {
+        toggleNotifications() {
+            Notification.requestPermission().then(function(permission) {
+                //If the user accepts, let's create a notification
+                if (permission === "granted") {
+                    new Notification("Notifications enabled", {
+                        icon: "static/favicon.ico"
+                    });
+                } else {
+                    alert("Notifications disabled, please re-enable through browser settings.");
+                }
+            });
+        },
         openManageCustomBuilds() {
             const Galaxy = getGalaxyInstance();
             Galaxy.page.router.push(`${getAppRoot()}custom_builds`);
diff --git a/client/galaxy/scripts/mvc/dataset/dataset-model.js b/client/galaxy/scripts/mvc/dataset/dataset-model.js
index b16aae7a0bd..fa781f13b4e 100644
--- a/client/galaxy/scripts/mvc/dataset/dataset-model.js
+++ b/client/galaxy/scripts/mvc/dataset/dataset-model.js
@@ -101,25 +101,27 @@ var DatasetAssociation = Backbone.Model.extend(BASE_MVC.LoggableMixin).extend(
                     this.log(`${this} has changed state:`, currModel, newState);
                     if (this.inReadyState()) {
                         this.trigger("state:ready", currModel, newState, this.previous("state"));
-                        if (newState != "discarded"){
-                            if(newState === "ok"){
+                        if (newState != "discarded") {
+                            if (newState === "ok") {
+                                console.log("helloworld");
                                 new Notification(`Job complete: ${this.get("name")}`, {
-                                icon: "static/favicon.ico"
-                                      });
-                                if(TAB_UPDATES.is_hidden()){
+                                    icon: "static/favicon.ico"
+                                });
+                                new Notification("HELLO");
+                                if (TAB_UPDATES.is_hidden()) {
                                     TAB_UPDATES.hidden_count(hiddenupdates);
                                     hiddenupdates++;
                                 }
-                            } else if(newState == "error"){
+                            } else if (newState == "error") {
+                                console.log("goodbye");
                                 new Notification(`Job failure: ${this.get("name")}`, {
-                                icon: "static/erricon.ico"
-                                      });
-                                if(TAB_UPDATES.is_hidden() && Notification.permission == "granted"){
-                                    TAB_UPDATES.change_favicon("static/erricon.ico")
+                                    icon: "static/erricon.ico"
+                                });
+                                if (TAB_UPDATES.is_hidden() && Notification.permission == "granted") {
+                                    TAB_UPDATES.change_favicon("static/erricon.ico");
                                 }
                             }
-
-                          }
+                        }
                     }
                 });
                 // the download url (currently) relies on having a correct file extension
@@ -370,8 +372,9 @@ var DatasetAssociationCollection = Backbone.Collection.extend(BASE_MVC.LoggableM
     }
 );
 
-window.addEventListener("focus", function(){
-    if(document.getElementById('tabicon').href.search("erricon") != -1){
+window.addEventListener("focus", function() {
+    const testing = document.getElementById("tabicon");
+    if (testing && testing.href.search("erricon") != -1) {
         TAB_UPDATES.change_favicon("static/favicon.ico");
     }
     document.title = "Galaxy";
diff --git a/client/galaxy/scripts/mvc/user/tab-updates.js b/client/galaxy/scripts/mvc/user/tab-updates.js
index b098eae4fa9..5b32f63d7ea 100644
--- a/client/galaxy/scripts/mvc/user/tab-updates.js
+++ b/client/galaxy/scripts/mvc/user/tab-updates.js
@@ -1,5 +1,5 @@
 //Allow change tab on event=====================================================
-function change_favicon(img){
+function change_favicon(img) {
     var favicon = document.createElement("link");
     favicon.id = "tabicon";
     favicon.setAttribute("rel", "shortcut icon");
@@ -10,13 +10,13 @@ function change_favicon(img){
 }
 
 //Check if browser is hidden-aware==============================================
-function hidden_permit(){
-    var browsers = ["webkit","moz","ms","o"];
-    if ("hidden" in document){
+function hidden_permit() {
+    var browsers = ["webkit", "moz", "ms", "o"];
+    if ("hidden" in document) {
         return "hidden";
     }
-    for (var i=0; i < browsers.length; i++){
-        if ((browsers[i] + "Hidden") in document){
+    for (var i = 0; i < browsers.length; i++) {
+        if (browsers[i] + "Hidden" in document) {
             return browsers[i] + "Hidden";
         }
     }
@@ -24,27 +24,26 @@ function hidden_permit(){
 }
 
 //Find out if browser tab is hidden=============================================
-function is_hidden(){
+function is_hidden() {
     var state = hidden_permit();
-    if (!state){
+    if (!state) {
         return false;
     }
     return document[state];
 }
 
 //Generate new tab message to display on update in window=======================
-function hidden_count(current){
-    if(is_hidden()){
-        if(current === 1){
+function hidden_count(current) {
+    if (is_hidden()) {
+        if (current === 1) {
             document.title = "Galaxy (1 job completed)";
-        }else{
-        document.title = "Galaxy (" + current + " jobs completed)";
+        } else {
+            document.title = "Galaxy (" + current + " jobs completed)";
         }
     }
 }
 
-
-export default{
+export default {
     change_favicon: change_favicon,
     hidden_permit: hidden_permit,
     is_hidden: is_hidden,

From 4717e621e6f75107831ef3d1f7436f12cbb41d10 Mon Sep 17 00:00:00 2001
From: Dannon Baker <dannon.baker@gmail.com>
Date: Tue, 25 Feb 2020 11:12:35 -0500
Subject: [PATCH 04/71] Fix styling of entry in preferences list, add info on
 how to unset

---
 client/galaxy/scripts/components/User/UserPreferences.vue | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/client/galaxy/scripts/components/User/UserPreferences.vue b/client/galaxy/scripts/components/User/UserPreferences.vue
index dc86e9b09f1..31d081bc356 100644
--- a/client/galaxy/scripts/components/User/UserPreferences.vue
+++ b/client/galaxy/scripts/components/User/UserPreferences.vue
@@ -23,10 +23,13 @@
             </div>
         </b-row>
         <b-row class="ml-3 mb-1">
-            <i class="fa-plus-square-o" />
+            <i class="pref-icon pt-1 fa fa-lg fa-plus-square-o" />
             <div class="pref-content pr-1">
                 <a @click="toggleNotifications" href="javascript:void(0)"><b>Enable notifications</b></a>
-                <div class="form-text text-muted">Allow push and tab notifcations on job completion/failure</div>
+                <div class="form-text text-muted">
+                    Allow push and tab notifcations on job completion. To disable, revoke the site notification
+                    privilege in your browser.
+                </div>
             </div>
         </b-row>
         <p class="mt-2">

From 64a741b247917f97ea4b3a8d77f0da9cac574cc6 Mon Sep 17 00:00:00 2001
From: Alexander OSTROVSKY <alexanderostrovsky@p-body.win.ad.jhu.edu>
Date: Tue, 25 Feb 2020 11:14:38 -0500
Subject: [PATCH 05/71] final tweaks

---
 client/galaxy/scripts/mvc/dataset/dataset-model.js | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/client/galaxy/scripts/mvc/dataset/dataset-model.js b/client/galaxy/scripts/mvc/dataset/dataset-model.js
index fa781f13b4e..85a8559bea5 100644
--- a/client/galaxy/scripts/mvc/dataset/dataset-model.js
+++ b/client/galaxy/scripts/mvc/dataset/dataset-model.js
@@ -103,17 +103,14 @@ var DatasetAssociation = Backbone.Model.extend(BASE_MVC.LoggableMixin).extend(
                         this.trigger("state:ready", currModel, newState, this.previous("state"));
                         if (newState != "discarded") {
                             if (newState === "ok") {
-                                console.log("helloworld");
                                 new Notification(`Job complete: ${this.get("name")}`, {
                                     icon: "static/favicon.ico"
                                 });
-                                new Notification("HELLO");
                                 if (TAB_UPDATES.is_hidden()) {
                                     TAB_UPDATES.hidden_count(hiddenupdates);
                                     hiddenupdates++;
                                 }
                             } else if (newState == "error") {
-                                console.log("goodbye");
                                 new Notification(`Job failure: ${this.get("name")}`, {
                                     icon: "static/erricon.ico"
                                 });

From 0aafc6e1592c9a4769507c07d3098484bd9ba079 Mon Sep 17 00:00:00 2001
From: Dannon Baker <dannon.baker@gmail.com>
Date: Fri, 28 Feb 2020 16:17:13 -0500
Subject: [PATCH 06/71] Login is its own entrypoint and we don't want it shoved
 into an iframe anymore.

---
 client/galaxy/scripts/layout/menu.js | 1 -
 1 file changed, 1 deletion(-)

diff --git a/client/galaxy/scripts/layout/menu.js b/client/galaxy/scripts/layout/menu.js
index 46c1ead9f75..ff5c15ec448 100644
--- a/client/galaxy/scripts/layout/menu.js
+++ b/client/galaxy/scripts/layout/menu.js
@@ -253,7 +253,6 @@ const Collection = Backbone.Collection.extend({
                     cls: "loggedout-only",
                     tooltip: _l("Login"),
                     url: "login",
-                    target: "galaxy_main",
                     noscratchbook: true
                 };
             }

From a8cbd36d9049f4cd8c86bf12cb1790a16d2304a4 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Sat, 29 Feb 2020 17:09:26 +0100
Subject: [PATCH 07/71] Drop manage_dependency_relationships altogether

Probably didn't work for some time and seems unnecessary.

Fixes
```
galaxy.tool_shed.galaxy_install.installed_repository_manager DEBUG 2020-02-28 10:57:01,109 [p:94029,w:0,m:0] [MainThread] Adding an entry for version 2.2.4 of package bowtie2 to runtime_tool_dependencies_of_installed_tool_dependencies.
Traceback (most recent call last):
  File "lib/galaxy/webapps/galaxy/buildapp.py", line 48, in app_factory
    app = galaxy.app.UniverseApplication(global_conf=global_conf, **kwargs)
  File "lib/galaxy/app.py", line 95, in __init__
    self.installed_repository_manager = InstalledRepositoryManager(self)
  File "lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py", line 84, in __init__
    self.load_dependency_relationships()
  File "lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py", line 734, in load_dependency_relationships
    self.add_entry_to_runtime_tool_dependencies_of_installed_tool_dependencies(tool_dependency)
  File "lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py", line 218, in add_entry_to_runtime_tool_dependencies_of_installed_tool_dependencies
    status=None)
  File "lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py", line 604, in get_runtime_dependent_tool_dependency_tuples
    required_env_shell_file_path = tool_dependency.get_env_shell_file_path(self.app)
  File "lib/galaxy/model/tool_shed_install/__init__.py", line 522, in get_env_shell_file_path
    installation_directory = self.installation_directory(app)
  File "lib/galaxy/model/tool_shed_install/__init__.py", line 534, in installation_directory
    return os.path.join(app.tool_dependency_dir,
  File "lib/galaxy/config/__init__.py", line 1186, in tool_dependency_dir
    return self.toolbox.dependency_manager.default_base_path
AttributeError: 'UniverseApplication' object has no attribute 'toolbox'
```
reported by @scholtalbers
---
 doc/source/admin/galaxy_options.rst           | 13 ---
 lib/galaxy/app.py                             |  2 -
 lib/galaxy/config/sample/galaxy.yml.sample    |  6 --
 .../galaxy_install/install_manager.py         |  8 --
 .../installed_repository_manager.py           | 91 -------------------
 lib/galaxy/webapps/galaxy/config_schema.yml   |  9 --
 6 files changed, 129 deletions(-)

diff --git a/doc/source/admin/galaxy_options.rst b/doc/source/admin/galaxy_options.rst
index f4948b3c19e..619dd9b285f 100644
--- a/doc/source/admin/galaxy_options.rst
+++ b/doc/source/admin/galaxy_options.rst
@@ -697,19 +697,6 @@
 :Type: int
 
 
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-``manage_dependency_relationships``
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-:Description:
-    Enable use of an in-memory registry with bi-directional
-    relationships between repositories (i.e., in addition to lists of
-    dependencies for a repository, keep an in-memory registry of
-    dependent items for each repository.
-:Default: ``false``
-:Type: bool
-
-
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 ``tool_data_table_config_path``
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/lib/galaxy/app.py b/lib/galaxy/app.py
index 7cc31f01f08..5b13f02a54b 100644
--- a/lib/galaxy/app.py
+++ b/lib/galaxy/app.py
@@ -91,9 +91,7 @@ class UniverseApplication(config.ConfiguresGalaxyMixin):
         check_migrate_tools = self.config.check_migrate_tools
         self._configure_models(check_migrate_databases=self.config.check_migrate_databases, check_migrate_tools=check_migrate_tools, config_file=config_file)
 
-        # Manage installed tool shed repositories.
         self.installed_repository_manager = InstalledRepositoryManager(self)
-
         self._configure_datatypes_registry(self.installed_repository_manager)
         galaxy.model.set_datatypes_registry(self.datatypes_registry)
 
diff --git a/lib/galaxy/config/sample/galaxy.yml.sample b/lib/galaxy/config/sample/galaxy.yml.sample
index 1263f0d1b1f..acd9c1bd0a0 100644
--- a/lib/galaxy/config/sample/galaxy.yml.sample
+++ b/lib/galaxy/config/sample/galaxy.yml.sample
@@ -447,12 +447,6 @@ galaxy:
   # between 1 and 24.
   #hours_between_check: 12
 
-  # Enable use of an in-memory registry with bi-directional
-  # relationships between repositories (i.e., in addition to lists of
-  # dependencies for a repository, keep an in-memory registry of
-  # dependent items for each repository.
-  #manage_dependency_relationships: false
-
   # XML config file that contains data table entries for the
   # ToolDataTableManager.  This file is manually # maintained by the
   # Galaxy administrator (.sample used if default does not exist).
diff --git a/lib/galaxy/tool_shed/galaxy_install/install_manager.py b/lib/galaxy/tool_shed/galaxy_install/install_manager.py
index 3a2b2bedfc2..687aa9bf964 100644
--- a/lib/galaxy/tool_shed/galaxy_install/install_manager.py
+++ b/lib/galaxy/tool_shed/galaxy_install/install_manager.py
@@ -215,10 +215,6 @@ class InstallToolDependencyManager(object):
                         if tool_dependency and tool_dependency.status in [self.install_model.ToolDependency.installation_status.INSTALLED,
                                                                           self.install_model.ToolDependency.installation_status.ERROR]:
                             installed_packages.append(tool_dependency)
-                            if self.app.config.manage_dependency_relationships:
-                                # Add the tool_dependency to the in-memory dictionaries in the installed_repository_manager.
-                                self.app.installed_repository_manager.handle_tool_dependency_install(tool_shed_repository,
-                                                                                                     tool_dependency)
         return installed_packages
 
     def install_via_fabric(self, tool_shed_repository, tool_dependency, install_dir, package_name=None, custom_fabfile_path=None,
@@ -946,10 +942,6 @@ class InstallRepositoryManager(object):
                 basic_util.remove_dir(work_dir)
             self.update_tool_shed_repository_status(tool_shed_repository,
                                                     self.install_model.ToolShedRepository.installation_status.INSTALLED)
-            if self.app.config.manage_dependency_relationships:
-                # Add the installed repository and any tool dependencies to the in-memory dictionaries
-                # in the installed_repository_manager.
-                self.app.installed_repository_manager.handle_repository_install(tool_shed_repository)
         else:
             # An error occurred while cloning the repository, so reset everything necessary to enable another attempt.
             repository_util.set_repository_attributes(self.app,
diff --git a/lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py b/lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py
index 58f3ba686d6..3ccd62febb7 100644
--- a/lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py
+++ b/lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py
@@ -79,9 +79,6 @@ class InstalledRepositoryManager(object):
         # whose values are a list of tuples defining tool_dependency objects (whose status is 'Installed') that require the key
         # at runtime.  The value defines the entire tool dependency tree.
         self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies = {}
-        if app.config.manage_dependency_relationships:
-            # Load defined dependency relationships for installed tool shed repositories and their contents.
-            self.load_dependency_relationships()
 
     def activate_repository(self, repository):
         """Activate an installed tool shed repository that has been marked as deactivated."""
@@ -651,90 +648,6 @@ class InstalledRepositoryManager(object):
                     deleted_tool_dependency_names.append(original_dependency_val_dict['name'])
         return updated_tool_dependency_names, deleted_tool_dependency_names
 
-    def handle_repository_install(self, repository):
-        """Load the dependency relationships for a repository that was just installed or reinstalled."""
-        # Populate self.repository_dependencies_of_installed_repositories.
-        self.add_entry_to_repository_dependencies_of_installed_repositories(repository)
-        # Populate self.installed_repository_dependencies_of_installed_repositories.
-        self.add_entry_to_installed_repository_dependencies_of_installed_repositories(repository)
-        # Populate self.tool_dependencies_of_installed_repositories.
-        self.add_entry_to_tool_dependencies_of_installed_repositories(repository)
-        # Populate self.installed_tool_dependencies_of_installed_repositories.
-        self.add_entry_to_installed_tool_dependencies_of_installed_repositories(repository)
-        for tool_dependency in repository.tool_dependencies:
-            # Populate self.runtime_tool_dependencies_of_installed_tool_dependencies.
-            self.add_entry_to_runtime_tool_dependencies_of_installed_tool_dependencies(tool_dependency)
-            # Populate self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies.
-            self.add_entry_to_installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies(tool_dependency)
-
-    def handle_repository_uninstall(self, repository):
-        """Remove the dependency relationships for a repository that was just uninstalled."""
-        for tool_dependency in repository.tool_dependencies:
-            tool_dependency_tup = self.get_tool_dependency_tuple_for_installed_repository_manager(tool_dependency)
-            # Remove this tool_dependency from all values in
-            # self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies
-            altered_installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies = {}
-            for (td_tup, installed_runtime_dependent_tool_dependency_tups) in self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies.items():
-                if tool_dependency_tup in installed_runtime_dependent_tool_dependency_tups:
-                    # Remove the tool_dependency from the list.
-                    installed_runtime_dependent_tool_dependency_tups.remove(tool_dependency_tup)
-                # Add the possibly altered list to the altered dictionary.
-                altered_installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies[td_tup] = \
-                    installed_runtime_dependent_tool_dependency_tups
-            self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies = \
-                altered_installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies
-            # Remove the entry for this tool_dependency from self.runtime_tool_dependencies_of_installed_tool_dependencies.
-            self.remove_entry_from_runtime_tool_dependencies_of_installed_tool_dependencies(tool_dependency)
-            # Remove the entry for this tool_dependency from
-            # self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies.
-            self.remove_entry_from_installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies(tool_dependency)
-        # Remove this repository's entry from self.installed_tool_dependencies_of_installed_repositories.
-        self.remove_entry_from_installed_tool_dependencies_of_installed_repositories(repository)
-        # Remove this repository's entry from self.tool_dependencies_of_installed_repositories
-        self.remove_entry_from_tool_dependencies_of_installed_repositories(repository)
-        # Remove this repository's entry from self.installed_repository_dependencies_of_installed_repositories.
-        self.remove_entry_from_installed_repository_dependencies_of_installed_repositories(repository)
-        # Remove this repository's entry from self.repository_dependencies_of_installed_repositories.
-        self.remove_entry_from_repository_dependencies_of_installed_repositories(repository)
-
-    def handle_tool_dependency_install(self, repository, tool_dependency):
-        """Load the dependency relationships for a tool dependency that was just installed independently of its containing repository."""
-        # The received repository must have a status of 'Installed'.  The value of tool_dependency.status will either be
-        # 'Installed' or 'Error', but we only need to change the in-memory dictionaries if it is 'Installed'.
-        if tool_dependency.is_installed:
-            # Populate self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies.
-            self.add_entry_to_installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies(tool_dependency)
-            # Populate self.installed_tool_dependencies_of_installed_repositories.
-            repository_tup = self.get_repository_tuple_for_installed_repository_manager(repository)
-            tool_dependency_tup = self.get_tool_dependency_tuple_for_installed_repository_manager(tool_dependency)
-            if repository_tup in self.installed_tool_dependencies_of_installed_repositories:
-                self.installed_tool_dependencies_of_installed_repositories[repository_tup].append(tool_dependency_tup)
-            else:
-                self.installed_tool_dependencies_of_installed_repositories[repository_tup] = [tool_dependency_tup]
-
-    def load_dependency_relationships(self):
-        """Load relationships for all installed repositories and tool dependencies into in-memnory dictionaries."""
-        # Get the list of installed tool shed repositories.
-        for repository in self.context.query(self.app.install_model.ToolShedRepository) \
-                                      .filter(self.app.install_model.ToolShedRepository.table.c.status ==
-                                              self.app.install_model.ToolShedRepository.installation_status.INSTALLED):
-            # Populate self.repository_dependencies_of_installed_repositories.
-            self.add_entry_to_repository_dependencies_of_installed_repositories(repository)
-            # Populate self.installed_repository_dependencies_of_installed_repositories.
-            self.add_entry_to_installed_repository_dependencies_of_installed_repositories(repository)
-            # Populate self.tool_dependencies_of_installed_repositories.
-            self.add_entry_to_tool_dependencies_of_installed_repositories(repository)
-            # Populate self.installed_tool_dependencies_of_installed_repositories.
-            self.add_entry_to_installed_tool_dependencies_of_installed_repositories(repository)
-        # Get the list of installed tool dependencies.
-        for tool_dependency in self.context.query(self.app.install_model.ToolDependency) \
-                                           .filter(self.app.install_model.ToolDependency.table.c.status ==
-                                                   self.app.install_model.ToolDependency.installation_status.INSTALLED):
-            # Populate self.runtime_tool_dependencies_of_installed_tool_dependencies.
-            self.add_entry_to_runtime_tool_dependencies_of_installed_tool_dependencies(tool_dependency)
-            # Populate self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies.
-            self.add_entry_to_installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies(tool_dependency)
-
     def load_proprietary_datatypes(self):
         cdl = custom_datatype_manager.CustomDatatypeLoader(self.app)
         for tool_shed_repository in self.context.query(self.install_model.ToolShedRepository) \
@@ -814,10 +727,6 @@ class InstalledRepositoryManager(object):
         if remove_from_disk:
             repository.status = self.app.install_model.ToolShedRepository.installation_status.UNINSTALLED
             repository.error_message = None
-            if self.app.config.manage_dependency_relationships:
-                # Remove the uninstalled repository and any tool dependencies from the in-memory dictionaries in the
-                # installed_repository_manager.
-                self.handle_repository_uninstall(repository)
         else:
             repository.status = self.app.install_model.ToolShedRepository.installation_status.DEACTIVATED
         self.app.install_model.context.current.add(repository)
diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml
index a723a9aec95..d8101b9a342 100644
--- a/lib/galaxy/webapps/galaxy/config_schema.yml
+++ b/lib/galaxy/webapps/galaxy/config_schema.yml
@@ -528,15 +528,6 @@ mapping:
           server process should be able to check for repository updates.  The
           setting for hours_between_check should be an integer between 1 and 24.
 
-      manage_dependency_relationships:
-        type: bool
-        default: false
-        required: false
-        desc: |
-          Enable use of an in-memory registry with bi-directional relationships between
-          repositories (i.e., in addition to lists of dependencies for a repository,
-          keep an in-memory registry of dependent items for each repository.
-
       tool_data_table_config_path:
         type: str
         default: config/tool_data_table_conf.xml

From add9e565aa151b41eb4e7e6e8d0a548c0a9095fc Mon Sep 17 00:00:00 2001
From: Matthias Bernt <bernt@informatik.uni-leipzig.de>
Date: Sun, 1 Mar 2020 00:23:07 +0100
Subject: [PATCH 08/71] fix options_filter_attribute: needs to be str

since in the tool xml the values can only be specified as str
also the attribute needs to be a str. currently, for instance,
filtering for non-str (e.g. int) metadata values does not work.

for instance this does not worked so far

```
    <param ... type="data" format="fasta">
      <options options_filter_attribute="metadata.sequences">
        <filter type="add_value" value="1"/>
      </options>
    </param>
```
---
 lib/galaxy/tools/parameters/basic.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index a6f830e746a..8f64a80c050 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -1913,7 +1913,7 @@ class DataToolParameter(BaseDataToolParameter):
             ref = getattr(ref, attribute)
         if call_attribute:
             ref = ref()
-        return ref
+        return str(ref)
 
     def to_dict(self, trans, other_values={}):
         # create dictionary and fill default parameters

From 666eb07bfbc06d45a8a2726cb90eeff954b72d22 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Sat, 29 Feb 2020 21:00:48 -0500
Subject: [PATCH 09/71] Render d3 instead of canvas as workflow noodle

---
 .../mvc/workflow/workflow-connector.js        | 88 +++++++++----------
 .../scripts/mvc/workflow/workflow-manager.js  |  6 +-
 .../mvc/workflow/workflow-view-terminals.js   |  8 +-
 client/galaxy/style/scss/workflow.scss        |  3 -
 4 files changed, 50 insertions(+), 55 deletions(-)

diff --git a/client/galaxy/scripts/mvc/workflow/workflow-connector.js b/client/galaxy/scripts/mvc/workflow/workflow-connector.js
index 7201d9162e2..3671b9c3f50 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-connector.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-connector.js
@@ -1,16 +1,17 @@
 import $ from "jquery";
-
+import * as d3 from "d3";
 import { Toast } from "ui/toast";
 
-function Connector(manager, handle1, handle2) {
+function Connector(manager) {
     this.manager = manager;
-    this.canvas = null;
     this.dragging = false;
     this.inner_color = "#FFFFFF";
     this.outer_color = "#25537b";
-    if (handle1 && handle2) {
-        this.connect(handle1, handle2);
-    }
+    this.canvas = document.createElement("div");
+    this.canvas.style.position = "absolute";
+    const container = document.getElementById("canvas-container");
+    container.appendChild(this.canvas);
+    this.svg = d3.select(this.canvas).append("svg");
 }
 $.extend(Connector.prototype, {
     connect: function(t1, t2) {
@@ -30,7 +31,7 @@ $.extend(Connector.prototype, {
         if (this.handle2) {
             this.handle2.disconnect(this);
         }
-        $(this.canvas).remove();
+        this.canvas.remove();
     },
     destroyIfInvalid: function(warn) {
         if (this.handle1 && this.handle2 && !this.handle2.attachable(this.handle1).canAccept) {
@@ -48,12 +49,8 @@ $.extend(Connector.prototype, {
         const canvasClass = `${startRibbon ? "start-ribbon" : ""} ${endRibbon ? "end-ribbon" : ""}`;
         var canvas_container = $("#canvas-container");
         var canvasZoom = this.manager.canvasZoom;
-        if (!this.canvas) {
-            this.canvas = document.createElement("canvas");
-            canvas_container.append($(this.canvas));
-            if (this.dragging) {
-                this.canvas.style.zIndex = "300";
-            }
+        if (this.dragging) {
+            this.canvas.style.zIndex = 1000;
         }
         this.canvas.setAttribute(
             "handle1-id",
@@ -73,6 +70,7 @@ $.extend(Connector.prototype, {
         var start_y = relativeTop(handle1.element) + 5;
         var end_x = relativeLeft(handle2.element) + 5;
         var end_y = relativeTop(handle2.element) + 5;
+
         // Calculate canvas area
         var canvas_extra = 100;
         var canvas_min_x = Math.min(start_x, end_x);
@@ -84,20 +82,32 @@ $.extend(Connector.prototype, {
         var canvas_top = canvas_min_y - canvas_extra;
         var canvas_width = canvas_max_x - canvas_min_x + 2 * canvas_extra;
         var canvas_height = canvas_max_y - canvas_min_y + 2 * canvas_extra;
+
         // Place the canvas
         this.canvas.style.left = `${canvas_left}px`;
         this.canvas.style.top = `${canvas_top}px`;
-        this.canvas.setAttribute("width", canvas_width);
-        this.canvas.setAttribute("height", canvas_height);
+        this.svg.style("width", `${canvas_width}px`);
+        this.svg.style("height", `${canvas_height}px`);
         this.canvas.setAttribute("class", canvasClass);
+
         // Adjust points to be relative to the canvas
         start_x -= canvas_left;
         start_y -= canvas_top;
         end_x -= canvas_left;
         end_y -= canvas_top;
 
-        // Draw the line
+        this.draw_outlined_curve(
+            start_x,
+            start_y,
+            end_x,
+            end_y,
+            0,
+            0,
+            0,
+            0
+        );
 
+        /*/ Draw the line
         var start_offsets = null;
         var end_offsets = null;
         var num_offsets = 1;
@@ -133,7 +143,7 @@ $.extend(Connector.prototype, {
                 start_offsets[i % start_offsets.length],
                 end_offsets[i % end_offsets.length]
             );
-        }
+        }*/
     },
     draw_outlined_curve: function(
         start_x,
@@ -146,37 +156,23 @@ $.extend(Connector.prototype, {
         offset_start,
         offset_end
     ) {
-        var c = this.canvas.getContext("2d");
         offset_start = offset_start || 0;
         offset_end = offset_end || 0;
-        c.lineCap = "round";
-        c.strokeStyle = this.outer_color;
-        c.lineWidth = outer_width;
-        c.beginPath();
-        c.moveTo(start_x, start_y + offset_start);
-        c.bezierCurveTo(
-            start_x + cp_shift,
-            start_y + offset_start,
-            end_x - cp_shift,
-            end_y + offset_end,
-            end_x,
-            end_y + offset_end
-        );
-        c.stroke();
-        // Inner line
-        c.strokeStyle = this.inner_color;
-        c.lineWidth = inner_width;
-        c.beginPath();
-        c.moveTo(start_x, start_y + offset_start);
-        c.bezierCurveTo(
-            start_x + cp_shift,
-            start_y + offset_start,
-            end_x - cp_shift,
-            end_y + offset_end,
-            end_x,
-            end_y + offset_end
-        );
-        c.stroke();
+        var lineData = [
+            { x: start_x, y: start_y + offset_start },
+            //{ x: start_x + cp_shift,   y: start_y + offset_start},
+            //{ x: end_x - cp_shift,  y: end_y + offset_end },
+            { x: end_x,  y: end_y + offset_end } ];
+        var lineFunction = d3.svg.line()
+                            .x(function(d) { return d.x; })
+                            .y(function(d) { return d.y; })
+                            .interpolate("linear");
+        // The line SVG Path we draw
+        this.svg.selectAll("*").remove();
+        this.svg.append("path")
+                .attr("d", lineFunction(lineData))
+                .attr("stroke", "blue")
+                .attr("stroke-width", 2);
     }
 });
 export default Connector;
diff --git a/client/galaxy/scripts/mvc/workflow/workflow-manager.js b/client/galaxy/scripts/mvc/workflow/workflow-manager.js
index cc8f7cf83ac..ada79e2c245 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-manager.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-manager.js
@@ -380,15 +380,15 @@ class Workflow extends EventEmitter {
         wf.id_counter = max_id + 1;
         // Second pass, connections
         $.each(data.steps, (id, step) => {
-            var node = wf.nodes[parseInt(id) + offset];
+            const node = wf.nodes[parseInt(id) + offset];
             $.each(step.input_connections, (k, v) => {
                 if (v) {
                     if (!$.isArray(v)) {
                         v = [v];
                     }
                     $.each(v, (l, x) => {
-                        var other_node = wf.nodes[parseInt(x.id) + offset];
-                        var c = new Connector(this.canvas_manager);
+                        const other_node = wf.nodes[parseInt(x.id) + offset];
+                        const c = new Connector(this.canvas_manager);
                         c.connect(other_node.output_terminals[x.output_name], node.input_terminals[k]);
                         c.redraw();
                     });
diff --git a/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js b/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js
index 546fead9497..a6c4886f11e 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js
@@ -114,7 +114,9 @@ class BaseInputTerminalView extends TerminalView {
         d.proxy.dropTooltip = "";
         if (this.$el.hasClass("can-accept")) {
             const terminal = this.el.terminal;
-            new Connector(this.app.canvas_manager, d.drag.terminal, terminal).redraw();
+            const c = new Connector(this.app.canvas_manager);
+            c.connect(d.drag.terminal, terminal);
+            c.redraw();
         }
     }
     onHover() {
@@ -251,7 +253,7 @@ export class BaseOutputTerminalView extends TerminalView {
                     break;
                 case 32: // Space
                     removeMenu();
-                    new Connector(this.app.canvas_manager, this.el.terminal, inputTerminal).redraw();
+                    new Connector(this.app.canvas_manager).connect(this.el.terminal, inputTerminal).redraw();
                     ariaAlert("Node connected");
 
                     if (inputTerminal.connectors.length > 0) {
@@ -357,7 +359,7 @@ export class BaseOutputTerminalView extends TerminalView {
             }
         });
         h.terminal = new Terminals.OutputTerminal({ element: h });
-        var c = new Connector(this.app.canvas_manager);
+        const c = new Connector(this.app.canvas_manager);
         c.dragging = true;
         c.connect(this.el.terminal, h.terminal);
         return h;
diff --git a/client/galaxy/style/scss/workflow.scss b/client/galaxy/style/scss/workflow.scss
index 5061dc8b904..4227ff58d0b 100644
--- a/client/galaxy/style/scss/workflow.scss
+++ b/client/galaxy/style/scss/workflow.scss
@@ -23,9 +23,6 @@
                 @extend .px-2;
                 cursor: move;
                 background: $brand-secondary;
-                .header {
-                    width: 7rem;
-                }
             }
             .node-body {
                 @extend .card-body;

From 078e50c6e7ab858580d1323e35ef1b0fdaf6d4ba Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Mon, 2 Mar 2020 11:39:25 -0500
Subject: [PATCH 10/71] Use const/let in connector, restore curve loop, use d3

---
 .../mvc/workflow/workflow-connector.js        | 93 +++++++++----------
 1 file changed, 44 insertions(+), 49 deletions(-)

diff --git a/client/galaxy/scripts/mvc/workflow/workflow-connector.js b/client/galaxy/scripts/mvc/workflow/workflow-connector.js
index 3671b9c3f50..23b390ae78d 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-connector.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-connector.js
@@ -46,9 +46,8 @@ $.extend(Connector.prototype, {
         const handle2 = this.handle2;
         const startRibbon = handle1 && handle1.isMappedOver();
         const endRibbon = handle2 && handle2.isMappedOver();
-        const canvasClass = `${startRibbon ? "start-ribbon" : ""} ${endRibbon ? "end-ribbon" : ""}`;
-        var canvas_container = $("#canvas-container");
-        var canvasZoom = this.manager.canvasZoom;
+        const canvas_container = $("#canvas-container");
+        const canvasZoom = this.manager.canvasZoom;
         if (this.dragging) {
             this.canvas.style.zIndex = 1000;
         }
@@ -60,35 +59,33 @@ $.extend(Connector.prototype, {
             "handle2-id",
             handle2 && handle2.element.getAttribute ? handle2.element.getAttribute("id") : ""
         );
-        var relativeLeft = e => ($(e).offset().left - canvas_container.offset().left) / canvasZoom;
-        var relativeTop = e => ($(e).offset().top - canvas_container.offset().top) / canvasZoom;
+        const relativeLeft = e => ($(e).offset().left - canvas_container.offset().left) / canvasZoom;
+        const relativeTop = e => ($(e).offset().top - canvas_container.offset().top) / canvasZoom;
         if (!handle1 || !handle2) {
             return;
         }
         // Find the position of each handle
-        var start_x = relativeLeft(handle1.element) + 5;
-        var start_y = relativeTop(handle1.element) + 5;
-        var end_x = relativeLeft(handle2.element) + 5;
-        var end_y = relativeTop(handle2.element) + 5;
+        let start_x = relativeLeft(handle1.element) + 5;
+        let start_y = relativeTop(handle1.element) + 5;
+        let end_x = relativeLeft(handle2.element) + 5;
+        let end_y = relativeTop(handle2.element) + 5;
 
         // Calculate canvas area
-        var canvas_extra = 100;
-        var canvas_min_x = Math.min(start_x, end_x);
-        var canvas_max_x = Math.max(start_x, end_x);
-        var canvas_min_y = Math.min(start_y, end_y);
-        var canvas_max_y = Math.max(start_y, end_y);
-        var cp_shift = Math.min(Math.max(Math.abs(canvas_max_y - canvas_min_y) / 2, 100), 300);
-        var canvas_left = canvas_min_x - canvas_extra;
-        var canvas_top = canvas_min_y - canvas_extra;
-        var canvas_width = canvas_max_x - canvas_min_x + 2 * canvas_extra;
-        var canvas_height = canvas_max_y - canvas_min_y + 2 * canvas_extra;
+        const canvas_extra = 100;
+        const canvas_min_x = Math.min(start_x, end_x);
+        const canvas_max_x = Math.max(start_x, end_x);
+        const canvas_min_y = Math.min(start_y, end_y);
+        const canvas_max_y = Math.max(start_y, end_y);
+        const canvas_left = canvas_min_x - canvas_extra;
+        const canvas_top = canvas_min_y - canvas_extra;
+        const canvas_width = canvas_max_x - canvas_min_x + 2 * canvas_extra;
+        const canvas_height = canvas_max_y - canvas_min_y + 2 * canvas_extra;
 
         // Place the canvas
         this.canvas.style.left = `${canvas_left}px`;
         this.canvas.style.top = `${canvas_top}px`;
         this.svg.style("width", `${canvas_width}px`);
         this.svg.style("height", `${canvas_height}px`);
-        this.canvas.setAttribute("class", canvasClass);
 
         // Adjust points to be relative to the canvas
         start_x -= canvas_left;
@@ -96,21 +93,14 @@ $.extend(Connector.prototype, {
         end_x -= canvas_left;
         end_y -= canvas_top;
 
-        this.draw_outlined_curve(
-            start_x,
-            start_y,
-            end_x,
-            end_y,
-            0,
-            0,
-            0,
-            0
-        );
+        // Determine line shift
+        const cp_factor = 10;
+        const cp_shift = Math.min(Math.max(Math.abs(canvas_max_y - canvas_min_y) / 2, cp_factor), 300 / cp_factor);
 
-        /*/ Draw the line
-        var start_offsets = null;
-        var end_offsets = null;
-        var num_offsets = 1;
+        // Draw the line
+        let start_offsets = null;
+        let end_offsets = null;
+        let num_offsets = 1;
         if (startRibbon) {
             start_offsets = [-6, -3, 0, 3, 6];
             num_offsets = 5;
@@ -123,16 +113,15 @@ $.extend(Connector.prototype, {
         } else {
             end_offsets = [0];
         }
-        var connector = this;
-        for (var i = 0; i < num_offsets; i++) {
-            var inner_width = 5;
-            var outer_width = 7;
+        for (let i = 0; i < num_offsets; i++) {
+            let inner_width = 5;
+            let outer_width = 7;
             if (start_offsets.length > 1 || end_offsets.length > 1) {
                 // We have a multi-run, using many lines, make them small.
                 inner_width = 1;
                 outer_width = 3;
             }
-            connector.draw_outlined_curve(
+            this.draw_outlined_curve(
                 start_x,
                 start_y,
                 end_x,
@@ -143,7 +132,7 @@ $.extend(Connector.prototype, {
                 start_offsets[i % start_offsets.length],
                 end_offsets[i % end_offsets.length]
             );
-        }*/
+        }
     },
     draw_outlined_curve: function(
         start_x,
@@ -158,21 +147,27 @@ $.extend(Connector.prototype, {
     ) {
         offset_start = offset_start || 0;
         offset_end = offset_end || 0;
-        var lineData = [
+        const lineData = [
             { x: start_x, y: start_y + offset_start },
-            //{ x: start_x + cp_shift,   y: start_y + offset_start},
-            //{ x: end_x - cp_shift,  y: end_y + offset_end },
+            { x: start_x + cp_shift,   y: start_y + offset_start},
+            { x: end_x - cp_shift,  y: end_y + offset_end },
             { x: end_x,  y: end_y + offset_end } ];
-        var lineFunction = d3.svg.line()
-                            .x(function(d) { return d.x; })
-                            .y(function(d) { return d.y; })
-                            .interpolate("linear");
+        const lineFunction = d3.svg.line()
+                                   .x(function(d) { return d.x; })
+                                   .y(function(d) { return d.y; })
+                                   .interpolate("basis");
         // The line SVG Path we draw
         this.svg.selectAll("*").remove();
         this.svg.append("path")
                 .attr("d", lineFunction(lineData))
-                .attr("stroke", "blue")
-                .attr("stroke-width", 2);
+                .attr("stroke", this.inner_color)
+                .attr("stroke-width", inner_width)
+                .attr("fill", "none");
+        this.svg.append("path")
+                .attr("d", lineFunction(lineData))
+                .attr("stroke", this.outer_color)
+                .attr("stroke-width", outer_width)
+                .attr("fill", "none");
     }
 });
 export default Connector;

From 47f590c929e6dcb4b0b9b99ab798006498f1dd81 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Mon, 2 Mar 2020 11:41:37 -0500
Subject: [PATCH 11/71] Switch connector to es6 class

---
 .../mvc/workflow/workflow-connector.js        | 44 +++++++++----------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/client/galaxy/scripts/mvc/workflow/workflow-connector.js b/client/galaxy/scripts/mvc/workflow/workflow-connector.js
index 23b390ae78d..7e777c4ae25 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-connector.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-connector.js
@@ -2,19 +2,19 @@ import $ from "jquery";
 import * as d3 from "d3";
 import { Toast } from "ui/toast";
 
-function Connector(manager) {
-    this.manager = manager;
-    this.dragging = false;
-    this.inner_color = "#FFFFFF";
-    this.outer_color = "#25537b";
-    this.canvas = document.createElement("div");
-    this.canvas.style.position = "absolute";
-    const container = document.getElementById("canvas-container");
-    container.appendChild(this.canvas);
-    this.svg = d3.select(this.canvas).append("svg");
-}
-$.extend(Connector.prototype, {
-    connect: function(t1, t2) {
+class Connector {
+    constructor(manager) {
+        this.manager = manager;
+        this.dragging = false;
+        this.inner_color = "#FFFFFF";
+        this.outer_color = "#25537b";
+        this.canvas = document.createElement("div");
+        this.canvas.style.position = "absolute";
+        const container = document.getElementById("canvas-container");
+        container.appendChild(this.canvas);
+        this.svg = d3.select(this.canvas).append("svg");
+    }
+    connect(t1, t2) {
         this.handle1 = t1;
         if (this.handle1) {
             this.handle1.connect(this);
@@ -23,8 +23,8 @@ $.extend(Connector.prototype, {
         if (this.handle2) {
             this.handle2.connect(this);
         }
-    },
-    destroy: function() {
+    }
+    destroy() {
         if (this.handle1) {
             this.handle1.disconnect(this);
         }
@@ -32,16 +32,16 @@ $.extend(Connector.prototype, {
             this.handle2.disconnect(this);
         }
         this.canvas.remove();
-    },
-    destroyIfInvalid: function(warn) {
+    }
+    destroyIfInvalid(warn) {
         if (this.handle1 && this.handle2 && !this.handle2.attachable(this.handle1).canAccept) {
             if (warn) {
                 Toast.warning("Destroying a connection because collection type has changed.");
             }
             this.destroy();
         }
-    },
-    redraw: function() {
+    }
+    redraw() {
         const handle1 = this.handle1;
         const handle2 = this.handle2;
         const startRibbon = handle1 && handle1.isMappedOver();
@@ -133,8 +133,8 @@ $.extend(Connector.prototype, {
                 end_offsets[i % end_offsets.length]
             );
         }
-    },
-    draw_outlined_curve: function(
+    }
+    draw_outlined_curve(
         start_x,
         start_y,
         end_x,
@@ -169,5 +169,5 @@ $.extend(Connector.prototype, {
                 .attr("stroke-width", outer_width)
                 .attr("fill", "none");
     }
-});
+}
 export default Connector;

From 76455e9fc46f9ca9946467ba45a1472b6b96cc5e Mon Sep 17 00:00:00 2001
From: Ralf Weber <RJMW@users.noreply.github.com>
Date: Mon, 2 Mar 2020 17:18:21 +0000
Subject: [PATCH 12/71] Add datatypes for xcms and camera

---
 lib/galaxy/config/sample/datatypes_conf.xml.sample | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/galaxy/config/sample/datatypes_conf.xml.sample b/lib/galaxy/config/sample/datatypes_conf.xml.sample
index b5301b9a427..8a92bd8e970 100644
--- a/lib/galaxy/config/sample/datatypes_conf.xml.sample
+++ b/lib/galaxy/config/sample/datatypes_conf.xml.sample
@@ -563,6 +563,14 @@
     <datatype extension="gal" type="galaxy.datatypes.microarrays:Gal" display_in_upload="true"/>
     <datatype extension="rdata" type="galaxy.datatypes.binary:RData" display_in_upload="true" description="Stored data from an R session"/>
     <datatype extension="rdata.sce" type="galaxy.datatypes.binary:RData" description="Stored RDS from a SingleCellObject" subclass="true" display_in_upload="true"/>
+    <datatype extension="rdata.msnbase.raw" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
+    <datatype extension="rdata.xcms.findchrompeaks" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
+    <datatype extension="rdata.xcms.group" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
+    <datatype extension="rdata.xcms.retcor" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
+    <datatype extension="rdata.xcms.fillpeaks" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
+    <datatype extension="rdata.camera.positive" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
+    <datatype extension="rdata.camera.negative" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
+    <datatype extension="rdata.camera.quick" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
     <datatype extension="rdock_as" type="galaxy.datatypes.binary:Binary" description="rDock active site format" subclass="true" display_in_upload="true"/>
     <datatype extension="oxlicg" type="galaxy.datatypes.binary:OxliCountGraph" mimetype="application/octet-stream" display_in_upload="true"/>
     <datatype extension="oxling" type="galaxy.datatypes.binary:OxliNodeGraph" mimetype="application/octet-stream" display_in_upload="true"/>

From 37dfd4292a69e4f76dcec785361649956eb1c8ca Mon Sep 17 00:00:00 2001
From: Helena Rasche <hxr@hx42.org>
Date: Fri, 1 Feb 2019 18:45:52 +0100
Subject: [PATCH 13/71] Fix?

---
 lib/galaxy/objectstore/s3.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py
index a2577e8464e..09e78626e24 100644
--- a/lib/galaxy/objectstore/s3.py
+++ b/lib/galaxy/objectstore/s3.py
@@ -378,8 +378,6 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
         except S3ResponseError:
             log.exception("Trouble checking existence of S3 key '%s'", rel_path)
             return False
-        if rel_path[0] == '/':
-            raise
         return exists
 
     def _in_cache(self, rel_path):

From 98cbc672b8dc1da48dcd85811db4e1da992bcd5c Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Mon, 2 Mar 2020 13:18:01 -0500
Subject: [PATCH 14/71] Remove static ribbon colors, assign brand colors in css
 instead

---
 .../mvc/workflow/workflow-connector.js        | 147 ++++++++++--------
 .../mvc/workflow/workflow-view-terminals.js   |   6 +-
 client/galaxy/style/scss/workflow.scss        |  14 ++
 3 files changed, 102 insertions(+), 65 deletions(-)

diff --git a/client/galaxy/scripts/mvc/workflow/workflow-connector.js b/client/galaxy/scripts/mvc/workflow/workflow-connector.js
index 7e777c4ae25..02cb0ce3a67 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-connector.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-connector.js
@@ -2,17 +2,29 @@ import $ from "jquery";
 import * as d3 from "d3";
 import { Toast } from "ui/toast";
 
+// Rendering parameters
+const zIndex = 1000;
+const cpFactor = 10;
+const canvasExtra = 100;
+const handleMarginX = 8;
+const handleMarginY = 5;
+const ribbonMargin = 4;
+const ribbonInnerSingle = 4;
+const ribbonOuterSingle = 6;
+const ribbonInnerMultiple = 1;
+const ribbonOuterMultiple = 3;
+
 class Connector {
     constructor(manager) {
         this.manager = manager;
         this.dragging = false;
-        this.inner_color = "#FFFFFF";
-        this.outer_color = "#25537b";
+        this.innerClass = "ribbon-inner";
         this.canvas = document.createElement("div");
         this.canvas.style.position = "absolute";
         const container = document.getElementById("canvas-container");
         container.appendChild(this.canvas);
         this.svg = d3.select(this.canvas).append("svg");
+        this.svg.attr("class", "ribbon");
     }
     connect(t1, t2) {
         this.handle1 = t1;
@@ -41,15 +53,24 @@ class Connector {
             this.destroy();
         }
     }
+    dropStart(canAccept) {
+        if (canAccept) {
+            this.innerClass = "ribbon-inner-valid";
+        } else {
+            this.innerClass = "ribbon-inner-invalid";
+        }
+    }
+    dropEnd() {
+        this.innerClass = "ribbon-inner";
+    }
     redraw() {
+        // Identify handles
         const handle1 = this.handle1;
         const handle2 = this.handle2;
-        const startRibbon = handle1 && handle1.isMappedOver();
-        const endRibbon = handle2 && handle2.isMappedOver();
         const canvas_container = $("#canvas-container");
         const canvasZoom = this.manager.canvasZoom;
         if (this.dragging) {
-            this.canvas.style.zIndex = 1000;
+            this.canvas.style.zIndex = zIndex;
         }
         this.canvas.setAttribute(
             "handle1-id",
@@ -64,26 +85,28 @@ class Connector {
         if (!handle1 || !handle2) {
             return;
         }
+
         // Find the position of each handle
-        let start_x = relativeLeft(handle1.element) + 5;
-        let start_y = relativeTop(handle1.element) + 5;
-        let end_x = relativeLeft(handle2.element) + 5;
-        let end_y = relativeTop(handle2.element) + 5;
+        let start_x = relativeLeft(handle1.element) + handleMarginX;
+        let start_y = relativeTop(handle1.element) + handleMarginY;
+        let end_x = relativeLeft(handle2.element) + handleMarginX;
+        let end_y = relativeTop(handle2.element) + handleMarginY;
 
         // Calculate canvas area
-        const canvas_extra = 100;
         const canvas_min_x = Math.min(start_x, end_x);
         const canvas_max_x = Math.max(start_x, end_x);
         const canvas_min_y = Math.min(start_y, end_y);
         const canvas_max_y = Math.max(start_y, end_y);
-        const canvas_left = canvas_min_x - canvas_extra;
-        const canvas_top = canvas_min_y - canvas_extra;
-        const canvas_width = canvas_max_x - canvas_min_x + 2 * canvas_extra;
-        const canvas_height = canvas_max_y - canvas_min_y + 2 * canvas_extra;
+        const canvas_left = canvas_min_x - canvasExtra;
+        const canvas_top = canvas_min_y - canvasExtra;
+        const canvas_width = canvas_max_x - canvas_min_x + 2 * canvasExtra;
+        const canvas_height = canvas_max_y - canvas_min_y + 2 * canvasExtra;
 
         // Place the canvas
         this.canvas.style.left = `${canvas_left}px`;
         this.canvas.style.top = `${canvas_top}px`;
+
+        // Resize the svg
         this.svg.style("width", `${canvas_width}px`);
         this.svg.style("height", `${canvas_height}px`);
 
@@ -94,34 +117,38 @@ class Connector {
         end_y -= canvas_top;
 
         // Determine line shift
-        const cp_factor = 10;
-        const cp_shift = Math.min(Math.max(Math.abs(canvas_max_y - canvas_min_y) / 2, cp_factor), 300 / cp_factor);
+        const cp_shift = Math.min(Math.max(Math.abs(canvas_max_y - canvas_min_y) / 2, cpFactor), 300 / cpFactor);
+
+        // Draw ribbons
+        this.drawRibbon(handle1, handle2, cp_shift, start_x, start_y, end_x, end_y);
+    }
+    drawRibbon(handle1, handle2, cp_shift, start_x, start_y, end_x, end_y) {
+        // Check ribbon type
+        const startRibbon = handle1 && handle1.isMappedOver();
+        const endRibbon = handle2 && handle2.isMappedOver();
 
         // Draw the line
-        let start_offsets = null;
-        let end_offsets = null;
+        let start_offsets = [0];
+        let end_offsets = [0];
+        const offsets = [-2 * ribbonMargin, -ribbonMargin, 0, ribbonMargin, 2 * ribbonMargin];
         let num_offsets = 1;
         if (startRibbon) {
-            start_offsets = [-6, -3, 0, 3, 6];
-            num_offsets = 5;
-        } else {
-            start_offsets = [0];
+            start_offsets = offsets;
+            num_offsets = offsets.length;
         }
         if (endRibbon) {
-            end_offsets = [-6, -3, 0, 3, 6];
-            num_offsets = 5;
-        } else {
-            end_offsets = [0];
+            end_offsets = offsets;
+            num_offsets = offsets.length;
         }
+        this.svg.selectAll("*").remove();
         for (let i = 0; i < num_offsets; i++) {
-            let inner_width = 5;
-            let outer_width = 7;
+            let inner_width = ribbonInnerSingle;
+            let outer_width = ribbonOuterSingle;
             if (start_offsets.length > 1 || end_offsets.length > 1) {
-                // We have a multi-run, using many lines, make them small.
-                inner_width = 1;
-                outer_width = 3;
+                inner_width = ribbonInnerMultiple;
+                outer_width = ribbonOuterMultiple;
             }
-            this.draw_outlined_curve(
+            this.drawCurve(
                 start_x,
                 start_y,
                 end_x,
@@ -134,40 +161,36 @@ class Connector {
             );
         }
     }
-    draw_outlined_curve(
-        start_x,
-        start_y,
-        end_x,
-        end_y,
-        cp_shift,
-        inner_width,
-        outer_width,
-        offset_start,
-        offset_end
-    ) {
+    drawCurve(start_x, start_y, end_x, end_y, cp_shift, inner_width, outer_width, offset_start, offset_end) {
         offset_start = offset_start || 0;
         offset_end = offset_end || 0;
         const lineData = [
             { x: start_x, y: start_y + offset_start },
-            { x: start_x + cp_shift,   y: start_y + offset_start},
-            { x: end_x - cp_shift,  y: end_y + offset_end },
-            { x: end_x,  y: end_y + offset_end } ];
-        const lineFunction = d3.svg.line()
-                                   .x(function(d) { return d.x; })
-                                   .y(function(d) { return d.y; })
-                                   .interpolate("basis");
-        // The line SVG Path we draw
-        this.svg.selectAll("*").remove();
-        this.svg.append("path")
-                .attr("d", lineFunction(lineData))
-                .attr("stroke", this.inner_color)
-                .attr("stroke-width", inner_width)
-                .attr("fill", "none");
-        this.svg.append("path")
-                .attr("d", lineFunction(lineData))
-                .attr("stroke", this.outer_color)
-                .attr("stroke-width", outer_width)
-                .attr("fill", "none");
+            { x: start_x + cp_shift, y: start_y + offset_start },
+            { x: end_x - cp_shift, y: end_y + offset_end },
+            { x: end_x, y: end_y + offset_end }
+        ];
+        const lineFunction = d3.svg
+            .line()
+            .x(function(d) {
+                return d.x;
+            })
+            .y(function(d) {
+                return d.y;
+            })
+            .interpolate("basis");
+        this.svg
+            .append("path")
+            .attr("class", "ribbon-outer")
+            .attr("d", lineFunction(lineData))
+            .attr("stroke-width", outer_width)
+            .attr("fill", "none");
+        this.svg
+            .append("path")
+            .attr("class", this.innerClass)
+            .attr("d", lineFunction(lineData))
+            .attr("stroke-width", inner_width)
+            .attr("fill", "none");
     }
 }
 export default Connector;
diff --git a/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js b/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js
index a6c4886f11e..f52c9eb33f7 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js
@@ -91,10 +91,10 @@ class BaseInputTerminalView extends TerminalView {
     onDropStart(e, d = {}) {
         if (d.proxy.terminal) {
             if (this.$el.hasClass("can-accept")) {
-                d.proxy.terminal.connectors[0].inner_color = "#BBFFBB";
+                d.proxy.terminal.connectors[0].dropStart(true);
                 d.proxy.dropTooltip = "";
             } else {
-                d.proxy.terminal.connectors[0].inner_color = "#fe7f02";
+                d.proxy.terminal.connectors[0].dropStart(false);
                 if (this.reason) {
                     d.proxy.dropTooltip = this.reason;
                     $(d.proxy).tooltip("show");
@@ -107,7 +107,7 @@ class BaseInputTerminalView extends TerminalView {
     onDropEnd(e, d = {}) {
         d.proxy.dropTooltip = "";
         if (d.proxy.terminal) {
-            d.proxy.terminal.connectors[0].inner_color = "#FFFFFF";
+            d.proxy.terminal.connectors[0].dropEnd();
         }
     }
     onDrop(e, d = {}) {
diff --git a/client/galaxy/style/scss/workflow.scss b/client/galaxy/style/scss/workflow.scss
index 4227ff58d0b..de03e0c8d3b 100644
--- a/client/galaxy/style/scss/workflow.scss
+++ b/client/galaxy/style/scss/workflow.scss
@@ -55,6 +55,20 @@
                 position: relative;
                 overflow: hidden;
             }
+            .ribbon {
+                .ribbon-outer {
+                    stroke: $brand-primary;
+                }
+                .ribbon-inner {
+                    stroke: $white;
+                }
+                .ribbon-inner-valid {
+                    stroke: $brand-success;
+                }
+                .ribbon-inner-invalid {
+                    stroke: $brand-warning;
+                }
+            }
             .base-terminal {
                 @extend .fa;
                 @extend .fa-circle;

From fc503789451cffafd73e979043601e70bf4ef888 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Mon, 2 Mar 2020 15:03:17 -0500
Subject: [PATCH 15/71] Fix qunit workflow test cases

---
 .../mvc/workflow/workflow-connector.js        |  5 +++-
 .../mvc/workflow/workflow-view-terminals.js   |  5 ++--
 .../qunit/tests/workflow_editor_tests.js      | 23 ++++++++-----------
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/client/galaxy/scripts/mvc/workflow/workflow-connector.js b/client/galaxy/scripts/mvc/workflow/workflow-connector.js
index 02cb0ce3a67..9f107e01199 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-connector.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-connector.js
@@ -15,7 +15,7 @@ const ribbonInnerMultiple = 1;
 const ribbonOuterMultiple = 3;
 
 class Connector {
-    constructor(manager) {
+    constructor(manager = {}, handle1 = null, handle2 = null) {
         this.manager = manager;
         this.dragging = false;
         this.innerClass = "ribbon-inner";
@@ -25,6 +25,9 @@ class Connector {
         container.appendChild(this.canvas);
         this.svg = d3.select(this.canvas).append("svg");
         this.svg.attr("class", "ribbon");
+        if (handle1 && handle2) {
+            this.connect(handle1, handle2);
+        }
     }
     connect(t1, t2) {
         this.handle1 = t1;
diff --git a/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js b/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js
index f52c9eb33f7..c481940b653 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js
@@ -114,8 +114,7 @@ class BaseInputTerminalView extends TerminalView {
         d.proxy.dropTooltip = "";
         if (this.$el.hasClass("can-accept")) {
             const terminal = this.el.terminal;
-            const c = new Connector(this.app.canvas_manager);
-            c.connect(d.drag.terminal, terminal);
+            const c = new Connector(this.app.canvas_manager, d.drag.terminal, terminal);
             c.redraw();
         }
     }
@@ -253,7 +252,7 @@ export class BaseOutputTerminalView extends TerminalView {
                     break;
                 case 32: // Space
                     removeMenu();
-                    new Connector(this.app.canvas_manager).connect(this.el.terminal, inputTerminal).redraw();
+                    new Connector(this.app.canvas_manager, this.el.terminal, inputTerminal).redraw();
                     ariaAlert("Node connected");
 
                     if (inputTerminal.connectors.length > 0) {
diff --git a/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js b/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js
index 116ecb708d2..643564797f8 100644
--- a/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js
+++ b/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js
@@ -242,9 +242,7 @@ QUnit.test("default attributes", function(assert) {
     const output = { connect: sinon.spy() };
     const connector = new Connector({}, input, output);
     assert.equal(connector.dragging, false);
-    assert.equal(connector.canvas, null);
-    assert.equal(connector.inner_color, "#FFFFFF");
-    assert.equal(connector.outer_color, "#25537b");
+    assert.equal(connector.svg.attr("class"), "ribbon");
 });
 
 QUnit.test("destroy", function(assert) {
@@ -271,13 +269,12 @@ QUnit.test("initial redraw", function(assert) {
             return false;
         }
     };
+    const n = $("#canvas-container").find("svg").length;
     const connector = new Connector({}, input, output);
-    const n = $("#canvas-container").find("canvas").length;
-    connector.redraw();
     // Ensure canvas gets set
     assert.ok(connector.canvas);
     // Ensure it got added to canvas container
-    assert.equal(n + 1, $("#canvas-container").find("canvas").length);
+    assert.equal(n + 1, $("#canvas-container").find("svg").length);
 });
 
 QUnit.module("Input collection terminal model test", {
@@ -366,9 +363,9 @@ QUnit.module("Node unit test", {
 });
 
 QUnit.test("make active", function(assert) {
-    assert.ok(!this.element.hasClass("toolForm-active"));
+    assert.ok(!this.element.hasClass("node-active"));
     this.node.make_active();
-    assert.ok(this.element.hasClass("toolForm-active"));
+    assert.ok(this.element.hasClass("node-active"));
 });
 
 QUnit.test("destroy", function(assert) {
@@ -384,7 +381,7 @@ QUnit.test("error", function(assert) {
     var test = this;
     this.expect_workflow_node_changed(assert, function() {
         test.node.error("TOOL ERROR");
-        assert.equal($(test.$(".toolFormBody").children()[0]).html(), "TOOL ERROR");
+        assert.equal($(test.$(".node-body").children()[0]).html(), "TOOL ERROR");
     });
 });
 
@@ -428,10 +425,10 @@ QUnit.test("init_field_data data", function(assert) {
         assert.equal(this.$(".output-terminal").length, 1);
         assert.equal(this.$(".input-terminal").length, 1);
         assert.equal(this.$(".rule").length, 1);
-        assert.equal(this.$(".toolFormBody").children().length, 3);
+        assert.equal(this.$(".node-body").children().length, 3);
         assert.equal(this.$(".node-title").text(), "newnode");
         assert.ok(
-            this.$(".toolFormTitle")
+            this.$(".node-header")
                 .find("i")
                 .hasClass("fa-wrench")
         );
@@ -607,10 +604,10 @@ QUnit.module("Node view ", {
 QUnit.test("tool error styling", function(assert) {
     this.set_for_node({ errors: false });
     this.view.render();
-    assert.ok(!this.view.$el.hasClass("tool-node-error"));
+    assert.ok(!this.view.$el.hasClass("node-error"));
     this.set_for_node({ errors: true });
     this.view.render();
-    assert.ok(this.view.$el.hasClass("tool-node-error"));
+    assert.ok(this.view.$el.hasClass("node-error"));
 });
 
 QUnit.test("rendering correct width", function(assert) {

From 2be3b473f694c841681d14bdf9da7729ca497537 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Mon, 2 Mar 2020 15:43:52 -0500
Subject: [PATCH 16/71] Fix error handling, display error message in node body

---
 .../scripts/mvc/workflow/workflow-node.js     | 12 ++-------
 .../mvc/workflow/workflow-view-node.js        | 25 +++++++++++--------
 .../qunit/tests/workflow_editor_tests.js      |  6 ++---
 3 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/client/galaxy/scripts/mvc/workflow/workflow-node.js b/client/galaxy/scripts/mvc/workflow/workflow-node.js
index 72ca51e2754..94938aff0f2 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-node.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-node.js
@@ -10,7 +10,7 @@ export class Node {
         this.element = attr.element;
         this.input_terminals = {};
         this.output_terminals = {};
-        this.errors = {};
+        this.errors = null;
         this.workflow_outputs = [];
     }
     getWorkflowOutput(outputName) {
@@ -309,7 +309,7 @@ export class Node {
             var pja_in = data.post_job_actions;
             this.post_job_actions = pja_in ? pja_in : {};
         }
-        node.nodeView.renderToolErrors();
+        node.nodeView.renderErrors();
         // Update input rows
         var old_body = nodeView.$el.find("div.inputs");
         var new_body = nodeView.newInputsDiv();
@@ -340,14 +340,6 @@ export class Node {
         this.markChanged();
         this.redraw();
     }
-    error(text) {
-        var b = $(this.element).find(".node-body");
-        b.find("div").remove();
-        var tmp = `<div style='color: red; text-style: italic;'>${text}</div>`;
-        this.config_form = tmp;
-        b.html(tmp);
-        this.app.node_changed(this);
-    }
     markChanged() {
         this.app.node_changed(this);
     }
diff --git a/client/galaxy/scripts/mvc/workflow/workflow-view-node.js b/client/galaxy/scripts/mvc/workflow/workflow-view-node.js
index 2b16458a0ff..0686f57ff94 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-view-node.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-view-node.js
@@ -9,26 +9,31 @@ export class NodeView {
         this.$el = options.$el;
         this.node = options.node;
         this.output_width = Math.max(150, this.$el.width());
-        this.tool_body = this.$el.find(".node-body");
-        this.tool_body.find("div").remove();
-        this.newInputsDiv().appendTo(this.tool_body);
+        this.node_body = this.$el.find(".node-body");
+        this.node_body.find("div").remove();
+        this.newInputsDiv().appendTo(this.node_body);
         this.terminalViews = {};
         this.outputViews = {};
     }
 
     render() {
-        this.renderToolLabel();
-        this.renderToolErrors();
+        this.renderLabel();
+        this.renderErrors();
         this.$el.css("width", Math.min(250, Math.max(this.$el.width(), this.output_width)));
     }
 
-    renderToolLabel() {
+    renderLabel() {
         this.$el.find(".node-title").text(this.node.label || this.node.name);
         this.$el.attr("node-label", this.node.label);
     }
 
-    renderToolErrors() {
-        this.node.errors ? this.$el.addClass("node-error") : this.$el.removeClass("node-error");
+    renderErrors() {
+        if (this.node.errors) {
+            this.$el.addClass("node-error");
+            this.node_body.text(this.node.errors);
+        } else {
+            this.$el.removeClass("node-error");
+        }
     }
 
     newInputsDiv() {
@@ -40,7 +45,7 @@ export class NodeView {
     }
 
     addRule() {
-        this.tool_body.append($("<div/>").addClass("rule"));
+        this.node_body.append($("<div/>").addClass("rule"));
     }
 
     addDataInput(input, body) {
@@ -111,7 +116,7 @@ export class NodeView {
         const terminalView = this.terminalViewForOutput(output);
         const outputView = this.outputViewforOutput(output, terminalView);
         this.outputViews[output.name] = outputView;
-        this.tool_body.append(outputView.$el.append(terminalView.terminalElements()));
+        this.node_body.append(outputView.$el.append(terminalView.terminalElements()));
     }
 
     redrawWorkflowOutputs() {
diff --git a/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js b/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js
index 643564797f8..449ab170409 100644
--- a/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js
+++ b/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js
@@ -377,11 +377,11 @@ QUnit.test("destroy", function(assert) {
 });
 
 QUnit.test("error", function(assert) {
-    // Test body of toolFormBody div updated and workflow notified of change.
+    // Test body of div updated and workflow notified of change.
     var test = this;
     this.expect_workflow_node_changed(assert, function() {
-        test.node.error("TOOL ERROR");
-        assert.equal($(test.$(".node-body").children()[0]).html(), "TOOL ERROR");
+        test.node.init_field_data({ errors: "NODE ERROR", inputs: [], outputs: [] });
+        assert.equal(test.$(".node-body").html(), "NODE ERROR");
     });
 });
 

From c631d1f7061267ed9916dbeb952da53695f2da46 Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Thu, 13 Feb 2020 01:56:06 -0500
Subject: [PATCH 17/71] Move schema loading/validating code out of config

Add integration test; update several unit tests
---
 lib/galaxy/config/__init__.py          | 64 ++++---------------------
 lib/galaxy/config/schema.py            | 66 +++++++++++++++++++++++++-
 test/integration/test_config_schema.py |  9 ++++
 test/unit/config/test_load_config.py   | 50 +++++++++++++++----
 test/unit/config/test_reload_config.py |  6 ++-
 test/unit/config/test_schema.py        | 53 +++++++++++++++++----
 6 files changed, 175 insertions(+), 73 deletions(-)
 create mode 100644 test/integration/test_config_schema.py

diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py
index 3fdc7f9af3a..8bfcb691846 100644
--- a/lib/galaxy/config/__init__.py
+++ b/lib/galaxy/config/__init__.py
@@ -217,9 +217,11 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
     deprecated_dirs = {'config_dir': 'config', 'data_dir': 'database'}
 
     def __init__(self, **kwargs):
-        self._load_schema()  # Load schema from schema definition file
-        self._load_config_from_schema()  # Load default propery values from schema
-        self._validate_schema_paths()  # check that paths can be resolved
+        self.schema = self._load_schema()  # Load schema from schema definition file
+        self.appschema = self.schema.app_schema
+        self._raw_config = self.schema.defaults.copy()  # keeps track of startup values (kwargs or schema default)
+
+        self._load_schema()
         self._update_raw_config_from_kwargs(kwargs)  # Overwrite default values passed as kwargs
         self._create_attributes_from_raw_config()  # Create attributes for LOADED properties
 
@@ -227,57 +229,11 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
         self.root = find_root(kwargs)
         self._set_config_base(kwargs)  # must be called prior to _resolve_paths()
 
-        self._resolve_paths(kwargs)  # Overwrite attributes (not _raw_config) w/resolved paths
+        self._resolve_paths(self.schema.paths_to_resolve)  # Overwrite attributes (not _raw_config) w/resolved paths
         self._process_config(kwargs)  # Finish processing configuration
 
     def _load_schema(self):
-        self.schema = AppSchema(GALAXY_CONFIG_SCHEMA_PATH, GALAXY_APP_NAME)
-        self.appschema = self.schema.app_schema
-
-    def _load_config_from_schema(self):
-        self._raw_config = {}  # keeps track of startup values (kwargs or schema default)
-        self.reloadable_options = set()  # config options we can reload at runtime
-        self._paths_to_resolve = {}  # {config option: referenced config option}
-        for key, data in self.appschema.items():
-            self._raw_config[key] = data.get('default')
-            if data.get('reloadable'):
-                self.reloadable_options.add(key)
-            if data.get('path_resolves_to'):
-                self._paths_to_resolve[key] = data.get('path_resolves_to')
-
-    def _validate_schema_paths(self):
-
-        def check_exists(option, key):
-            if not option:
-                message = "Invalid schema: property '{}' listed as path resolution target " \
-                    "for '{}' does not exist".format(resolves_to, key)
-                raise_error(message)
-
-        def check_type_is_str(option, key):
-            if option.get('type') != 'str':
-                message = "Invalid schema: property '{}' should have type 'str'".format(key)
-                raise_error(message)
-
-        def check_is_dag():
-            visited = set()
-            for key in self._paths_to_resolve:
-                visited.clear()
-                while key:
-                    visited.add(key)
-                    key = self.appschema[key].get('path_resolves_to')
-                    if key and key in visited:
-                        raise_error('Invalid schema: cycle detected')
-
-        def raise_error(message):
-            log.error(message)
-            raise ConfigurationError(message)
-
-        for key, resolves_to in self._paths_to_resolve.items():
-            parent = self.appschema.get(resolves_to)
-            check_exists(parent, key)
-            check_type_is_str(parent, key)
-            check_type_is_str(self.appschema[key], key)
-        check_is_dag()  # must be called last: walks entire graph
+        return AppSchema(GALAXY_CONFIG_SCHEMA_PATH, GALAXY_APP_NAME)
 
     def _update_raw_config_from_kwargs(self, kwargs):
 
@@ -316,7 +272,7 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
                 raise ConfigurationError("Attempting to override existing attribute '%s'" % key)
             setattr(self, key, value)
 
-    def _resolve_paths(self, kwargs):
+    def _resolve_paths(self, paths_to_resolve):
 
         def resolve(key):
             if key in _cache:  # resolve each path only once
@@ -337,7 +293,7 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
             return path
 
         _cache = {}
-        for key in self._paths_to_resolve:
+        for key in paths_to_resolve:
             resolve(key)
 
     def _process_config(self, kwargs):
@@ -897,7 +853,7 @@ Configuration = GalaxyAppConfiguration
 def reload_config_options(current_config):
     """ Reload modified reloadable config options """
     modified_config = read_properties_from_file(current_config.config_file)
-    for option in current_config.reloadable_options:
+    for option in current_config.schema.reloadable_options:
         if option in modified_config:
             # compare to raw value, as that one is set only on load and reload
             if current_config._raw_config[option] != modified_config[option]:
diff --git a/lib/galaxy/config/schema.py b/lib/galaxy/config/schema.py
index 780f6763ac5..9ce91977558 100644
--- a/lib/galaxy/config/schema.py
+++ b/lib/galaxy/config/schema.py
@@ -1,8 +1,12 @@
+import logging
+
+from galaxy.exceptions import ConfigurationError
 from galaxy.util.yaml_util import (
     OPTION_DEFAULTS,
     ordered_load,
 )
 
+log = logging.getLogger(__name__)
 
 UNKNOWN_OPTION = {
     "type": "str",
@@ -33,8 +37,68 @@ class AppSchema(Schema):
         self.raw_schema = self._read_schema(schema_path)
         self.description = self.raw_schema.get("desc", None)
         app_schema = self.raw_schema['mapping'][app_name]['mapping']
-        super(AppSchema, self).__init__(app_schema)
+        self._preprocess(app_schema)
+        super().__init__(app_schema)
 
     def _read_schema(self, path):
         with open(path, "r") as f:
             return ordered_load(f)
+
+    def _preprocess(self, app_schema):
+        self._defaults = {}  # {config option: default value or null}
+        self._reloadable_options = set()  # config options we can reload at runtime
+        self._paths_to_resolve = {}  # {config option: referenced config option}
+        for key, data in app_schema.items():
+            self._defaults[key] = data.get('default')
+            if data.get('reloadable'):
+                self._reloadable_options.add(key)
+            if data.get('path_resolves_to'):
+                self._paths_to_resolve[key] = data.get('path_resolves_to')
+
+    @property
+    def defaults(self):
+        return self._defaults
+
+    @property
+    def paths_to_resolve(self):
+        return self._paths_to_resolve
+
+    @property
+    def reloadable_options(self):
+        return self._reloadable_options
+
+    def validate_path_resolution_graph(self):
+        # This method is for tests only: we SHOULD validate the schema's path resolution graph
+        # as part of automated testing; but we should NOT validate it at runtime.
+        def check_exists(option, key):
+            if not option:
+                message = "Invalid schema: property '{}' listed as path resolution target " \
+                    "for '{}' does not exist".format(resolves_to, key)
+                raise_error(message)
+
+        def check_type_is_str(option, key):
+            if option.get('type') != 'str':
+                message = "Invalid schema: property '{}' should have type 'str'".format(key)
+                raise_error(message)
+
+        def check_is_dag():
+            visited = set()
+            for key in self.paths_to_resolve:
+                visited.clear()
+                while key:
+                    visited.add(key)
+                    key = self.app_schema[key].get('path_resolves_to')
+                    if key and key in visited:
+                        raise_error('Invalid schema: cycle detected')
+
+        def raise_error(message):
+            log.error(message)
+            raise ConfigurationError(message)
+
+        for key, resolves_to in self.paths_to_resolve.items():
+            print(key)
+            parent = self.app_schema.get(resolves_to)
+            check_exists(parent, key)
+            check_type_is_str(parent, key)
+            check_type_is_str(self.app_schema[key], key)
+        check_is_dag()  # must be called last: walks entire graph
diff --git a/test/integration/test_config_schema.py b/test/integration/test_config_schema.py
new file mode 100644
index 00000000000..8766c91f660
--- /dev/null
+++ b/test/integration/test_config_schema.py
@@ -0,0 +1,9 @@
+from galaxy_test.driver import integration_util
+
+
+class ConfigSchemaTestCase(integration_util.IntegrationTestCase):
+
+    def test_schema_path_resolution_graph(self):
+        # Run schema's validation method; throws error if schema invalid
+        schema = self._app.config.schema
+        schema.validate_path_resolution_graph()
diff --git a/test/unit/config/test_load_config.py b/test/unit/config/test_load_config.py
index e6c9f36a370..86c14e57317 100644
--- a/test/unit/config/test_load_config.py
+++ b/test/unit/config/test_load_config.py
@@ -1,27 +1,59 @@
 import pytest
 
 from galaxy.config import GalaxyAppConfiguration
+from galaxy.config.schema import AppSchema
+from galaxy.util.yaml_util import (
+    ordered_load,
+    OrderedLoader,
+)
 
 
-MOCK_PROPERTIES = {
-    'property1': {'default': 'a', 'type': 'str'},  # str
-    'property2': {'default': 1, 'type': 'int'},  # int
-    'property3': {'default': 1.0, 'type': 'float'},  # float
-    'property4': {'default': True, 'type': 'bool'},  # bool
-    'property5': {'something_else': 'b', 'type': 'invalid'},
-    'property6': {'something_else': 'b'},  # no type
-}
+MOCK_YAML = '''
+    type: map
+    desc: mocked schema
+    mapping:
+      mockgalaxy:
+        type: map
+        mapping:
+          property1:
+            default: a
+            type: str
+          property2:
+            default: 1
+            type: int
+            reloadable: true
+          property3:
+            default: 1.0
+            type: float
+            reloadable: true
+          property4:
+            default: true
+            type: bool
+          property5:
+            something_else: b
+            type: invalid
+          property6:
+            something_else: b
+    '''
 
 
 @pytest.fixture
 def mock_init(monkeypatch):
 
+    def mock_read_schema(self, path):
+        return ordered_load(MOCK_YAML)
+
+    def mock_init(self, stream):
+        super(OrderedLoader, self).__init__(stream)
+
     def mock_load_schema(self):
-        self.appschema = MOCK_PROPERTIES
+        return AppSchema('no path', 'mockgalaxy')
 
     def mock_process_config(self, kwargs):
         pass
 
+    monkeypatch.setattr(AppSchema, '_read_schema', mock_read_schema)
+    monkeypatch.setattr(OrderedLoader, '__init__', mock_init)
     monkeypatch.setattr(GalaxyAppConfiguration, '_load_schema', mock_load_schema)
     monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', mock_process_config)
 
diff --git a/test/unit/config/test_reload_config.py b/test/unit/config/test_reload_config.py
index e63cdcbce0a..2bb5f72b74f 100644
--- a/test/unit/config/test_reload_config.py
+++ b/test/unit/config/test_reload_config.py
@@ -6,9 +6,13 @@ R1, R2, N1, N2 = 'reloadable1', 'reloadable2', 'nonrelodable1', 'nonreloadable2'
 
 
 class MockGalaxyAppConfiguration():
+
+    class MockSchema():
+        reloadable_options = {R1, R2}
+
     def __init__(self, properties):
         self.config_file = None
-        self.reloadable_options = {R1, R2}
+        self.schema = self.MockSchema
         self._raw_config = {}
         for key, value in properties.items():
             setattr(self, key, value)
diff --git a/test/unit/config/test_schema.py b/test/unit/config/test_schema.py
index 6569627f2e2..c727d0e4a00 100644
--- a/test/unit/config/test_schema.py
+++ b/test/unit/config/test_schema.py
@@ -12,9 +12,27 @@ MOCK_YAML = '''
       mockgalaxy:
         type: map
         mapping:
-          option:
-            attr1: a
-            attr2: b
+          property1:
+            default: a
+            type: str
+            path_resolves_to: foo
+          property2:
+            default: 1
+            type: int
+            reloadable: true
+          property3:
+            default: 1.0
+            type: float
+            reloadable: true
+          property4:
+            default: true
+            type: bool
+            path_resolves_to: foo
+          property5:
+            something_else: b
+            type: invalid
+          property6:
+            something_else: b
     '''
 
 
@@ -29,10 +47,29 @@ def test_schema_is_loaded(monkeypatch):
     monkeypatch.setattr(AppSchema, '_read_schema', mock_read_schema)
     monkeypatch.setattr(OrderedLoader, '__init__', mock_init)
 
-    loaded_schema = AppSchema('no path', 'mockgalaxy')
+    schema = AppSchema('no path', 'mockgalaxy')
     data = ordered_load(MOCK_YAML)
 
-    assert loaded_schema.description == data['desc']
-    assert loaded_schema.raw_schema['foo'] == 'bar'
-    assert loaded_schema.app_schema['option']['attr1'] == 'a'
-    assert loaded_schema.get_app_option('option')['attr2'] == 'b'
+    assert schema.description == data['desc']
+    assert schema.raw_schema['foo'] == 'bar'
+
+    assert len(schema.defaults) == 6
+    assert schema.defaults['property1'] == 'a'
+    assert schema.defaults['property2'] == 1
+    assert schema.defaults['property3'] == 1.0
+    assert schema.defaults['property4'] is True
+    assert schema.defaults['property5'] is None
+    assert schema.defaults['property6'] is None
+
+    assert type(schema.defaults['property1']) is str
+    assert type(schema.defaults['property2']) is int
+    assert type(schema.defaults['property3']) is float
+    assert type(schema.defaults['property4']) is bool
+
+    assert len(schema.reloadable_options) == 2
+    assert 'property2' in schema.reloadable_options
+    assert 'property3' in schema.reloadable_options
+
+    assert len(schema.paths_to_resolve) == 2
+    assert 'property1' in schema.paths_to_resolve
+    assert 'property4' in schema.paths_to_resolve

From f292f4dd80b9db59a2c2dce1bd971848d7889247 Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Fri, 14 Feb 2020 02:03:54 -0500
Subject: [PATCH 18/71] Make schema module python2-compatible; cleanup comments

---
 lib/galaxy/config/schema.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/galaxy/config/schema.py b/lib/galaxy/config/schema.py
index 9ce91977558..34491906e26 100644
--- a/lib/galaxy/config/schema.py
+++ b/lib/galaxy/config/schema.py
@@ -38,13 +38,14 @@ class AppSchema(Schema):
         self.description = self.raw_schema.get("desc", None)
         app_schema = self.raw_schema['mapping'][app_name]['mapping']
         self._preprocess(app_schema)
-        super().__init__(app_schema)
+        super(AppSchema, self).__init__(app_schema)
 
     def _read_schema(self, path):
         with open(path, "r") as f:
             return ordered_load(f)
 
     def _preprocess(self, app_schema):
+        """Populate schema collections used for app configuration."""
         self._defaults = {}  # {config option: default value or null}
         self._reloadable_options = set()  # config options we can reload at runtime
         self._paths_to_resolve = {}  # {config option: referenced config option}
@@ -68,8 +69,9 @@ class AppSchema(Schema):
         return self._reloadable_options
 
     def validate_path_resolution_graph(self):
-        # This method is for tests only: we SHOULD validate the schema's path resolution graph
-        # as part of automated testing; but we should NOT validate it at runtime.
+        """This method is for tests only: we SHOULD validate the schema's path resolution graph
+           as part of automated testing; but we should NOT validate it at runtime.
+        """
         def check_exists(option, key):
             if not option:
                 message = "Invalid schema: property '{}' listed as path resolution target " \

From 70f227da246502c2467418f9bfc8fd56635e196f Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Fri, 14 Feb 2020 02:08:38 -0500
Subject: [PATCH 19/71] Refactor config unit tests

1. Modify test setup code to adjust to edits in previous commit. Do NOT
   change the tests.

2. Simplify tests; get rid of mocking where possible (use real objects).
---
 test/unit/config/test_load_config.py      | 62 +++++----------------
 test/unit/config/test_path_graph.py       | 66 ++++++++---------------
 test/unit/config/test_path_resolves_to.py | 30 ++++-------
 test/unit/config/test_reload_config.py    | 64 +++++++++++-----------
 4 files changed, 78 insertions(+), 144 deletions(-)

diff --git a/test/unit/config/test_load_config.py b/test/unit/config/test_load_config.py
index 86c14e57317..f7deb4a49cf 100644
--- a/test/unit/config/test_load_config.py
+++ b/test/unit/config/test_load_config.py
@@ -2,60 +2,26 @@ import pytest
 
 from galaxy.config import GalaxyAppConfiguration
 from galaxy.config.schema import AppSchema
-from galaxy.util.yaml_util import (
-    ordered_load,
-    OrderedLoader,
-)
 
 
-MOCK_YAML = '''
-    type: map
-    desc: mocked schema
-    mapping:
-      mockgalaxy:
-        type: map
-        mapping:
-          property1:
-            default: a
-            type: str
-          property2:
-            default: 1
-            type: int
-            reloadable: true
-          property3:
-            default: 1.0
-            type: float
-            reloadable: true
-          property4:
-            default: true
-            type: bool
-          property5:
-            something_else: b
-            type: invalid
-          property6:
-            something_else: b
-    '''
+MOCK_SCHEMA = {
+    'property1': {'default': 'a', 'type': 'str'},  # str
+    'property2': {'default': 1, 'type': 'int'},  # int
+    'property3': {'default': 1.0, 'type': 'float'},  # float
+    'property4': {'default': True, 'type': 'bool'},  # bool
+    'property5': {'something_else': 'b', 'type': 'invalid'},
+    'property6': {'something_else': 'b'},  # no type
+}
+
+
+def get_schema(app_mapping):
+    return {'mapping': {'galaxy': {'mapping': app_mapping}}}
 
 
 @pytest.fixture
 def mock_init(monkeypatch):
-
-    def mock_read_schema(self, path):
-        return ordered_load(MOCK_YAML)
-
-    def mock_init(self, stream):
-        super(OrderedLoader, self).__init__(stream)
-
-    def mock_load_schema(self):
-        return AppSchema('no path', 'mockgalaxy')
-
-    def mock_process_config(self, kwargs):
-        pass
-
-    monkeypatch.setattr(AppSchema, '_read_schema', mock_read_schema)
-    monkeypatch.setattr(OrderedLoader, '__init__', mock_init)
-    monkeypatch.setattr(GalaxyAppConfiguration, '_load_schema', mock_load_schema)
-    monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', mock_process_config)
+    monkeypatch.setattr(AppSchema, '_read_schema', lambda a, b: get_schema(MOCK_SCHEMA))
+    monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', lambda a, b: None)
 
 
 def test_load_config_from_schema(mock_init):
diff --git a/test/unit/config/test_path_graph.py b/test/unit/config/test_path_graph.py
index 7d3a3626653..73c719594ea 100644
--- a/test/unit/config/test_path_graph.py
+++ b/test/unit/config/test_path_graph.py
@@ -1,6 +1,7 @@
 import pytest
 
 from galaxy.config import GalaxyAppConfiguration
+from galaxy.config.schema import AppSchema
 from galaxy.exceptions import ConfigurationError
 
 
@@ -15,14 +16,11 @@ from galaxy.exceptions import ConfigurationError
 # whereas an invalid configuration raises an error.
 
 
-@pytest.fixture
-def mock_config(monkeypatch):
-    def mock_process_config(self, kwargs):
-        pass
-    monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', mock_process_config)
+def get_schema(app_mapping):
+    return {'mapping': {'galaxy': {'mapping': app_mapping}}}
 
 
-def test_basecase(mock_config, monkeypatch):
+def test_basecase(monkeypatch):
     # Check that a valid graph is loaded correctly (this graph has 2 components)
     mock_schema = {
         'component1_path0': {
@@ -48,12 +46,9 @@ def test_basecase(mock_config, monkeypatch):
             'default': 'value4',
             'path_resolves_to': 'component2_path0',
         },
-
     }
-
-    def mock_load_schema(self):
-        self.appschema = mock_schema
-    monkeypatch.setattr(GalaxyAppConfiguration, '_load_schema', mock_load_schema)
+    monkeypatch.setattr(AppSchema, '_read_schema', lambda a, b: get_schema(mock_schema))
+    monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', lambda a, b: None)
 
     config = GalaxyAppConfiguration()
     assert config.component1_path0 == 'value0'
@@ -63,7 +58,7 @@ def test_basecase(mock_config, monkeypatch):
     assert config.component2_path1 == 'value3/value4'
 
 
-def test_resolves_to_invalid_property(mock_config, monkeypatch):
+def test_resolves_to_invalid_property(monkeypatch):
     # 'path_resolves_to' should point to an existing property in the schema
     mock_schema = {
         'path0': {
@@ -76,17 +71,13 @@ def test_resolves_to_invalid_property(mock_config, monkeypatch):
             'path_resolves_to': 'invalid',  # invalid
         },
     }
-
-    def mock_load_schema(self):
-        self.appschema = mock_schema
-
-    monkeypatch.setattr(GalaxyAppConfiguration, '_load_schema', mock_load_schema)
+    monkeypatch.setattr(AppSchema, '_read_schema', lambda a, b: get_schema(mock_schema))
 
     with pytest.raises(ConfigurationError):
-        GalaxyAppConfiguration()
+        AppSchema(None, 'galaxy').validate_path_resolution_graph()
 
 
-def test_path_resolution_cycle(mock_config, monkeypatch):
+def test_path_resolution_cycle(monkeypatch):
     # Must be a DAG, but this one has a cycle
     mock_schema = {
         'path0': {
@@ -105,17 +96,13 @@ def test_path_resolution_cycle(mock_config, monkeypatch):
             'path_resolves_to': 'path1',
         },
     }
-
-    def mock_load_schema(self):
-        self.appschema = mock_schema
-
-    monkeypatch.setattr(GalaxyAppConfiguration, '_load_schema', mock_load_schema)
+    monkeypatch.setattr(AppSchema, '_read_schema', lambda a, b: get_schema(mock_schema))
 
     with pytest.raises(ConfigurationError):
-        GalaxyAppConfiguration()
+        AppSchema(None, 'galaxy').validate_path_resolution_graph()
 
 
-def test_path_invalid_type(mock_config, monkeypatch):
+def test_path_invalid_type(monkeypatch):
     # Paths should be strings
     mock_schema = {
         'path0': {
@@ -128,17 +115,13 @@ def test_path_invalid_type(mock_config, monkeypatch):
             'path_resolves_to': 'path0',
         },
     }
-
-    def mock_load_schema(self):
-        self.appschema = mock_schema
-
-    monkeypatch.setattr(GalaxyAppConfiguration, '_load_schema', mock_load_schema)
+    monkeypatch.setattr(AppSchema, '_read_schema', lambda a, b: get_schema(mock_schema))
 
     with pytest.raises(ConfigurationError):
-        GalaxyAppConfiguration()
+        AppSchema(None, 'galaxy').validate_path_resolution_graph()
 
 
-def test_resolves_to_invalid_type(mock_config, monkeypatch):
+def test_resolves_to_invalid_type(monkeypatch):
     # Paths should be strings
     mock_schema = {
         'path0': {
@@ -151,17 +134,13 @@ def test_resolves_to_invalid_type(mock_config, monkeypatch):
             'path_resolves_to': 'path0',
         },
     }
-
-    def mock_load_schema(self):
-        self.appschema = mock_schema
-
-    monkeypatch.setattr(GalaxyAppConfiguration, '_load_schema', mock_load_schema)
+    monkeypatch.setattr(AppSchema, '_read_schema', lambda a, b: get_schema(mock_schema))
 
     with pytest.raises(ConfigurationError):
-        GalaxyAppConfiguration()
+        AppSchema(None, 'galaxy').validate_path_resolution_graph()
 
 
-def test_resolves_with_empty_component(mock_config, monkeypatch):
+def test_resolves_with_empty_component(monkeypatch):
     # A path can be None (root path is never None; may be asigned elsewhere)
     mock_schema = {
         'path0': {
@@ -178,11 +157,8 @@ def test_resolves_with_empty_component(mock_config, monkeypatch):
             'path_resolves_to': 'path1',
         },
     }
-
-    def mock_load_schema(self):
-        self.appschema = mock_schema
-
-    monkeypatch.setattr(GalaxyAppConfiguration, '_load_schema', mock_load_schema)
+    monkeypatch.setattr(AppSchema, '_read_schema', lambda a, b: get_schema(mock_schema))
+    monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', lambda a, b: None)
 
     config = GalaxyAppConfiguration()
     assert config.path0 == 'value0'
diff --git a/test/unit/config/test_path_resolves_to.py b/test/unit/config/test_path_resolves_to.py
index 83b5c3198d1..439d2c6c001 100644
--- a/test/unit/config/test_path_resolves_to.py
+++ b/test/unit/config/test_path_resolves_to.py
@@ -1,6 +1,7 @@
 import pytest
 
 from galaxy.config import GalaxyAppConfiguration
+from galaxy.config.schema import AppSchema
 
 
 MOCK_DEPRECATED_DIRS = {
@@ -35,34 +36,23 @@ MOCK_SCHEMA = {
 }
 
 
+def get_schema(app_mapping):
+    return {'mapping': {'galaxy': {'mapping': app_mapping}}}
+
+
 def test_deprecated_prefixes_set_correctly(monkeypatch):
     # Before we mock them, check that correct values are assigned
-    def mock_load_schema(self):
-        self.appschema = {}
-
-    def mock_process_config(self, kwargs):
-        pass
-
-    monkeypatch.setattr(GalaxyAppConfiguration, '_load_schema', mock_load_schema)
-    monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', mock_process_config)
+    monkeypatch.setattr(AppSchema, '_read_schema', lambda a, b: get_schema(MOCK_SCHEMA))
+    monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', lambda a, b: None)
 
     config = GalaxyAppConfiguration()
-    expected_deprecated_dirs = {'config_dir': 'config', 'data_dir': 'database'}
-
-    assert config.deprecated_dirs == expected_deprecated_dirs
+    assert config.deprecated_dirs == {'config_dir': 'config', 'data_dir': 'database'}
 
 
 @pytest.fixture
 def mock_init(monkeypatch):
-
-    def mock_load_schema(self):
-        self.appschema = MOCK_SCHEMA
-
-    def mock_process_config(self, kwargs):
-        pass  # because we're done before this is called
-
-    monkeypatch.setattr(GalaxyAppConfiguration, '_load_schema', mock_load_schema)
-    monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', mock_process_config)
+    monkeypatch.setattr(AppSchema, '_read_schema', lambda a, b: get_schema(MOCK_SCHEMA))
+    monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', lambda a, b: None)
     monkeypatch.setattr(GalaxyAppConfiguration, 'deprecated_dirs', MOCK_DEPRECATED_DIRS)
 
 
diff --git a/test/unit/config/test_reload_config.py b/test/unit/config/test_reload_config.py
index 2bb5f72b74f..3064b0475ad 100644
--- a/test/unit/config/test_reload_config.py
+++ b/test/unit/config/test_reload_config.py
@@ -1,37 +1,41 @@
+import pytest
+
 from galaxy import config
+from galaxy.config import GalaxyAppConfiguration
 from galaxy.config import reload_config_options
+from galaxy.config.schema import AppSchema
 
 
 R1, R2, N1, N2 = 'reloadable1', 'reloadable2', 'nonrelodable1', 'nonreloadable2'  # config options
 
-
-class MockGalaxyAppConfiguration():
-
-    class MockSchema():
-        reloadable_options = {R1, R2}
-
-    def __init__(self, properties):
-        self.config_file = None
-        self.schema = self.MockSchema
-        self._raw_config = {}
-        for key, value in properties.items():
-            setattr(self, key, value)
-            self._raw_config[key] = value
-
-    def update_reloadable_property(self, key, value):
-        setattr(self, key, value)
+MOCK_SCHEMA = {
+    R1: {'reloadable': True, 'default': 1},
+    R2: {'reloadable': True, 'default': 2},
+    N1: {'default': 3},
+    N2: {'default': 4},
+}
 
 
-def test_update_property(monkeypatch):
+def get_schema(app_mapping):
+    return {'mapping': {'galaxy': {'mapping': app_mapping}}}
+
+
+@pytest.fixture
+def mock_init(monkeypatch):
+
+    monkeypatch.setattr(AppSchema, '_read_schema', lambda a, b: get_schema(MOCK_SCHEMA))
+    monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', lambda a, b: None)
+
+
+def test_update_property(mock_init, monkeypatch):
     # This also covers adding a property. When a config file does not set a property,
     # that property is set to its default value. Thus, if we add a reloadable property
     # to the config file, it's the same as modifying that property's value.
-    appconfig = MockGalaxyAppConfiguration({R1: 1, R2: 2, N1: 3})
 
-    def mock_read_properties_from_file(values):
-        return {R1: 1, R2: 42, N1: 99}  # edits: R2, N1 modified
+    # edits to config file: R2, N1 modified
+    monkeypatch.setattr(config, 'read_properties_from_file', lambda _: {R1: 1, R2: 42, N1: 99})
 
-    monkeypatch.setattr(config, 'read_properties_from_file', mock_read_properties_from_file)
+    appconfig = GalaxyAppConfiguration()
 
     assert getattr(appconfig, R1) == 1
     assert getattr(appconfig, R2) == 2
@@ -44,7 +48,7 @@ def test_update_property(monkeypatch):
     assert getattr(appconfig, N1) == 3  # no change: option modified but is non-relodable
 
 
-def test_overwrite_reloadable_attribute(monkeypatch):
+def test_overwrite_reloadable_attribute(mock_init, monkeypatch):
     # This is similar to test_update_property, but here we overwrite the attribute before reloading.
     # This can happen if a config property is modified AFTER it has been loaded from schema or kwargs.
     # For example: load `foo` (from schema or kwargs), but then, in a # subsequent step while initializing
@@ -52,12 +56,11 @@ def test_overwrite_reloadable_attribute(monkeypatch):
     # is not what was initially loaded, and if `foo` is reloadable, it will be reset to its default as soon
     # as the config file is modified. To prevent this, we compare the values read from the modified file
     # to the `_raw_config` dict. This test ensures this works correctly.
-    appconfig = MockGalaxyAppConfiguration({R1: 1, R2: 2, N1: 3})
 
-    def mock_read_properties_from_file(values):
-        return {R1: 1, R2: 42}  # edits: R2 modified
+    # edits to config file: R2 modified
+    monkeypatch.setattr(config, 'read_properties_from_file', lambda _: {R1: 1, R2: 42})
 
-    monkeypatch.setattr(config, 'read_properties_from_file', mock_read_properties_from_file)
+    appconfig = GalaxyAppConfiguration()
 
     assert getattr(appconfig, R1) == 1
     assert getattr(appconfig, R2) == 2
@@ -71,16 +74,15 @@ def test_overwrite_reloadable_attribute(monkeypatch):
     assert getattr(appconfig, R2) == 42  # change: reloadable option modified
 
 
-def test_cant_delete_property(monkeypatch):
+def test_cant_delete_property(mock_init, monkeypatch):
     # A property should not be deleted: we don't know whether it was initially
     # set to a default, loaded from a config file, env var, etc. Therefore, if a property
     # is removed from the config file, it will not be modified or deleted.
-    appconfig = MockGalaxyAppConfiguration({R1: 1, R2: 2, N1: 3, N2: 4})
 
-    def mock_read_properties_from_file(values):
-        return {R1: 1, N1: 3}  # edits: R2, N2 deleted
+    # edits to config file: R2, N2 deleted
+    monkeypatch.setattr(config, 'read_properties_from_file', lambda _: {R1: 1, N1: 3})
 
-    monkeypatch.setattr(config, 'read_properties_from_file', mock_read_properties_from_file)
+    appconfig = GalaxyAppConfiguration()
 
     assert getattr(appconfig, R1) == 1
     assert getattr(appconfig, R2) == 2

From 8d40b7441a2ff0bda5375ee2bd4c74fbcb8e70a4 Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Fri, 14 Feb 2020 20:27:59 -0500
Subject: [PATCH 20/71] Move OPTION_DEFAULTS to Schema

Rationale: group related items together. Constants OPTION_DEFAULTS and
UNKNOWN_OPTION are closely related and are used in the same context.

OPTION_DEFAULTS is only used in `schema.py` and `config_manage.py`; the
latter imports items from `schema.py`. Thus, it is reasonable to place
both constants in the same module.

(cherry-picked from closed #9307)
---
 lib/galaxy/config/config_manage.py |  2 +-
 lib/galaxy/config/schema.py        | 12 ++++++++----
 lib/galaxy/util/yaml_util.py       |  7 -------
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/lib/galaxy/config/config_manage.py b/lib/galaxy/config/config_manage.py
index 8ae4035bc63..9588d2d9c15 100644
--- a/lib/galaxy/config/config_manage.py
+++ b/lib/galaxy/config/config_manage.py
@@ -30,12 +30,12 @@ if __name__ == '__main__':
 from galaxy.config import GALAXY_CONFIG_SCHEMA_PATH
 from galaxy.config.schema import (
     AppSchema,
+    OPTION_DEFAULTS,
     Schema,
 )
 from galaxy.util import safe_makedirs
 from galaxy.util.properties import nice_config_parser
 from galaxy.util.yaml_util import (
-    OPTION_DEFAULTS,
     ordered_dump,
     ordered_load,
 )
diff --git a/lib/galaxy/config/schema.py b/lib/galaxy/config/schema.py
index 34491906e26..66e3905881f 100644
--- a/lib/galaxy/config/schema.py
+++ b/lib/galaxy/config/schema.py
@@ -1,13 +1,17 @@
 import logging
 
 from galaxy.exceptions import ConfigurationError
-from galaxy.util.yaml_util import (
-    OPTION_DEFAULTS,
-    ordered_load,
-)
+from galaxy.util.yaml_util import ordered_load
 
 log = logging.getLogger(__name__)
 
+OPTION_DEFAULTS = {
+    "type": "str",
+    "unknown_option": False,
+    "default": None,
+    "desc": None,
+}
+
 UNKNOWN_OPTION = {
     "type": "str",
     "required": False,
diff --git a/lib/galaxy/util/yaml_util.py b/lib/galaxy/util/yaml_util.py
index 70f4683359d..bb466a96981 100644
--- a/lib/galaxy/util/yaml_util.py
+++ b/lib/galaxy/util/yaml_util.py
@@ -10,13 +10,6 @@ from yaml.constructor import ConstructorError
 
 log = logging.getLogger(__name__)
 
-OPTION_DEFAULTS = {
-    "type": "str",
-    "unknown_option": False,
-    "default": None,
-    "desc": None,
-}
-
 
 class OrderedLoader(yaml.SafeLoader):
     # This class was pulled out of ordered_load() for the sake of

From 230e292db56311104bf23aa2dfb4dd40055d22d1 Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Fri, 14 Feb 2020 21:51:51 -0500
Subject: [PATCH 21/71] Log message instead of raising an exception

This commit drops the exception that guarded against accidental
overwriting of existing attributes, and instead ignores the setattr
call, logging a debug message (which was initialy proposed in #8728).

Rationale: with the previous version, base configuration properties
(config_dir, data_dir, etc.) had to be set *after* this method was
called (otherwise an exeption was raised), but *before* any path
resolving code (because these properties are used for path
resolution). As a result, config loading required a rigid sequence of steps:
1: load defaults and kwargs, create all config attributes.
2: set base configuration properties [CANNOT MOVE THIS]
3: resolve paths, do the rest: load individual properties, etc.

However, setting base configuration properties is a special case that is
*independent of schema loading*, is handled separately (in
set_config_base), and takes precendence over schema loading procedures.

This modification makes config loading more flexible, and logical:
1. set base configuration properties
2. do the rest (now any refactoring can go here)

(cherry-picked from closed #9307)
---
 lib/galaxy/config/__init__.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py
index 8bfcb691846..344b9a2a395 100644
--- a/lib/galaxy/config/__init__.py
+++ b/lib/galaxy/config/__init__.py
@@ -268,9 +268,10 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
 
     def _create_attributes_from_raw_config(self):
         for key, value in self._raw_config.items():
-            if hasattr(self, key):
-                raise ConfigurationError("Attempting to override existing attribute '%s'" % key)
-            setattr(self, key, value)
+            if not hasattr(self, key):
+                setattr(self, key, value)
+            else:
+                log.debug("Attribute '%s' is set and cannot be overwritten with value '%s'" % (key, value))
 
     def _resolve_paths(self, paths_to_resolve):
 

From 5c07b65f8b961f9d730f939817d03d14fb414812 Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Fri, 14 Feb 2020 22:33:17 -0500
Subject: [PATCH 22/71] Remove appconfig instance var; simplify code

GalaxyAppConfiguration has a reference to an AppSchema object (`schema`
instance variable), which provides access to relevant schema data. So we
can simplify:
`self.appschema[key].get('path_resolves_to')` >> `self.schema.paths_to_resolve.get(key)`

Accessing the application schema mapping directly is straightforward:
`self.schema.app_schema`
---
 lib/galaxy/config/__init__.py            | 11 +++++------
 lib/galaxy/tools/toolbox/base.py         |  2 +-
 test/integration/test_config_defaults.py |  2 +-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py
index 344b9a2a395..c98dfe9800f 100644
--- a/lib/galaxy/config/__init__.py
+++ b/lib/galaxy/config/__init__.py
@@ -218,7 +218,6 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
 
     def __init__(self, **kwargs):
         self.schema = self._load_schema()  # Load schema from schema definition file
-        self.appschema = self.schema.app_schema
         self._raw_config = self.schema.defaults.copy()  # keeps track of startup values (kwargs or schema default)
 
         self._load_schema()
@@ -238,14 +237,14 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
     def _update_raw_config_from_kwargs(self, kwargs):
 
         def convert_datatype(key, value):
-            datatype = self.appschema[key].get('type')
+            datatype = self.schema.app_schema[key].get('type')
             # check for `not None` explicitly (value can be falsy)
             if value is not None and datatype in type_converters:
                 return type_converters[datatype](value)
             return value
 
         def strip_deprecated_dir(key, value):
-            resolves_to = self.appschema[key].get('path_resolves_to')
+            resolves_to = self.schema.paths_to_resolve.get(key)
             if resolves_to:  # value is a path that will be resolved
                 first_dir = value.split(os.sep)[0]  # get first directory component
                 if first_dir == self.deprecated_dirs[resolves_to]:  # first_dir is deprecated for this option
@@ -260,7 +259,7 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
         type_converters = {'bool': string_as_bool, 'int': int, 'float': float, 'str': str}
 
         for key, value in kwargs.items():
-            if key in self.appschema:
+            if key in self.schema.app_schema:
                 value = convert_datatype(key, value)
                 if value:
                     value = strip_deprecated_dir(key, value)
@@ -280,7 +279,7 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
                 return _cache[key]
 
             path = getattr(self, key)  # path prior to being resolved
-            parent = self.appschema[key].get('path_resolves_to')
+            parent = paths_to_resolve.get(key)
             if not parent:  # base case: nothing else needs resolving
                 return path
             parent_path = resolve(parent)  # recursively resolve parent path
@@ -483,7 +482,7 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
         if self.tool_dependency_dir and self.tool_dependency_dir.lower() == 'none':
             self.tool_dependency_dir = None
         if self.involucro_path is None:
-            target_dir = self.tool_dependency_dir or self.appschema['tool_dependency_dir'].get('default')
+            target_dir = self.tool_dependency_dir or self.schema.defaults['tool_dependency_dir']
             self.involucro_path = os.path.join(self.data_dir, target_dir, "involucro")
         self.involucro_path = os.path.join(self.root, self.involucro_path)
         if self.mulled_channels:
diff --git a/lib/galaxy/tools/toolbox/base.py b/lib/galaxy/tools/toolbox/base.py
index 129dd3909b2..03c1cc96872 100644
--- a/lib/galaxy/tools/toolbox/base.py
+++ b/lib/galaxy/tools/toolbox/base.py
@@ -1217,7 +1217,7 @@ class BaseGalaxyToolBox(AbstractToolBox):
             return
         app_config_dict = self.app.config.config_dict
         conf_file = app_config_dict.get("dependency_resolvers_config_file")
-        default_tool_dependency_dir = os.path.join(self.app.config.data_dir, self.app.config.appschema['tool_dependency_dir'].get('default'))
+        default_tool_dependency_dir = os.path.join(self.app.config.data_dir, self.app.config.schema.defaults['tool_dependency_dir'])
         self.dependency_manager = build_dependency_manager(app_config_dict=app_config_dict, conf_file=conf_file,
                                                            default_tool_dependency_dir=default_tool_dependency_dir)
 
diff --git a/test/integration/test_config_defaults.py b/test/integration/test_config_defaults.py
index c8d2ff5f26e..1087fe5b4f0 100644
--- a/test/integration/test_config_defaults.py
+++ b/test/integration/test_config_defaults.py
@@ -253,7 +253,7 @@ def get_config_data():
 
     create_driver()  # create + setup DRIVER
     parent_dirs = load_parent_dirs()  # called after DRIVER is setup
-    items = ((k, v) for k, v in DRIVER.app.config.appschema.items() if k not in DO_NOT_TEST)
+    items = ((k, v) for k, v in DRIVER.app.config.schema.app_schema.items() if k not in DO_NOT_TEST)
     for key, data in items:
         expected_value = get_expected(key, data, parent_dirs)
         loaded_value = getattr(DRIVER.app.config, key)

From 6f53e68e18c2d2955e256f40a6e4cbfef9a57341 Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Sat, 15 Feb 2020 21:49:50 -0500
Subject: [PATCH 23/71] Fix mock + related code

Add schema var to galaxy mock as a MockSchema stub object
---
 lib/galaxy/tools/toolbox/base.py        | 7 ++++---
 test/unit/tools/test_toolbox.py         | 2 +-
 test/unit/unittest_utils/galaxy_mock.py | 4 ++++
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/lib/galaxy/tools/toolbox/base.py b/lib/galaxy/tools/toolbox/base.py
index 03c1cc96872..08a4697564a 100644
--- a/lib/galaxy/tools/toolbox/base.py
+++ b/lib/galaxy/tools/toolbox/base.py
@@ -1217,9 +1217,10 @@ class BaseGalaxyToolBox(AbstractToolBox):
             return
         app_config_dict = self.app.config.config_dict
         conf_file = app_config_dict.get("dependency_resolvers_config_file")
-        default_tool_dependency_dir = os.path.join(self.app.config.data_dir, self.app.config.schema.defaults['tool_dependency_dir'])
-        self.dependency_manager = build_dependency_manager(app_config_dict=app_config_dict, conf_file=conf_file,
-                                                           default_tool_dependency_dir=default_tool_dependency_dir)
+        default_tool_dependency_dir = os.path.join(
+            self.app.config.data_dir, self.app.config.schema.app_schema['tool_dependency_dir'].get('default'))
+        self.dependency_manager = build_dependency_manager(
+            app_config_dict=app_config_dict, conf_file=conf_file, default_tool_dependency_dir=default_tool_dependency_dir)
 
     def reload_dependency_manager(self):
         self._init_dependency_manager()
diff --git a/test/unit/tools/test_toolbox.py b/test/unit/tools/test_toolbox.py
index c5695c4ef78..d2bfc6caaab 100644
--- a/test/unit/tools/test_toolbox.py
+++ b/test/unit/tools/test_toolbox.py
@@ -564,7 +564,7 @@ class SimplifiedToolBox(ToolBox):
         app.tool_cache = ToolCache() if not hasattr(app, 'tool_cache') else app.tool_cache
         app.job_config.get_tool_resource_parameters = lambda tool_id: None
         app.config.update_integrated_tool_panel = True
-        app.config.appschema = {
+        app.config.schema.app_schema = {
             'tool_dependency_dir': {
                 'default': 'dependencies',
             }
diff --git a/test/unit/unittest_utils/galaxy_mock.py b/test/unit/unittest_utils/galaxy_mock.py
index 5b9d06978fc..3cde0eaa3e3 100644
--- a/test/unit/unittest_utils/galaxy_mock.py
+++ b/test/unit/unittest_utils/galaxy_mock.py
@@ -111,6 +111,9 @@ class MockLock(object):
 
 class MockAppConfig(Bunch):
 
+    class MockSchema(Bunch):
+        pass
+
     def __init__(self, root=None, **kwargs):
         Bunch.__init__(self, **kwargs)
         if not root:
@@ -118,6 +121,7 @@ class MockAppConfig(Bunch):
             self._remove_root = True
         else:
             self._remove_root = False
+        self.schema = self.MockSchema()
         self.security = idencoding.IdEncodingHelper(id_secret='6e46ed6483a833c100e68cc3f1d0dd76')
         self.database_connection = kwargs.get('database_connection', "sqlite:///:memory:")
         self.use_remote_user = kwargs.get('use_remote_user', False)

From 9a3400fa0c5129bfd62cb4d303097dfe7d6c16e8 Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Sat, 15 Feb 2020 23:40:35 -0500
Subject: [PATCH 24/71] Move a lot of code from config into base class

Provide stub methods for TS config implementation as a temporary
measure. No refactoring of TS config in this commit.
---
 lib/galaxy/config/__init__.py  | 154 +++++++++++++++++----------------
 lib/tool_shed/webapp/config.py |  16 ++++
 2 files changed, 95 insertions(+), 75 deletions(-)

diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py
index c98dfe9800f..0cc90edb6d9 100644
--- a/lib/galaxy/config/__init__.py
+++ b/lib/galaxy/config/__init__.py
@@ -105,6 +105,83 @@ def find_root(kwargs):
 
 
 class BaseAppConfiguration(object):
+
+    def __init__(self, **kwargs):
+        self.schema = self._load_schema()  # Load schema from schema definition file
+        self._raw_config = self.schema.defaults.copy()  # keeps track of startup values (kwargs or schema default)
+        self._load_schema()
+        self._update_raw_config_from_kwargs(kwargs)  # Overwrite default values passed as kwargs
+        self._create_attributes_from_raw_config()  # Create attributes for LOADED properties
+        self.config_dict = kwargs
+        self.root = find_root(kwargs)
+        self._set_config_base(kwargs)  # must be called prior to _resolve_paths()
+        self._resolve_paths(self.schema.paths_to_resolve)  # Overwrite attributes (not _raw_config) w/resolved paths
+
+    def _load_schema(self):
+        raise Exception('Not implemented')
+
+    def _update_raw_config_from_kwargs(self, kwargs):
+
+        def convert_datatype(key, value):
+            datatype = self.schema.app_schema[key].get('type')
+            # check for `not None` explicitly (value can be falsy)
+            if value is not None and datatype in type_converters:
+                return type_converters[datatype](value)
+            return value
+
+        def strip_deprecated_dir(key, value):
+            resolves_to = self.schema.paths_to_resolve.get(key)
+            if resolves_to:  # value is a path that will be resolved
+                first_dir = value.split(os.sep)[0]  # get first directory component
+                if first_dir == self.deprecated_dirs[resolves_to]:  # first_dir is deprecated for this option
+                    ignore = first_dir + os.sep
+                    log.warning(
+                        "Paths for the '%s' option are now relative to '%s', remove the leading '%s' "
+                        "to suppress this warning: %s", key, resolves_to, ignore, value
+                    )
+                    return value[len(ignore):]
+            return value
+
+        type_converters = {'bool': string_as_bool, 'int': int, 'float': float, 'str': str}
+
+        for key, value in kwargs.items():
+            if key in self.schema.app_schema:
+                value = convert_datatype(key, value)
+                if value:
+                    value = strip_deprecated_dir(key, value)
+                self._raw_config[key] = value
+
+    def _create_attributes_from_raw_config(self):
+        for key, value in self._raw_config.items():
+            if not hasattr(self, key):
+                setattr(self, key, value)
+            else:
+                log.debug("Attribute '%s' is set and cannot be overwritten with value '%s'" % (key, value))
+
+    def _resolve_paths(self, paths_to_resolve):
+
+        def resolve(key):
+            if key in _cache:  # resolve each path only once
+                return _cache[key]
+
+            path = getattr(self, key)  # path prior to being resolved
+            parent = paths_to_resolve.get(key)
+            if not parent:  # base case: nothing else needs resolving
+                return path
+            parent_path = resolve(parent)  # recursively resolve parent path
+            if path is not None:
+                path = os.path.join(parent_path, path)  # resolve path
+            else:
+                path = parent_path  # or use parent path
+
+            setattr(self, key, path)  # update property
+            _cache[key] = path  # cache it!
+            return path
+
+        _cache = {}
+        for key in paths_to_resolve:
+            resolve(key)
+
     def _set_config_base(self, config_kwargs):
 
         def _set_global_conf():
@@ -217,85 +294,12 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
     deprecated_dirs = {'config_dir': 'config', 'data_dir': 'database'}
 
     def __init__(self, **kwargs):
-        self.schema = self._load_schema()  # Load schema from schema definition file
-        self._raw_config = self.schema.defaults.copy()  # keeps track of startup values (kwargs or schema default)
-
-        self._load_schema()
-        self._update_raw_config_from_kwargs(kwargs)  # Overwrite default values passed as kwargs
-        self._create_attributes_from_raw_config()  # Create attributes for LOADED properties
-
-        self.config_dict = kwargs
-        self.root = find_root(kwargs)
-        self._set_config_base(kwargs)  # must be called prior to _resolve_paths()
-
-        self._resolve_paths(self.schema.paths_to_resolve)  # Overwrite attributes (not _raw_config) w/resolved paths
-        self._process_config(kwargs)  # Finish processing configuration
+        super(GalaxyAppConfiguration, self).__init__(**kwargs)
+        self._process_config(kwargs)
 
     def _load_schema(self):
         return AppSchema(GALAXY_CONFIG_SCHEMA_PATH, GALAXY_APP_NAME)
 
-    def _update_raw_config_from_kwargs(self, kwargs):
-
-        def convert_datatype(key, value):
-            datatype = self.schema.app_schema[key].get('type')
-            # check for `not None` explicitly (value can be falsy)
-            if value is not None and datatype in type_converters:
-                return type_converters[datatype](value)
-            return value
-
-        def strip_deprecated_dir(key, value):
-            resolves_to = self.schema.paths_to_resolve.get(key)
-            if resolves_to:  # value is a path that will be resolved
-                first_dir = value.split(os.sep)[0]  # get first directory component
-                if first_dir == self.deprecated_dirs[resolves_to]:  # first_dir is deprecated for this option
-                    ignore = first_dir + os.sep
-                    log.warning(
-                        "Paths for the '%s' option are now relative to '%s', remove the leading '%s' "
-                        "to suppress this warning: %s", key, resolves_to, ignore, value
-                    )
-                    return value[len(ignore):]
-            return value
-
-        type_converters = {'bool': string_as_bool, 'int': int, 'float': float, 'str': str}
-
-        for key, value in kwargs.items():
-            if key in self.schema.app_schema:
-                value = convert_datatype(key, value)
-                if value:
-                    value = strip_deprecated_dir(key, value)
-                self._raw_config[key] = value
-
-    def _create_attributes_from_raw_config(self):
-        for key, value in self._raw_config.items():
-            if not hasattr(self, key):
-                setattr(self, key, value)
-            else:
-                log.debug("Attribute '%s' is set and cannot be overwritten with value '%s'" % (key, value))
-
-    def _resolve_paths(self, paths_to_resolve):
-
-        def resolve(key):
-            if key in _cache:  # resolve each path only once
-                return _cache[key]
-
-            path = getattr(self, key)  # path prior to being resolved
-            parent = paths_to_resolve.get(key)
-            if not parent:  # base case: nothing else needs resolving
-                return path
-            parent_path = resolve(parent)  # recursively resolve parent path
-            if path is not None:
-                path = os.path.join(parent_path, path)  # resolve path
-            else:
-                path = parent_path  # or use parent path
-
-            setattr(self, key, path)  # update property
-            _cache[key] = path  # cache it!
-            return path
-
-        _cache = {}
-        for key in paths_to_resolve:
-            resolve(key)
-
     def _process_config(self, kwargs):
         # Resolve paths of other config files
         self.parse_config_file_options(kwargs)
diff --git a/lib/tool_shed/webapp/config.py b/lib/tool_shed/webapp/config.py
index 424207d353f..fa7cd14da2e 100644
--- a/lib/tool_shed/webapp/config.py
+++ b/lib/tool_shed/webapp/config.py
@@ -10,6 +10,7 @@ from datetime import timedelta
 from six.moves import configparser
 
 from galaxy.config import BaseAppConfiguration
+from galaxy.config.schema import AppSchema
 from galaxy.util import string_as_bool
 from galaxy.version import VERSION, VERSION_MAJOR
 from galaxy.web.formatting import expand_pretty_datetime_format
@@ -19,6 +20,9 @@ log = logging.getLogger(__name__)
 ts_webapp_path = os.path.abspath(os.path.dirname(__file__))
 templates_path = os.path.join(ts_webapp_path, "templates")
 
+TOOLSHED_APP_NAME = 'tool_shed'
+TOOLSHED_CONFIG_SCHEMA_PATH = 'lib/tool_shed/webapp/config_schema.yml'
+
 
 def resolve_path(path, root):
     """If 'path' is relative make absolute by prepending 'root'"""
@@ -34,6 +38,18 @@ class ConfigurationError(Exception):
 class ToolShedAppConfiguration(BaseAppConfiguration):
     default_config_file_name = 'tool_shed.yml'
 
+    def _load_schema():
+        return AppSchema(TOOLSHED_CONFIG_SCHEMA_PATH, TOOLSHED_APP_NAME)
+
+    def _update_raw_config_from_kwargs(self, kwargs):
+        pass  # Intentionally does nothing. Remove to enable.
+
+    def _create_attributes_from_raw_config(self):
+        pass  # Intentionally does nothing. Remove to enable.
+
+    def _resolve_paths(self, paths_to_resolve):
+        pass  # Intentionally does nothing. Remove to enable.
+
     def __init__(self, **kwargs):
         self.config_dict = kwargs
         self.root = kwargs.get('root_dir', '.')

From bd99a3a734d835a1c60b10955e874f33fc2b9ba8 Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Sun, 16 Feb 2020 00:56:51 -0500
Subject: [PATCH 25/71] Change handling of deprecated_dirs

This will prevent an error in TS if the base config class
`_update_raw_config_from_kwargs()` method is invoked.
---
 lib/galaxy/config/__init__.py             | 17 ++++++++++++-----
 test/unit/config/test_path_resolves_to.py |  4 ++--
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py
index 0cc90edb6d9..f6cd1fa0340 100644
--- a/lib/galaxy/config/__init__.py
+++ b/lib/galaxy/config/__init__.py
@@ -118,8 +118,15 @@ class BaseAppConfiguration(object):
         self._resolve_paths(self.schema.paths_to_resolve)  # Overwrite attributes (not _raw_config) w/resolved paths
 
     def _load_schema(self):
+        # Override in subclasses
         raise Exception('Not implemented')
 
+    def _deprecated_dirs():
+        # Override in subclasses (optional)
+        # {KEY: config option, VALUE: deprecated directory name}
+        # If VALUE == first directory in a user path that resolves to KEY, it will be stripped from the path
+        return None
+
     def _update_raw_config_from_kwargs(self, kwargs):
 
         def convert_datatype(key, value):
@@ -133,7 +140,7 @@ class BaseAppConfiguration(object):
             resolves_to = self.schema.paths_to_resolve.get(key)
             if resolves_to:  # value is a path that will be resolved
                 first_dir = value.split(os.sep)[0]  # get first directory component
-                if first_dir == self.deprecated_dirs[resolves_to]:  # first_dir is deprecated for this option
+                if first_dir == self._deprecated_dirs().get(resolves_to):  # first_dir is deprecated for this option
                     ignore = first_dir + os.sep
                     log.warning(
                         "Paths for the '%s' option are now relative to '%s', remove the leading '%s' "
@@ -147,7 +154,7 @@ class BaseAppConfiguration(object):
         for key, value in kwargs.items():
             if key in self.schema.app_schema:
                 value = convert_datatype(key, value)
-                if value:
+                if value and self._deprecated_dirs():
                     value = strip_deprecated_dir(key, value)
                 self._raw_config[key] = value
 
@@ -289,9 +296,6 @@ class BaseAppConfiguration(object):
 class GalaxyAppConfiguration(BaseAppConfiguration):
     deprecated_options = ('database_file', 'track_jobs_in_database')
     default_config_file_name = 'galaxy.yml'
-    # {key: config option, value: deprecated directory name}
-    # If value == first dir in a user path that resolves to key, it will be stripped from the path
-    deprecated_dirs = {'config_dir': 'config', 'data_dir': 'database'}
 
     def __init__(self, **kwargs):
         super(GalaxyAppConfiguration, self).__init__(**kwargs)
@@ -300,6 +304,9 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
     def _load_schema(self):
         return AppSchema(GALAXY_CONFIG_SCHEMA_PATH, GALAXY_APP_NAME)
 
+    def _deprecated_dirs(self):
+        return {'config_dir': 'config', 'data_dir': 'database'}
+
     def _process_config(self, kwargs):
         # Resolve paths of other config files
         self.parse_config_file_options(kwargs)
diff --git a/test/unit/config/test_path_resolves_to.py b/test/unit/config/test_path_resolves_to.py
index 439d2c6c001..b700a87d86a 100644
--- a/test/unit/config/test_path_resolves_to.py
+++ b/test/unit/config/test_path_resolves_to.py
@@ -46,14 +46,14 @@ def test_deprecated_prefixes_set_correctly(monkeypatch):
     monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', lambda a, b: None)
 
     config = GalaxyAppConfiguration()
-    assert config.deprecated_dirs == {'config_dir': 'config', 'data_dir': 'database'}
+    assert config._deprecated_dirs() == {'config_dir': 'config', 'data_dir': 'database'}
 
 
 @pytest.fixture
 def mock_init(monkeypatch):
     monkeypatch.setattr(AppSchema, '_read_schema', lambda a, b: get_schema(MOCK_SCHEMA))
     monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', lambda a, b: None)
-    monkeypatch.setattr(GalaxyAppConfiguration, 'deprecated_dirs', MOCK_DEPRECATED_DIRS)
+    monkeypatch.setattr(GalaxyAppConfiguration, '_deprecated_dirs', lambda a: MOCK_DEPRECATED_DIRS)
 
 
 def test_mock_schema_is_loaded(mock_init):

From f0bfcaf0cee5f2674aa8bef8b893e869529274c8 Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Sun, 16 Feb 2020 11:05:05 -0500
Subject: [PATCH 26/71] Reorder base config code, edit comments

---
 lib/galaxy/config/__init__.py | 132 +++++++++++++++++-----------------
 1 file changed, 65 insertions(+), 67 deletions(-)

diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py
index f6cd1fa0340..b3728b55acd 100644
--- a/lib/galaxy/config/__init__.py
+++ b/lib/galaxy/config/__init__.py
@@ -100,22 +100,78 @@ LOGGING_CONFIG_DEFAULT = {
 
 
 def find_root(kwargs):
-    root = os.path.abspath(kwargs.get('root_dir', '.'))
-    return root
+    return os.path.abspath(kwargs.get('root_dir', '.'))
 
 
 class BaseAppConfiguration(object):
 
     def __init__(self, **kwargs):
-        self.schema = self._load_schema()  # Load schema from schema definition file
-        self._raw_config = self.schema.defaults.copy()  # keeps track of startup values (kwargs or schema default)
-        self._load_schema()
-        self._update_raw_config_from_kwargs(kwargs)  # Overwrite default values passed as kwargs
-        self._create_attributes_from_raw_config()  # Create attributes for LOADED properties
         self.config_dict = kwargs
         self.root = find_root(kwargs)
-        self._set_config_base(kwargs)  # must be called prior to _resolve_paths()
-        self._resolve_paths(self.schema.paths_to_resolve)  # Overwrite attributes (not _raw_config) w/resolved paths
+        self._set_config_base(kwargs)
+        self.schema = self._load_schema()  # Load schema from schema definition file
+        self._raw_config = self.schema.defaults.copy()  # Save schema defaults as initial config values (raw_config)
+        self._update_raw_config_from_kwargs(kwargs)  # Overwrite raw_config with values passed in kwargs
+        self._create_attributes_from_raw_config()  # Create attributes based on raw_config
+        self._resolve_paths(self.schema.paths_to_resolve)  # Overwrite attribute values with resolved paths
+
+    def _set_config_base(self, config_kwargs):
+
+        def _set_global_conf():
+            self.config_file = find_config_file('galaxy')
+            self.global_conf = config_kwargs.get('global_conf')
+            self.global_conf_parser = configparser.ConfigParser()
+            if not self.config_file and self.global_conf and "__file__" in self.global_conf:
+                self.config_file = os.path.join(self.root, self.global_conf['__file__'])
+
+            if self.config_file is None:
+                log.warning("No Galaxy config file found, running from current working directory: %s", os.getcwd())
+            else:
+                try:
+                    self.global_conf_parser.read(self.config_file)
+                except (IOError, OSError):
+                    raise
+                except Exception:
+                    pass  # Not an INI file
+
+        def _set_config_directories():
+            # Set config_dir to value from kwargs OR dirname of config_file OR None
+            _config_dir = os.path.dirname(self.config_file) if self.config_file else None
+            self.config_dir = config_kwargs.get('config_dir', _config_dir)
+            # Make path absolute before using it as base for other paths
+            if self.config_dir:
+                self.config_dir = os.path.abspath(self.config_dir)
+
+            self.data_dir = config_kwargs.get('data_dir')
+            self.sample_config_dir = os.path.join(os.path.dirname(__file__), 'sample')
+            self.managed_config_dir = config_kwargs.get('managed_config_dir')
+            if self.managed_config_dir:
+                self.managed_config_dir = os.path.abspath(self.managed_config_dir)
+
+            if running_from_source:
+                if not self.config_dir:
+                    self.config_dir = os.path.join(self.root, 'config')
+                if not self.data_dir:
+                    self.data_dir = os.path.join(self.root, 'database')
+                if not self.managed_config_dir:
+                    self.managed_config_dir = self.config_dir
+            else:
+                if not self.config_dir:
+                    self.config_dir = os.getcwd()
+                if not self.data_dir:
+                    self.data_dir = self._in_config_dir('data')
+                if not self.managed_config_dir:
+                    self.managed_config_dir = self._in_data_dir('config')
+
+            # TODO: do we still need to support ../shed_tools when running_from_source?
+            self.shed_tools_dir = self._in_data_dir('shed_tools')
+
+            log.debug("Configuration directory is %s", self.config_dir)
+            log.debug("Data directory is %s", self.data_dir)
+            log.debug("Managed config directory is %s", self.managed_config_dir)
+
+        _set_global_conf()
+        _set_config_directories()
 
     def _load_schema(self):
         # Override in subclasses
@@ -189,64 +245,6 @@ class BaseAppConfiguration(object):
         for key in paths_to_resolve:
             resolve(key)
 
-    def _set_config_base(self, config_kwargs):
-
-        def _set_global_conf():
-            self.config_file = find_config_file('galaxy')
-            self.global_conf = config_kwargs.get('global_conf')
-            self.global_conf_parser = configparser.ConfigParser()
-            if not self.config_file and self.global_conf and "__file__" in self.global_conf:
-                self.config_file = os.path.join(self.root, self.global_conf['__file__'])
-
-            if self.config_file is None:
-                log.warning("No Galaxy config file found, running from current working directory: %s", os.getcwd())
-            else:
-                try:
-                    self.global_conf_parser.read(self.config_file)
-                except (IOError, OSError):
-                    raise
-                except Exception:
-                    pass  # Not an INI file
-
-        def _set_config_directories():
-            # Set config_dir to value from kwargs OR dirname of config_file OR None
-            _config_dir = os.path.dirname(self.config_file) if self.config_file else None
-            self.config_dir = config_kwargs.get('config_dir', _config_dir)
-            # Make path absolute before using it as base for other paths
-            if self.config_dir:
-                self.config_dir = os.path.abspath(self.config_dir)
-
-            self.data_dir = config_kwargs.get('data_dir')
-            self.sample_config_dir = os.path.join(os.path.dirname(__file__), 'sample')
-            self.managed_config_dir = config_kwargs.get('managed_config_dir')
-            if self.managed_config_dir:
-                self.managed_config_dir = os.path.abspath(self.managed_config_dir)
-
-            if running_from_source:
-                if not self.config_dir:
-                    self.config_dir = os.path.join(self.root, 'config')
-                if not self.data_dir:
-                    self.data_dir = os.path.join(self.root, 'database')
-                if not self.managed_config_dir:
-                    self.managed_config_dir = self.config_dir
-            else:
-                if not self.config_dir:
-                    self.config_dir = os.getcwd()
-                if not self.data_dir:
-                    self.data_dir = self._in_config_dir('data')
-                if not self.managed_config_dir:
-                    self.managed_config_dir = self._in_data_dir('config')
-
-            # TODO: do we still need to support ../shed_tools when running_from_source?
-            self.shed_tools_dir = self._in_data_dir('shed_tools')
-
-            log.debug("Configuration directory is %s", self.config_dir)
-            log.debug("Data directory is %s", self.data_dir)
-            log.debug("Managed config directory is %s", self.managed_config_dir)
-
-        _set_global_conf()
-        _set_config_directories()
-
     def _in_managed_config_dir(self, path):
         return os.path.join(self.managed_config_dir, path)
 

From d9041a76ebe4d1835eb2c0f264c5b2a2d50a7109 Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Mon, 17 Feb 2020 00:28:46 -0500
Subject: [PATCH 27/71] Correct _load_schema method signature; edit comments

Following the principle of least surprise.
The stub methods are not invoked because the parent's __init__() method
is not called. They can be safely removed. However, if a future commit adds
a call to parent's __init__, without these stub methods the parent's
methods will be called, which may result in a surprise (e.g., attributes
would be created for all schema properties).
---
 lib/tool_shed/webapp/config.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/tool_shed/webapp/config.py b/lib/tool_shed/webapp/config.py
index fa7cd14da2e..df627d91643 100644
--- a/lib/tool_shed/webapp/config.py
+++ b/lib/tool_shed/webapp/config.py
@@ -38,17 +38,17 @@ class ConfigurationError(Exception):
 class ToolShedAppConfiguration(BaseAppConfiguration):
     default_config_file_name = 'tool_shed.yml'
 
-    def _load_schema():
+    def _load_schema(self):
         return AppSchema(TOOLSHED_CONFIG_SCHEMA_PATH, TOOLSHED_APP_NAME)
 
     def _update_raw_config_from_kwargs(self, kwargs):
-        pass  # Intentionally does nothing. Remove to enable.
+        pass  # Intentionally does nothing. To enable, remove + call base __init__().
 
     def _create_attributes_from_raw_config(self):
-        pass  # Intentionally does nothing. Remove to enable.
+        pass  # Intentionally does nothing. To enable, remove + call base __init__().
 
     def _resolve_paths(self, paths_to_resolve):
-        pass  # Intentionally does nothing. Remove to enable.
+        pass  # Intentionally does nothing. To enable, remove + call base __init__().
 
     def __init__(self, **kwargs):
         self.config_dict = kwargs

From d8f98b227c5db0cce0c8159fa399bc273f9bf15a Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Mon, 17 Feb 2020 00:55:02 -0500
Subject: [PATCH 28/71] Fix _deprecated_dirs method signature

---
 lib/galaxy/config/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py
index b3728b55acd..9b77cbdde5e 100644
--- a/lib/galaxy/config/__init__.py
+++ b/lib/galaxy/config/__init__.py
@@ -177,7 +177,7 @@ class BaseAppConfiguration(object):
         # Override in subclasses
         raise Exception('Not implemented')
 
-    def _deprecated_dirs():
+    def _deprecated_dirs(self):
         # Override in subclasses (optional)
         # {KEY: config option, VALUE: deprecated directory name}
         # If VALUE == first directory in a user path that resolves to KEY, it will be stripped from the path

From 163ba59bb189cbb3bc42c7f7404ee075a6da8aeb Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Mon, 17 Feb 2020 17:27:24 -0500
Subject: [PATCH 29/71] Cleanup (address items from code review)

---
 lib/galaxy/config/__init__.py             | 25 +++++++++--------------
 test/unit/config/test_path_resolves_to.py |  4 ++--
 2 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py
index 9b77cbdde5e..982fa59666c 100644
--- a/lib/galaxy/config/__init__.py
+++ b/lib/galaxy/config/__init__.py
@@ -104,6 +104,9 @@ def find_root(kwargs):
 
 
 class BaseAppConfiguration(object):
+    # Override in subclasses (optional): {KEY: config option, VALUE: deprecated directory name}
+    # If VALUE == first directory in a user-supplied path that resolves to KEY, it will be stripped from that path
+    deprecated_dirs = None
 
     def __init__(self, **kwargs):
         self.config_dict = kwargs
@@ -113,7 +116,7 @@ class BaseAppConfiguration(object):
         self._raw_config = self.schema.defaults.copy()  # Save schema defaults as initial config values (raw_config)
         self._update_raw_config_from_kwargs(kwargs)  # Overwrite raw_config with values passed in kwargs
         self._create_attributes_from_raw_config()  # Create attributes based on raw_config
-        self._resolve_paths(self.schema.paths_to_resolve)  # Overwrite attribute values with resolved paths
+        self._resolve_paths()  # Overwrite attribute values with resolved paths
 
     def _set_config_base(self, config_kwargs):
 
@@ -177,12 +180,6 @@ class BaseAppConfiguration(object):
         # Override in subclasses
         raise Exception('Not implemented')
 
-    def _deprecated_dirs(self):
-        # Override in subclasses (optional)
-        # {KEY: config option, VALUE: deprecated directory name}
-        # If VALUE == first directory in a user path that resolves to KEY, it will be stripped from the path
-        return None
-
     def _update_raw_config_from_kwargs(self, kwargs):
 
         def convert_datatype(key, value):
@@ -196,7 +193,7 @@ class BaseAppConfiguration(object):
             resolves_to = self.schema.paths_to_resolve.get(key)
             if resolves_to:  # value is a path that will be resolved
                 first_dir = value.split(os.sep)[0]  # get first directory component
-                if first_dir == self._deprecated_dirs().get(resolves_to):  # first_dir is deprecated for this option
+                if first_dir == self.deprecated_dirs.get(resolves_to):  # first_dir is deprecated for this option
                     ignore = first_dir + os.sep
                     log.warning(
                         "Paths for the '%s' option are now relative to '%s', remove the leading '%s' "
@@ -210,7 +207,7 @@ class BaseAppConfiguration(object):
         for key, value in kwargs.items():
             if key in self.schema.app_schema:
                 value = convert_datatype(key, value)
-                if value and self._deprecated_dirs():
+                if value and self.deprecated_dirs:
                     value = strip_deprecated_dir(key, value)
                 self._raw_config[key] = value
 
@@ -221,14 +218,14 @@ class BaseAppConfiguration(object):
             else:
                 log.debug("Attribute '%s' is set and cannot be overwritten with value '%s'" % (key, value))
 
-    def _resolve_paths(self, paths_to_resolve):
+    def _resolve_paths(self):
 
         def resolve(key):
             if key in _cache:  # resolve each path only once
                 return _cache[key]
 
             path = getattr(self, key)  # path prior to being resolved
-            parent = paths_to_resolve.get(key)
+            parent = self.schema.paths_to_resolve.get(key)
             if not parent:  # base case: nothing else needs resolving
                 return path
             parent_path = resolve(parent)  # recursively resolve parent path
@@ -242,7 +239,7 @@ class BaseAppConfiguration(object):
             return path
 
         _cache = {}
-        for key in paths_to_resolve:
+        for key in self.schema.paths_to_resolve:
             resolve(key)
 
     def _in_managed_config_dir(self, path):
@@ -294,6 +291,7 @@ class BaseAppConfiguration(object):
 class GalaxyAppConfiguration(BaseAppConfiguration):
     deprecated_options = ('database_file', 'track_jobs_in_database')
     default_config_file_name = 'galaxy.yml'
+    deprecated_dirs = {'config_dir': 'config', 'data_dir': 'database'}
 
     def __init__(self, **kwargs):
         super(GalaxyAppConfiguration, self).__init__(**kwargs)
@@ -302,9 +300,6 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
     def _load_schema(self):
         return AppSchema(GALAXY_CONFIG_SCHEMA_PATH, GALAXY_APP_NAME)
 
-    def _deprecated_dirs(self):
-        return {'config_dir': 'config', 'data_dir': 'database'}
-
     def _process_config(self, kwargs):
         # Resolve paths of other config files
         self.parse_config_file_options(kwargs)
diff --git a/test/unit/config/test_path_resolves_to.py b/test/unit/config/test_path_resolves_to.py
index b700a87d86a..439d2c6c001 100644
--- a/test/unit/config/test_path_resolves_to.py
+++ b/test/unit/config/test_path_resolves_to.py
@@ -46,14 +46,14 @@ def test_deprecated_prefixes_set_correctly(monkeypatch):
     monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', lambda a, b: None)
 
     config = GalaxyAppConfiguration()
-    assert config._deprecated_dirs() == {'config_dir': 'config', 'data_dir': 'database'}
+    assert config.deprecated_dirs == {'config_dir': 'config', 'data_dir': 'database'}
 
 
 @pytest.fixture
 def mock_init(monkeypatch):
     monkeypatch.setattr(AppSchema, '_read_schema', lambda a, b: get_schema(MOCK_SCHEMA))
     monkeypatch.setattr(GalaxyAppConfiguration, '_process_config', lambda a, b: None)
-    monkeypatch.setattr(GalaxyAppConfiguration, '_deprecated_dirs', lambda a: MOCK_DEPRECATED_DIRS)
+    monkeypatch.setattr(GalaxyAppConfiguration, 'deprecated_dirs', MOCK_DEPRECATED_DIRS)
 
 
 def test_mock_schema_is_loaded(mock_init):

From 49803eeb033cdfa5c41b7699cf851500c6791f07 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Mon, 2 Mar 2020 15:58:55 -0500
Subject: [PATCH 30/71] Fix workflow ribbon shift calculation

---
 client/galaxy/scripts/mvc/workflow/workflow-connector.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/client/galaxy/scripts/mvc/workflow/workflow-connector.js b/client/galaxy/scripts/mvc/workflow/workflow-connector.js
index 9f107e01199..eeb2735d7de 100644
--- a/client/galaxy/scripts/mvc/workflow/workflow-connector.js
+++ b/client/galaxy/scripts/mvc/workflow/workflow-connector.js
@@ -120,7 +120,7 @@ class Connector {
         end_y -= canvas_top;
 
         // Determine line shift
-        const cp_shift = Math.min(Math.max(Math.abs(canvas_max_y - canvas_min_y) / 2, cpFactor), 300 / cpFactor);
+        const cp_shift = Math.min(Math.max(Math.abs(canvas_max_y - canvas_min_y) / 2, cpFactor), 3 * cpFactor);
 
         // Draw ribbons
         this.drawRibbon(handle1, handle2, cp_shift, start_x, start_y, end_x, end_y);

From 6f1c548ec21b828270652d98e0cf7e807e18612f Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Mon, 2 Mar 2020 16:22:48 -0500
Subject: [PATCH 31/71] Improve header button alignment

---
 client/galaxy/style/scss/workflow.scss | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/client/galaxy/style/scss/workflow.scss b/client/galaxy/style/scss/workflow.scss
index de03e0c8d3b..260b3f312ba 100644
--- a/client/galaxy/style/scss/workflow.scss
+++ b/client/galaxy/style/scss/workflow.scss
@@ -23,6 +23,9 @@
                 @extend .px-2;
                 cursor: move;
                 background: $brand-secondary;
+                .header {
+                    width: $worfklow-node-width - 2rem;
+                }
             }
             .node-body {
                 @extend .card-body;

From 9dca223525ddf67aef696d1a066613dfd5480638 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Tue, 3 Mar 2020 01:06:11 -0500
Subject: [PATCH 32/71] Fix css selector for workflow node connection canvas to
 svg, use brand primary for node headers

---
 client/galaxy/style/scss/theme/blue.scss | 2 +-
 client/galaxy/style/scss/workflow.scss   | 7 ++++++-
 lib/galaxy/selenium/navigation.yml       | 2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/client/galaxy/style/scss/theme/blue.scss b/client/galaxy/style/scss/theme/blue.scss
index 1f463801fb2..49961495ceb 100644
--- a/client/galaxy/style/scss/theme/blue.scss
+++ b/client/galaxy/style/scss/theme/blue.scss
@@ -152,7 +152,7 @@ $state-running-border: $border-color;
 // Workflow editor
 $workflow-editor-bg: $white;
 $workflow-editor-grid-size: 0.7rem;
-$workflow-editor-grid-color: lighten($brand-primary, 60%);
+$workflow-editor-grid-color: lighten($brand-primary, 65%);
 $workflow-overview-bg: $panel-bg-color;
 $worfklow-node-width: 10rem;
 
diff --git a/client/galaxy/style/scss/workflow.scss b/client/galaxy/style/scss/workflow.scss
index 260b3f312ba..039600e7c71 100644
--- a/client/galaxy/style/scss/workflow.scss
+++ b/client/galaxy/style/scss/workflow.scss
@@ -15,6 +15,7 @@
             border: solid $brand-primary 1px;
             &.node-active {
                 z-index: 1001;
+                border: solid $white 1px;
                 box-shadow: 0 0 0 2px $brand-primary;
             }
             .node-header {
@@ -22,10 +23,14 @@
                 @extend .py-1;
                 @extend .px-2;
                 cursor: move;
-                background: $brand-secondary;
+                background: $brand-primary;
+                color: $white;
                 .header {
                     width: $worfklow-node-width - 2rem;
                 }
+                .button {
+                    color: $white;
+                }
             }
             .node-body {
                 @extend .card-body;
diff --git a/lib/galaxy/selenium/navigation.yml b/lib/galaxy/selenium/navigation.yml
index ae2678deb0f..4b8851ab78c 100644
--- a/lib/galaxy/selenium/navigation.yml
+++ b/lib/galaxy/selenium/navigation.yml
@@ -294,7 +294,7 @@ workflow_editor:
     label_input: "[tour_id='__label'] input"
     annotation_input: "[tour_id='__annotation'] textarea"
 
-    connector_for: "canvas[handle1-id='${source_id}'][handle2-id='${sink_id}']"
+    connector_for: "div[handle1-id='${source_id}'][handle2-id='${sink_id}']"
 
     connector_destroy_callout: '.delete-terminal'
 

From 61705a054a30bff06d935477396e6c2a84d232c5 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Tue, 3 Mar 2020 02:53:56 -0500
Subject: [PATCH 33/71] Restore default workflow node styles

---
 client/galaxy/style/scss/theme/blue.scss | 2 +-
 client/galaxy/style/scss/workflow.scss   | 6 +-----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/client/galaxy/style/scss/theme/blue.scss b/client/galaxy/style/scss/theme/blue.scss
index 49961495ceb..1f463801fb2 100644
--- a/client/galaxy/style/scss/theme/blue.scss
+++ b/client/galaxy/style/scss/theme/blue.scss
@@ -152,7 +152,7 @@ $state-running-border: $border-color;
 // Workflow editor
 $workflow-editor-bg: $white;
 $workflow-editor-grid-size: 0.7rem;
-$workflow-editor-grid-color: lighten($brand-primary, 65%);
+$workflow-editor-grid-color: lighten($brand-primary, 60%);
 $workflow-overview-bg: $panel-bg-color;
 $worfklow-node-width: 10rem;
 
diff --git a/client/galaxy/style/scss/workflow.scss b/client/galaxy/style/scss/workflow.scss
index 039600e7c71..b34decf2b3b 100644
--- a/client/galaxy/style/scss/workflow.scss
+++ b/client/galaxy/style/scss/workflow.scss
@@ -23,14 +23,10 @@
                 @extend .py-1;
                 @extend .px-2;
                 cursor: move;
-                background: $brand-primary;
-                color: $white;
+                background: $brand-secondary;
                 .header {
                     width: $worfklow-node-width - 2rem;
                 }
-                .button {
-                    color: $white;
-                }
             }
             .node-body {
                 @extend .card-body;

From 20aaca36b1ed14487daedaea131a64e51f6bad66 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Tue, 3 Mar 2020 12:37:05 +0100
Subject: [PATCH 34/71] Add an integration test case for swift/s3

---
 .../objectstore/test_swift_objectstore.py     | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 test/integration/objectstore/test_swift_objectstore.py

diff --git a/test/integration/objectstore/test_swift_objectstore.py b/test/integration/objectstore/test_swift_objectstore.py
new file mode 100644
index 00000000000..91b778d263b
--- /dev/null
+++ b/test/integration/objectstore/test_swift_objectstore.py
@@ -0,0 +1,79 @@
+
+import os
+import string
+
+from galaxy_test.driver import integration_util
+
+OBJECT_STORE_HOST = os.environ.get('GALAXY_INTEGRATION_OBJECT_STORE_HOST', '127.0.0.1')
+OBJECT_STORE_PORT = int(os.environ.get('GALAXY_INTEGRATION_OBJECT_STORE_PORT', 9000))
+OBJECT_STORE_ACCESS_KEY = os.environ.get('GALAXY_INTEGRATION_OBJECT_STORE_ACCESS_KEY', 'minioadmin')
+OBJECT_STORE_SECRET_KEY = os.environ.get('GALAXY_INTEGRATION_OBJECT_STORE_SECRET_KEY', 'minioadmin')
+OBJECT_STORE_CONFIG = string.Template("""
+<object_store type="hierarchical" id="primary">
+    <backends>
+        <object_store id="swifty" type="swift" weight="1" order="0">
+            <auth access_key="${access_key}" secret_key="${secret_key}" />
+            <bucket name="galaxy" use_reduced_redundancy="False" max_chunk_size="250"/>
+            <connection host="${host}" port="${port}" is_secure="False" conn_path="" multipart="True"/>
+            <cache path="${temp_directory}/object_store_cache" size="1000" />
+            <extra_dir type="job_work" path="${temp_directory}/job_working_directory_swift"/>
+            <extra_dir type="temp" path="${temp_directory}/tmp_swift"/>
+        </object_store>
+    </backends>
+</object_store>
+""")
+TEST_TOOL_IDS = [
+    "multi_output",
+    "multi_output_configured",
+    "multi_output_assign_primary",
+    "multi_output_recurse",
+    "tool_provided_metadata_1",
+    "tool_provided_metadata_2",
+    "tool_provided_metadata_3",
+    "tool_provided_metadata_4",
+    "tool_provided_metadata_5",
+    "tool_provided_metadata_6",
+    "tool_provided_metadata_7",
+    "tool_provided_metadata_8",
+    "tool_provided_metadata_9",
+    "tool_provided_metadata_10",
+    "tool_provided_metadata_11",
+    "tool_provided_metadata_12",
+    "composite_output",
+    "composite_output_tests",
+    "metadata",
+    "metadata_bam",
+    "output_format",
+    "output_auto_format",
+]
+
+
+class SwiftObjectStoreIntegrationTestCase(integration_util.IntegrationTestCase):
+
+    @classmethod
+    def handle_galaxy_config_kwds(cls, config):
+        temp_directory = cls._test_driver.mkdtemp()
+        cls.object_stores_parent = temp_directory
+        config_path = os.path.join(temp_directory, "object_store_conf.xml")
+        config["object_store_store_by"] = "uuid"
+        config["metadata_strategy"] = "extended"
+        config["outpus_to_working_dir"] = True
+        config["retry_metadata_internally"] = False
+        with open(config_path, "w") as f:
+            f.write(
+                OBJECT_STORE_CONFIG.safe_substitute(
+                    {
+                        "temp_directory": temp_directory,
+                        "host": OBJECT_STORE_HOST,
+                        "port": OBJECT_STORE_PORT,
+                        "access_key": OBJECT_STORE_ACCESS_KEY,
+                        "secret_key": OBJECT_STORE_SECRET_KEY,
+                    }
+                )
+            )
+        config["object_store_config_file"] = config_path
+
+
+instance = integration_util.integration_module_instance(SwiftObjectStoreIntegrationTestCase)
+
+test_tools = integration_util.integration_tool_runner(TEST_TOOL_IDS)
\ No newline at end of file

From 8fef712a5d67e8f6fafef91816cae5b6e0e1722c Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Tue, 3 Mar 2020 12:57:40 +0100
Subject: [PATCH 35/71] Don't use mutable default argument

---
 lib/galaxy/objectstore/__init__.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/galaxy/objectstore/__init__.py b/lib/galaxy/objectstore/__init__.py
index ce8f2a579b5..d0dc0d6ac48 100644
--- a/lib/galaxy/objectstore/__init__.py
+++ b/lib/galaxy/objectstore/__init__.py
@@ -82,7 +82,7 @@ class ObjectStore(object):
         000/obj.id)
     """
 
-    def __init__(self, config, config_dict={}, **kwargs):
+    def __init__(self, config, config_dict=None, **kwargs):
         """
         :type config: object
         :param config: An object, most likely populated from
@@ -95,6 +95,8 @@ class ObjectStore(object):
               parent directory those directories will be created.
             * new_file_path -- Used to set the 'temp' extra_dir.
         """
+        if config_dict is None:
+            config_dict = {}
         self.running = True
         self.config = config
         self.check_old_style = config.object_store_check_old_style

From 6a56f3a5deb51dd1fafa8c588f32b14e67d10245 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Tue, 3 Mar 2020 12:58:48 +0100
Subject: [PATCH 36/71] Disable cleanup thread in set_metadata.py

---
 lib/galaxy/objectstore/s3.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py
index 09e78626e24..2da1d12b575 100644
--- a/lib/galaxy/objectstore/s3.py
+++ b/lib/galaxy/objectstore/s3.py
@@ -123,7 +123,7 @@ class CloudConfigMixin(object):
                 'conn_path': self.conn_path,
             },
             'cache': {
-                'size': self.cache_size,
+                'size': -1,  # disable cache cleaning when starting an exported object store in set_metadata.py
                 'path': self.staging_path,
             }
         }

From 57e84e78a38fda472067895e30b75d92c13084b6 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Tue, 3 Mar 2020 13:00:37 +0100
Subject: [PATCH 37/71] Fix store_by_uuid and extended metadata collection for
 s3 obejct stores

---
 lib/galaxy/objectstore/s3.py | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py
index 2da1d12b575..992c7fb968b 100644
--- a/lib/galaxy/objectstore/s3.py
+++ b/lib/galaxy/objectstore/s3.py
@@ -138,7 +138,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
     store_type = 's3'
 
     def __init__(self, config, config_dict):
-        super(S3ObjectStore, self).__init__(config)
+        super(S3ObjectStore, self).__init__(config, config_dict)
 
         self.transfer_progress = 0
 
@@ -162,6 +162,8 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
 
         self.cache_size = cache_dict.get('size', -1)
         self.staging_path = cache_dict.get('path') or self.config.object_store_cache_path
+        self.store_by = config_dict.get("store_by", None) or getattr(config, "object_store_store_by", "id")
+        assert self.store_by in ["id", "uuid"]
 
         extra_dirs = dict(
             (e['type'], e['path']) for e in config_dict.get('extra_dirs', []))
@@ -187,7 +189,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
                          'conn_path': self.conn_path}
 
         self._configure_connection()
-        self.bucket = self._get_bucket(self.bucket)
+        self.__bucket = self._get_bucket(self.bucket)
         # Clean cache only if value is set in galaxy.ini
         if self.cache_size != -1:
             # Convert GBs to bytes for comparison
@@ -325,7 +327,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
             # alt_name can contain parent directory references, but S3 will not
             # follow them, so if they are valid we normalize them out
             alt_name = os.path.normpath(alt_name)
-        rel_path = os.path.join(*directory_hash_id(obj.id))
+        rel_path = os.path.join(*directory_hash_id(self._get_object_id(obj)))
         if extra_dir is not None:
             if extra_dir_at_root:
                 rel_path = os.path.join(extra_dir, rel_path)
@@ -334,7 +336,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
 
         # for JOB_WORK directory
         if obj_dir:
-            rel_path = os.path.join(rel_path, str(obj.id))
+            rel_path = os.path.join(rel_path, str(self._get_object_id(obj)))
         if base_dir:
             base = self.extra_dirs.get(base_dir)
             return os.path.join(base, rel_path)
@@ -343,7 +345,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
         rel_path = '%s/' % rel_path
 
         if not dir_only:
-            rel_path = os.path.join(rel_path, alt_name if alt_name else "dataset_%s.dat" % obj.id)
+            rel_path = os.path.join(rel_path, alt_name if alt_name else "dataset_%s.dat" % self._get_object_id(obj))
         return rel_path
 
     def _get_cache_path(self, rel_path):
@@ -354,7 +356,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
 
     def _get_size_in_s3(self, rel_path):
         try:
-            key = self.bucket.get_key(rel_path)
+            key = self.__bucket.get_key(rel_path)
             if key:
                 return key.size
         except S3ResponseError:
@@ -367,13 +369,13 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
             # A hackish way of testing if the rel_path is a folder vs a file
             is_dir = rel_path[-1] == '/'
             if is_dir:
-                keyresult = self.bucket.get_all_keys(prefix=rel_path)
+                keyresult = self.__bucket.get_all_keys(prefix=rel_path)
                 if len(keyresult) > 0:
                     exists = True
                 else:
                     exists = False
             else:
-                key = Key(self.bucket, rel_path)
+                key = Key(self.__bucket, rel_path)
                 exists = key.exists()
         except S3ResponseError:
             log.exception("Trouble checking existence of S3 key '%s'", rel_path)
@@ -425,7 +427,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
     def _download(self, rel_path):
         try:
             log.debug("Pulling key '%s' into cache to %s", rel_path, self._get_cache_path(rel_path))
-            key = self.bucket.get_key(rel_path)
+            key = self.__bucket.get_key(rel_path)
             # Test if cache is large enough to hold the new file
             if self.cache_size > 0 and key.size > self.cache_size:
                 log.critical("File %s is larger (%s) than the cache size (%s). Cannot download.",
@@ -444,7 +446,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
                 key.get_contents_to_filename(self._get_cache_path(rel_path), cb=self._transfer_cb, num_cb=10)
                 return True
         except S3ResponseError:
-            log.exception("Problem downloading key '%s' from S3 bucket '%s'", rel_path, self.bucket.name)
+            log.exception("Problem downloading key '%s' from S3 bucket '%s'", rel_path, self.__bucket.name)
         return False
 
     def _push_to_os(self, rel_path, source_file=None, from_string=None):
@@ -458,7 +460,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
         try:
             source_file = source_file if source_file else self._get_cache_path(rel_path)
             if os.path.exists(source_file):
-                key = Key(self.bucket, rel_path)
+                key = Key(self.__bucket, rel_path)
                 if os.path.getsize(source_file) == 0 and key.exists():
                     log.debug("Wanted to push file '%s' to S3 key '%s' but its size is 0; skipping.", source_file, rel_path)
                     return True
@@ -476,7 +478,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
                                                        cb=self._transfer_cb,
                                                        num_cb=10)
                     else:
-                        multipart_upload(self.s3server, self.bucket, key.name, source_file, mb_size)
+                        multipart_upload(self.s3server, self.__bucket, key.name, source_file, mb_size)
                     end_time = datetime.now()
                     log.debug("Pushed cache file '%s' to key '%s' (%s bytes transfered in %s sec)",
                               source_file, rel_path, os.path.getsize(source_file), end_time - start_time)
@@ -545,7 +547,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
             alt_name = kwargs.get('alt_name', None)
 
             # Construct hashed path
-            rel_path = os.path.join(*directory_hash_id(obj.id))
+            rel_path = os.path.join(*directory_hash_id(self._get_object_id(obj)))
 
             # Optionally append extra_dir
             if extra_dir is not None:
@@ -566,7 +568,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
             # self._push_to_os(s3_dir, from_string='')
             # If instructed, create the dataset in cache & in S3
             if not dir_only:
-                rel_path = os.path.join(rel_path, alt_name if alt_name else "dataset_%s.dat" % obj.id)
+                rel_path = os.path.join(rel_path, alt_name if alt_name else "dataset_%s.dat" % self._get_object_id(obj))
                 open(os.path.join(self.staging_path, rel_path), 'w').close()
                 self._push_to_os(rel_path, from_string='')
 
@@ -607,7 +609,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
             # but requires iterating through each individual key in S3 and deleing it.
             if entire_dir and extra_dir:
                 shutil.rmtree(self._get_cache_path(rel_path))
-                results = self.bucket.get_all_keys(prefix=rel_path)
+                results = self.__bucket.get_all_keys(prefix=rel_path)
                 for key in results:
                     log.debug("Deleting key %s", key.name)
                     key.delete()
@@ -617,7 +619,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
                 os.unlink(self._get_cache_path(rel_path))
                 # Delete from S3 as well
                 if self._key_exists(rel_path):
-                    key = Key(self.bucket, rel_path)
+                    key = Key(self.__bucket, rel_path)
                     log.debug("Deleting key %s", key.name)
                     key.delete()
                     return True
@@ -705,7 +707,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
         if self.exists(obj, **kwargs):
             rel_path = self._construct_path(obj, **kwargs)
             try:
-                key = Key(self.bucket, rel_path)
+                key = Key(self.__bucket, rel_path)
                 return key.generate_url(expires_in=86400)  # 24hrs
             except S3ResponseError:
                 log.exception("Trouble generating URL for dataset '%s'", rel_path)

From 0a252ed4f184dd1a5c46b655c4d418173c999cce Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Tue, 3 Mar 2020 13:01:13 +0100
Subject: [PATCH 38/71] Manage minio docker container in integration test

---
 .../objectstore/test_swift_objectstore.py     | 36 ++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/test/integration/objectstore/test_swift_objectstore.py b/test/integration/objectstore/test_swift_objectstore.py
index 91b778d263b..a2efd076d54 100644
--- a/test/integration/objectstore/test_swift_objectstore.py
+++ b/test/integration/objectstore/test_swift_objectstore.py
@@ -1,6 +1,7 @@
 
 import os
 import string
+import subprocess
 
 from galaxy_test.driver import integration_util
 
@@ -48,11 +49,44 @@ TEST_TOOL_IDS = [
 ]
 
 
+def start_minio(container_name):
+    minio_start_args = [
+        'docker',
+        'run',
+        '-p',
+        '{port}:9000'.format(port=OBJECT_STORE_PORT),
+        '-d',
+        '--name',
+        container_name,
+        # '--rm',
+        'minio/minio:latest',
+        'server',
+        '/tmp/data']
+    subprocess.check_call(minio_start_args)
+
+
+def stop_minio(container_name):
+    subprocess.check_call(['docker', 'stop', container_name])
+
+
+@integration_util.skip_unless_docker()
 class SwiftObjectStoreIntegrationTestCase(integration_util.IntegrationTestCase):
 
+    @classmethod
+    def setUpClass(cls):
+        cls.container_name = "%s_container" % cls.__name__
+        start_minio(cls.container_name)
+        super(SwiftObjectStoreIntegrationTestCase, cls).setUpClass()
+
+    @classmethod
+    def tearDownClass(cls):
+        stop_minio(cls.container_name)
+        super(SwiftObjectStoreIntegrationTestCase, cls).tearDownClass()
+
     @classmethod
     def handle_galaxy_config_kwds(cls, config):
         temp_directory = cls._test_driver.mkdtemp()
+        cls.container_name = os.path.basename(temp_directory)
         cls.object_stores_parent = temp_directory
         config_path = os.path.join(temp_directory, "object_store_conf.xml")
         config["object_store_store_by"] = "uuid"
@@ -76,4 +110,4 @@ class SwiftObjectStoreIntegrationTestCase(integration_util.IntegrationTestCase):
 
 instance = integration_util.integration_module_instance(SwiftObjectStoreIntegrationTestCase)
 
-test_tools = integration_util.integration_tool_runner(TEST_TOOL_IDS)
\ No newline at end of file
+test_tools = integration_util.integration_tool_runner(TEST_TOOL_IDS)

From 110f00495fcfbc199e4737fae63b897f680ed9eb Mon Sep 17 00:00:00 2001
From: Oleg Zharkov <oleg.zharkov22@gmail.com>
Date: Tue, 11 Feb 2020 15:00:40 +0100
Subject: [PATCH 39/71] implement selenium test User->Preferences->Manage
 information workflow

---
 .../components/User/UserPreferences.vue       |   6 +-
 .../components/User/UserPreferencesModel.js   |   9 ++
 lib/galaxy/selenium/navigation.yml            |  16 +++
 .../selenium/test_personal_information.py     | 119 ++++++++++++++++++
 4 files changed, 147 insertions(+), 3 deletions(-)
 create mode 100644 lib/galaxy_test/selenium/test_personal_information.py

diff --git a/client/galaxy/scripts/components/User/UserPreferences.vue b/client/galaxy/scripts/components/User/UserPreferences.vue
index 6bc0e467dd5..e311ff93023 100644
--- a/client/galaxy/scripts/components/User/UserPreferences.vue
+++ b/client/galaxy/scripts/components/User/UserPreferences.vue
@@ -5,16 +5,16 @@
             {{ message }}
         </b-alert>
         <p>
-            You are logged in as <strong>{{ email }}</strong
+            You are logged in as <strong id="user-preferences-current-email">{{ email }}</strong
             >.
         </p>
         <b-row class="ml-3 mb-1" v-for="(link, index) in activeLinks" :key="index">
             <i :class="['pref-icon pt-1 fa fa-lg', link.icon]" />
             <div class="pref-content pr-1">
-                <a v-if="link.onclick" @click="link.onclick" href="javascript:void(0)"
+                <a :id="link.id" v-if="link.onclick" @click="link.onclick" href="javascript:void(0)"
                     ><b>{{ link.title }}</b></a
                 >
-                <a v-else :href="`${baseUrl}/${link.action}`"
+                <a :id="link.id" v-else :href="`${baseUrl}/${link.action}`"
                     ><b>{{ link.title }}</b></a
                 >
                 <div class="form-text text-muted">
diff --git a/client/galaxy/scripts/components/User/UserPreferencesModel.js b/client/galaxy/scripts/components/User/UserPreferencesModel.js
index 58660c8549e..702d038ae39 100644
--- a/client/galaxy/scripts/components/User/UserPreferencesModel.js
+++ b/client/galaxy/scripts/components/User/UserPreferencesModel.js
@@ -7,6 +7,7 @@ export const getUserPreferencesModel = () => {
     return {
         information: {
             title: _l("Manage Information"),
+            id: "edit-preferences-information",
             description: "Edit your email, addresses and custom parameters or change your public name.",
             url: `api/users/${Galaxy.user.id}/information/inputs`,
             icon: "fa-user",
@@ -15,6 +16,7 @@ export const getUserPreferencesModel = () => {
         },
         password: {
             title: _l("Change Password"),
+            id: "edit-preferences-password",
             description: _l("Allows you to change your login credentials."),
             icon: "fa-unlock-alt",
             url: `api/users/${Galaxy.user.id}/password/inputs`,
@@ -24,6 +26,7 @@ export const getUserPreferencesModel = () => {
         },
         communication: {
             title: _l("Change Communication Settings"),
+            id: "edit-preferences-communication",
             description: _l("Enable or disable the communication feature to chat with other users."),
             url: `api/users/${Galaxy.user.id}/communication/inputs`,
             icon: "fa-comments-o",
@@ -32,6 +35,7 @@ export const getUserPreferencesModel = () => {
         },
         permissions: {
             title: _l("Set Dataset Permissions for new Histories"),
+            id: "edit-preferences-permissions",
             description:
                 "Grant others default access to newly created histories. Changes made here will only affect histories created after these settings have been stored.",
             url: `api/users/${Galaxy.user.id}/permissions/inputs`,
@@ -41,11 +45,13 @@ export const getUserPreferencesModel = () => {
         },
         make_data_private: {
             title: _l("Make all Data Private"),
+            id: "edit-preferences-make-data-private",
             description: _l("Click here to make all data private."),
             icon: "fa-lock"
         },
         api_key: {
             title: _l("Manage API Key"),
+            id: "edit-preferences-api-key",
             description: _l("Access your current API key or create a new one."),
             url: `api/users/${Galaxy.user.id}/api_key/inputs`,
             icon: "fa-key",
@@ -53,6 +59,7 @@ export const getUserPreferencesModel = () => {
             submit_icon: "fa-check"
         },
         cloud_auth: {
+            id: "edit-preferences-cloud-auth",
             title: _l("Manage Cloud Authorization"),
             description: _l("Add or modify the configuration that grants Galaxy to access your cloud-based resources."),
             icon: "fa-cloud",
@@ -61,6 +68,7 @@ export const getUserPreferencesModel = () => {
         },
         toolbox_filters: {
             title: _l("Manage Toolbox Filters"),
+            id: "edit-preferences-toolbox-filters",
             description: _l("Customize your Toolbox by displaying or omitting sets of Tools."),
             url: `api/users/${Galaxy.user.id}/toolbox_filters/inputs`,
             icon: "fa-filter",
@@ -75,6 +83,7 @@ export const getUserPreferencesModel = () => {
         },
         logout: {
             title: _l("Sign Out"),
+            id: "edit-preferences-custom-builds",
             description: _l("Click here to sign out of all sessions."),
             icon: "fa-sign-out",
             shouldRender: !!Galaxy.session_csrf_token
diff --git a/lib/galaxy/selenium/navigation.yml b/lib/galaxy/selenium/navigation.yml
index ae2678deb0f..2e4ea2fe4b7 100644
--- a/lib/galaxy/selenium/navigation.yml
+++ b/lib/galaxy/selenium/navigation.yml
@@ -60,6 +60,7 @@ masthead:
     # user menu
     logout: 'Logout'
     custom_builds: 'Custom Builds'
+    preferences: 'Preferences'
     histories: 'Histories'
     pages: 'Pages'
 
@@ -67,6 +68,21 @@ masthead:
     libraries: 'Data Libraries'
     published_histories: 'Histories'
 
+preferences:
+  selectors:
+    manage_information: '#edit-preferences-information'
+    current_email: "#user-preferences-current-email"
+
+change_user_email:
+  selectors:
+    submit: '#submit'
+
+change_user_address:
+  selectors:
+    address_button:
+      type: xpath
+      selector: '//span[contains(text(), "Insert Address")]'
+
 history_panel:
   menu:
     labels:
diff --git a/lib/galaxy_test/selenium/test_personal_information.py b/lib/galaxy_test/selenium/test_personal_information.py
new file mode 100644
index 00000000000..d1aef1b7dee
--- /dev/null
+++ b/lib/galaxy_test/selenium/test_personal_information.py
@@ -0,0 +1,119 @@
+from .framework import (
+    selenium_test,
+    SeleniumTestCase,
+)
+
+
+class ManageInformationTestCase(SeleniumTestCase):
+
+    @selenium_test
+    def test_change_email(self):
+        def assert_email(email_to_check):
+            self.assertTrue(email_to_check == self.driver.find_element_by_id("user-preferences-current-email").text)
+
+        email = self._get_random_email()
+        self.register(email)
+        self.navigate_to_user_preferences()
+        self.sleep_for(self.wait_types.UX_RENDER)
+
+        # rendered email should be equal
+        assert_email(email)
+
+        self.components.preferences.manage_information.wait_for_and_click()
+        self.sleep_for(self.wait_types.UX_RENDER)
+
+        new_email = self._get_random_email()
+        # new email should be different from initially registered
+        self.assertTrue(email != new_email)
+        email_input_field = self.driver.find_element_by_css_selector("[tour_id='email'] input")
+
+        self.clear_input_field_and_write(email_input_field, new_email)
+        self.components.change_user_email.submit.wait_for_and_click()
+
+        # UX_RENDER time sometimes is not enough
+        self.sleep_for(self.wait_types.UX_TRANSITION)
+
+        # email should be changed
+        assert_email(new_email)
+
+    @selenium_test
+    def test_public_name(self):
+        def get_name_input_field():
+            return self.driver.find_element_by_css_selector("[tour_id='username'] input")
+
+        def assert_public_name(expected_name):
+            self.assertTrue(expected_name == get_name_input_field().get_attribute('value'))
+
+        public_name = "user-public-name"
+        self.register(username=public_name)
+        self.navigate_to_manage_information()
+        self.sleep_for(self.wait_types.UX_RENDER)
+
+        # make sure that public name is the same as registered
+        assert_public_name(public_name)
+        new_public_name = "new-public-name"
+        # change public name
+        self.clear_input_field_and_write(get_name_input_field(), new_public_name)
+        self.components.change_user_email.submit.wait_for_and_click()
+
+        self.sleep_for(self.wait_types.UX_TRANSITION)
+
+        self.navigate_to_manage_information()
+        self.sleep_for(self.wait_types.UX_RENDER)
+        # public name field should render new public name
+        assert_public_name(new_public_name)
+
+    @selenium_test
+    def test_user_address(self):
+        def get_address_form():
+            return self.driver.find_element_by_css_selector("div.ui-portlet-section > div.portlet-content")
+
+        self.register(self._get_random_email())
+        self.navigate_to_manage_information()
+        self.components.change_user_address.address_button.wait_for_and_click()
+
+        address_field_labels = ["Short address description", "Name", "Institution", "Address", "City",
+                                "State/Province/Region", "Postal Code", "Country", "Phone"]
+
+        address_fields = {}
+        # fill address fields with random data
+        for input_field_label in address_field_labels:
+            input_value = self._get_random_name(prefix=input_field_label)
+            address_fields[input_field_label] = input_value
+            input_field = self.get_address_input_field(get_address_form(), input_field_label)
+            self.clear_input_field_and_write(input_field, input_value)
+        # save new address
+        self.components.change_user_email.submit.wait_for_and_click()
+        self.sleep_for(self.wait_types.UX_TRANSITION)
+
+        self.navigate_to_manage_information()
+        self.sleep_for(self.wait_types.UX_RENDER)
+
+        # check if address was saved correctly
+        for input_field_label in address_fields.keys():
+            input_field = self.get_address_input_field(get_address_form(), input_field_label)
+            self.assertTrue(input_field.get_attribute('value') == address_fields[input_field_label])
+
+    def navigate_to_user_preferences(self):
+        self.home()
+        self.click_masthead_user()
+        self.components.masthead.preferences.wait_for_and_click()
+
+    def navigate_to_manage_information(self):
+        self.navigate_to_user_preferences()
+        self.components.preferences.manage_information.wait_for_and_click()
+
+    def clear_input_field_and_write(self, element, new_input_text):
+        element.clear()
+        element.send_keys(new_input_text)
+
+    # Since ids are not permanent we need to search with xpath
+    def get_address_input_field(self, address_form, input_field_label):
+        # . in the beginning = search only within current element
+        # span[.="variable"] find a span where text equals a variable
+        # //ancestor::div[@class = 'ui-form-title']" get an ancestor of found element with 'ui-form-title' classname
+        parent_element = address_form.find_element_by_xpath(
+            ".//span[.='" + input_field_label + "']//ancestor::div[@class = 'ui-form-title']")
+        # we cannot get to the desired parent directly (@class='ui-form-element section-row'),
+        # because then it will match the entire form. Thus, we need to use '..' to get one element up and get input tag
+        return parent_element.find_element_by_xpath('./..').find_element_by_tag_name('input')

From f2d7948d11633eba4ab79f1dd6bba1e419146ef1 Mon Sep 17 00:00:00 2001
From: Oleg Zharkov <oleg.zharkov22@gmail.com>
Date: Mon, 2 Mar 2020 17:24:28 +0100
Subject: [PATCH 40/71] add data-label attribute to address form. Remove parent
 search

---
 client/galaxy/scripts/mvc/form/form-input.js          |  1 +
 lib/galaxy_test/selenium/test_personal_information.py | 10 +---------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/client/galaxy/scripts/mvc/form/form-input.js b/client/galaxy/scripts/mvc/form/form-input.js
index 89ecd872df1..2f1c2274e07 100644
--- a/client/galaxy/scripts/mvc/form/form-input.js
+++ b/client/galaxy/scripts/mvc/form/form-input.js
@@ -177,6 +177,7 @@ export default Backbone.View.extend({
             this.$title_text.show().text(this.model.get("label"));
             this.$collapsible.hide();
         }
+        this.$field.attr("data-label", this.model.get("label"));
     },
 
     _template: function() {
diff --git a/lib/galaxy_test/selenium/test_personal_information.py b/lib/galaxy_test/selenium/test_personal_information.py
index d1aef1b7dee..4de0dd03060 100644
--- a/lib/galaxy_test/selenium/test_personal_information.py
+++ b/lib/galaxy_test/selenium/test_personal_information.py
@@ -107,13 +107,5 @@ class ManageInformationTestCase(SeleniumTestCase):
         element.clear()
         element.send_keys(new_input_text)
 
-    # Since ids are not permanent we need to search with xpath
     def get_address_input_field(self, address_form, input_field_label):
-        # . in the beginning = search only within current element
-        # span[.="variable"] find a span where text equals a variable
-        # //ancestor::div[@class = 'ui-form-title']" get an ancestor of found element with 'ui-form-title' classname
-        parent_element = address_form.find_element_by_xpath(
-            ".//span[.='" + input_field_label + "']//ancestor::div[@class = 'ui-form-title']")
-        # we cannot get to the desired parent directly (@class='ui-form-element section-row'),
-        # because then it will match the entire form. Thus, we need to use '..' to get one element up and get input tag
-        return parent_element.find_element_by_xpath('./..').find_element_by_tag_name('input')
+        return address_form.find_element_by_css_selector("[data-label='" + input_field_label + "'] > input")

From 7a63fd1db202a9993c09cb6f10c0615dc8dc5c99 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Tue, 3 Mar 2020 14:16:01 +0100
Subject: [PATCH 41/71] Fix S3 object store shutdown

---
 lib/galaxy/objectstore/s3.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py
index 992c7fb968b..71d11b7a1cf 100644
--- a/lib/galaxy/objectstore/s3.py
+++ b/lib/galaxy/objectstore/s3.py
@@ -716,6 +716,14 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
     def get_store_usage_percent(self):
         return 0.0
 
+    def shutdown(self):
+        self.running = False
+        thread = getattr(self, 'cache_monitor_thread', None)
+        if thread:
+            log.debug("Shutting down thread")
+            self.sleeper.wake()
+            thread.join(5)
+
 
 class SwiftObjectStore(S3ObjectStore):
     """

From b5dd8346fbe8b8bdd88eaf77566a707b9d5b3c94 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Tue, 3 Mar 2020 14:18:49 +0100
Subject: [PATCH 42/71] Fix test container shutdown

---
 test/integration/objectstore/test_swift_objectstore.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/test/integration/objectstore/test_swift_objectstore.py b/test/integration/objectstore/test_swift_objectstore.py
index a2efd076d54..0ee10a4b462 100644
--- a/test/integration/objectstore/test_swift_objectstore.py
+++ b/test/integration/objectstore/test_swift_objectstore.py
@@ -66,7 +66,7 @@ def start_minio(container_name):
 
 
 def stop_minio(container_name):
-    subprocess.check_call(['docker', 'stop', container_name])
+    subprocess.check_call(['docker', 'rm', '-f', container_name])
 
 
 @integration_util.skip_unless_docker()
@@ -86,7 +86,6 @@ class SwiftObjectStoreIntegrationTestCase(integration_util.IntegrationTestCase):
     @classmethod
     def handle_galaxy_config_kwds(cls, config):
         temp_directory = cls._test_driver.mkdtemp()
-        cls.container_name = os.path.basename(temp_directory)
         cls.object_stores_parent = temp_directory
         config_path = os.path.join(temp_directory, "object_store_conf.xml")
         config["object_store_store_by"] = "uuid"

From 510504075f2c079fe746414b27e2b206dde22652 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Tue, 3 Mar 2020 14:22:54 +0100
Subject: [PATCH 43/71] Don't run extended metadata strategy

---
 test/integration/objectstore/test_swift_objectstore.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/integration/objectstore/test_swift_objectstore.py b/test/integration/objectstore/test_swift_objectstore.py
index 0ee10a4b462..c3e90af1506 100644
--- a/test/integration/objectstore/test_swift_objectstore.py
+++ b/test/integration/objectstore/test_swift_objectstore.py
@@ -89,7 +89,8 @@ class SwiftObjectStoreIntegrationTestCase(integration_util.IntegrationTestCase):
         cls.object_stores_parent = temp_directory
         config_path = os.path.join(temp_directory, "object_store_conf.xml")
         config["object_store_store_by"] = "uuid"
-        config["metadata_strategy"] = "extended"
+        # This doesn't quite work yet, fails with extra_files_path
+        # config["metadata_strategy"] = "extended"
         config["outpus_to_working_dir"] = True
         config["retry_metadata_internally"] = False
         with open(config_path, "w") as f:

From 04db689fd0ffc71e491c4404759b27ba9ba0d4b1 Mon Sep 17 00:00:00 2001
From: Dannon Baker <dannon.baker@gmail.com>
Date: Tue, 3 Mar 2020 08:27:21 -0500
Subject: [PATCH 44/71] Fix default quota creation message.

---
 lib/galaxy/actions/admin.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/actions/admin.py b/lib/galaxy/actions/admin.py
index 3879f6b794e..d2acc040db9 100644
--- a/lib/galaxy/actions/admin.py
+++ b/lib/galaxy/actions/admin.py
@@ -45,7 +45,7 @@ class AdminActions(object):
             # If this is a default quota, create the DefaultQuotaAssociation
             if params.default != 'no':
                 self.app.quota_agent.set_default_quota(params.default, quota)
-                message = "Default quota '%s' has been created."
+                message = "Default quota '%s' has been created." % quota.name
             else:
                 # Create the UserQuotaAssociations
                 in_users = [self.sa_session.query(self.app.model.User).get(decode_id(x) if decode_id else x) for x in util.listify(params.in_users)]

From 46f3af60450098a5a7846edb2f1fec31873cc7d4 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Tue, 3 Mar 2020 15:20:17 +0100
Subject: [PATCH 45/71] Use single underscore bucket

---
 lib/galaxy/objectstore/s3.py | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py
index 71d11b7a1cf..30d5f1100e1 100644
--- a/lib/galaxy/objectstore/s3.py
+++ b/lib/galaxy/objectstore/s3.py
@@ -189,7 +189,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
                          'conn_path': self.conn_path}
 
         self._configure_connection()
-        self.__bucket = self._get_bucket(self.bucket)
+        self._bucket = self._get_bucket(self.bucket)
         # Clean cache only if value is set in galaxy.ini
         if self.cache_size != -1:
             # Convert GBs to bytes for comparison
@@ -356,7 +356,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
 
     def _get_size_in_s3(self, rel_path):
         try:
-            key = self.__bucket.get_key(rel_path)
+            key = self._bucket.get_key(rel_path)
             if key:
                 return key.size
         except S3ResponseError:
@@ -369,13 +369,13 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
             # A hackish way of testing if the rel_path is a folder vs a file
             is_dir = rel_path[-1] == '/'
             if is_dir:
-                keyresult = self.__bucket.get_all_keys(prefix=rel_path)
+                keyresult = self._bucket.get_all_keys(prefix=rel_path)
                 if len(keyresult) > 0:
                     exists = True
                 else:
                     exists = False
             else:
-                key = Key(self.__bucket, rel_path)
+                key = Key(self._bucket, rel_path)
                 exists = key.exists()
         except S3ResponseError:
             log.exception("Trouble checking existence of S3 key '%s'", rel_path)
@@ -427,7 +427,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
     def _download(self, rel_path):
         try:
             log.debug("Pulling key '%s' into cache to %s", rel_path, self._get_cache_path(rel_path))
-            key = self.__bucket.get_key(rel_path)
+            key = self._bucket.get_key(rel_path)
             # Test if cache is large enough to hold the new file
             if self.cache_size > 0 and key.size > self.cache_size:
                 log.critical("File %s is larger (%s) than the cache size (%s). Cannot download.",
@@ -446,7 +446,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
                 key.get_contents_to_filename(self._get_cache_path(rel_path), cb=self._transfer_cb, num_cb=10)
                 return True
         except S3ResponseError:
-            log.exception("Problem downloading key '%s' from S3 bucket '%s'", rel_path, self.__bucket.name)
+            log.exception("Problem downloading key '%s' from S3 bucket '%s'", rel_path, self._bucket.name)
         return False
 
     def _push_to_os(self, rel_path, source_file=None, from_string=None):
@@ -460,7 +460,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
         try:
             source_file = source_file if source_file else self._get_cache_path(rel_path)
             if os.path.exists(source_file):
-                key = Key(self.__bucket, rel_path)
+                key = Key(self._bucket, rel_path)
                 if os.path.getsize(source_file) == 0 and key.exists():
                     log.debug("Wanted to push file '%s' to S3 key '%s' but its size is 0; skipping.", source_file, rel_path)
                     return True
@@ -478,7 +478,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
                                                        cb=self._transfer_cb,
                                                        num_cb=10)
                     else:
-                        multipart_upload(self.s3server, self.__bucket, key.name, source_file, mb_size)
+                        multipart_upload(self.s3server, self._bucket, key.name, source_file, mb_size)
                     end_time = datetime.now()
                     log.debug("Pushed cache file '%s' to key '%s' (%s bytes transfered in %s sec)",
                               source_file, rel_path, os.path.getsize(source_file), end_time - start_time)
@@ -609,7 +609,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
             # but requires iterating through each individual key in S3 and deleing it.
             if entire_dir and extra_dir:
                 shutil.rmtree(self._get_cache_path(rel_path))
-                results = self.__bucket.get_all_keys(prefix=rel_path)
+                results = self._bucket.get_all_keys(prefix=rel_path)
                 for key in results:
                     log.debug("Deleting key %s", key.name)
                     key.delete()
@@ -619,7 +619,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
                 os.unlink(self._get_cache_path(rel_path))
                 # Delete from S3 as well
                 if self._key_exists(rel_path):
-                    key = Key(self.__bucket, rel_path)
+                    key = Key(self._bucket, rel_path)
                     log.debug("Deleting key %s", key.name)
                     key.delete()
                     return True
@@ -707,7 +707,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
         if self.exists(obj, **kwargs):
             rel_path = self._construct_path(obj, **kwargs)
             try:
-                key = Key(self.__bucket, rel_path)
+                key = Key(self._bucket, rel_path)
                 return key.generate_url(expires_in=86400)  # 24hrs
             except S3ResponseError:
                 log.exception("Trouble generating URL for dataset '%s'", rel_path)

From f54b0e729a5fa8d223df03f824e767ca413b0127 Mon Sep 17 00:00:00 2001
From: Dannon Baker <dannon.baker@gmail.com>
Date: Tue, 3 Mar 2020 10:02:19 -0500
Subject: [PATCH 46/71] Fix data manager table slot usage; this makes the
 header with the name and 'reload' icon display again.  Also refactor a few
 computed props. Should add this to eslint to catch these easier.

---
 .../admin/DataManager/DataManagerTable.vue    | 44 +++++++++++--------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue b/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue
index fa1139df378..f749b464514 100644
--- a/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue
+++ b/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue
@@ -13,22 +13,24 @@
             <b-row>
                 <b-col>
                     <b-card id="data-table-card" flush>
-                        <b-container v-slot:header>
-                            <b-row align-v="center">
-                                <b-col cols="auto">
-                                    <b-button
-                                        @click="reload()"
-                                        v-b-tooltip.hover
-                                        :title="'Reload ' + this.dataTable['name'] + ' tool data table'"
-                                    >
-                                        <span class="fa fa-refresh" />
-                                    </b-button>
-                                </b-col>
-                                <b-col>
-                                    <b>{{ this.dataTable["name"] }}</b>
-                                </b-col>
-                            </b-row>
-                        </b-container>
+                        <template v-slot:header>
+                            <b-container>
+                                <b-row align-v="center">
+                                    <b-col cols="auto">
+                                        <b-button
+                                            @click="reload()"
+                                            v-b-tooltip.hover
+                                            :title="buttonLabel"
+                                        >
+                                            <span class="fa fa-refresh" />
+                                        </b-button>
+                                    </b-col>
+                                    <b-col>
+                                        <b>{{ dataTableName }}</b>
+                                    </b-col>
+                                </b-row>
+                            </b-container>
+                        </template>
                         <b-table
                             :fields="fields(this.dataTable['columns'])"
                             :items="dataTable['data']"
@@ -68,6 +70,12 @@ export default {
         };
     },
     computed: {
+        dataTableName() {
+            return this.dataTable && this.dataTable.name ? this.dataTable.name : 'null';
+        },
+        buttonLabel() {
+            return `Reload ${this.dataTableName} tool data table`;
+        },
         breadcrumbItems() {
             return [
                 {
@@ -75,7 +83,7 @@ export default {
                     to: "/"
                 },
                 {
-                    text: this.dataTable["name"]
+                    text: this.dataTableName
                 }
             ];
         }
@@ -88,7 +96,7 @@ export default {
         },
         reload() {
             axios
-                .get(`${getAppRoot()}data_manager/reload_tool_data_tables?table_name=${this.dataTable["name"]}`)
+                .get(`${getAppRoot()}data_manager/reload_tool_data_tables?table_name=${this.dataTableName}`)
                 .then(response => {
                     if (response.data.dataTable) {
                         this.dataTable = response.data.dataTable;

From ac9249eb4e4e0f6a5d67b870fa62eccedbe2c7f3 Mon Sep 17 00:00:00 2001
From: Dannon Baker <dannon.baker@gmail.com>
Date: Tue, 3 Mar 2020 10:04:46 -0500
Subject: [PATCH 47/71] Prettier.

---
 .../components/admin/DataManager/DataManagerTable.vue     | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue b/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue
index f749b464514..972612a8116 100644
--- a/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue
+++ b/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue
@@ -17,11 +17,7 @@
                             <b-container>
                                 <b-row align-v="center">
                                     <b-col cols="auto">
-                                        <b-button
-                                            @click="reload()"
-                                            v-b-tooltip.hover
-                                            :title="buttonLabel"
-                                        >
+                                        <b-button @click="reload()" v-b-tooltip.hover :title="buttonLabel">
                                             <span class="fa fa-refresh" />
                                         </b-button>
                                     </b-col>
@@ -71,7 +67,7 @@ export default {
     },
     computed: {
         dataTableName() {
-            return this.dataTable && this.dataTable.name ? this.dataTable.name : 'null';
+            return this.dataTable && this.dataTable.name ? this.dataTable.name : "null";
         },
         buttonLabel() {
             return `Reload ${this.dataTableName} tool data table`;

From 09bfa57ab93657a08ed2035e2f8b6f0c5f334444 Mon Sep 17 00:00:00 2001
From: Helena Rasche <hxr@hx42.org>
Date: Tue, 3 Mar 2020 17:26:02 +0100
Subject: [PATCH 48/71] rebuild

---
 doc/source/admin/galaxy_options.rst         | 15 +++++++++++++--
 lib/galaxy/config/sample/galaxy.yml.sample  |  6 +++++-
 lib/galaxy/webapps/galaxy/config_schema.yml | 11 +++++++++--
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/doc/source/admin/galaxy_options.rst b/doc/source/admin/galaxy_options.rst
index ac2f2193d4b..10beabbb6ff 100644
--- a/doc/source/admin/galaxy_options.rst
+++ b/doc/source/admin/galaxy_options.rst
@@ -3271,6 +3271,17 @@
 :Type: bool
 
 
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+``job_metrics_config_file``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+:Description:
+    XML config file that contains the job metric collection
+    configuration.
+:Default: ``config/job_metrics_conf.xml``
+:Type: str
+
+
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 ``expose_potentially_sensitive_job_metrics``
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -3673,7 +3684,7 @@
 
 :Description:
     CSS file to apply to "Galaxy Page" exports to PDF. Generally
-    prefer  markdown_export_css, but this is here for deployments that
+    prefer markdown_export_css, but this is here for deployments that
     would like to tailor different kinds of exports.
 :Default: ``markdown_export_pages.css``
 :Type: str
@@ -3685,7 +3696,7 @@
 
 :Description:
     CSS file to apply to invocation report exports to PDF. Generally
-    prefer  markdown_export_css, but this is here for deployments that
+    prefer markdown_export_css, but this is here for deployments that
     would like to tailor different kinds of exports.
 :Default: ``markdown_export_invocation_reports.css``
 :Type: str
diff --git a/lib/galaxy/config/sample/galaxy.yml.sample b/lib/galaxy/config/sample/galaxy.yml.sample
index d8368867158..19721ae13b9 100644
--- a/lib/galaxy/config/sample/galaxy.yml.sample
+++ b/lib/galaxy/config/sample/galaxy.yml.sample
@@ -1611,6 +1611,10 @@ galaxy:
   # see dataset paths.
   #expose_dataset_path: false
 
+  # XML config file that contains the job metric collection
+  # configuration.
+  #job_metrics_config_file: config/job_metrics_conf.xml
+
   # This option allows users to see the job metrics (except for
   # environment variables).
   #expose_potentially_sensitive_job_metrics: false
@@ -1815,7 +1819,7 @@ galaxy:
   #markdown_export_css_pages: markdown_export_pages.css
 
   # CSS file to apply to invocation report exports to PDF. Generally
-  # prefer  markdown_export_css, but this is here for deployments that
+  # prefer markdown_export_css, but this is here for deployments that
   # would like to tailor different kinds of exports.
   #markdown_export_css_invocation_reports: markdown_export_invocation_reports.css
 
diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml
index 692a0e4a3b4..72db2811e08 100644
--- a/lib/galaxy/webapps/galaxy/config_schema.yml
+++ b/lib/galaxy/webapps/galaxy/config_schema.yml
@@ -2417,6 +2417,13 @@ mapping:
           Details" option in the history. This option also exposes the command line to
           non-administrative users. Administrators can always see dataset paths.
 
+      job_metrics_config_file:
+        type: str
+        default: config/job_metrics_conf.xml
+        required: false
+        desc: |
+          XML config file that contains the job metric collection configuration.
+
       expose_potentially_sensitive_job_metrics:
         type: bool
         default: false
@@ -2728,7 +2735,7 @@ mapping:
         default: markdown_export_pages.css
         required: false
         desc: |
-          CSS file to apply to "Galaxy Page" exports to PDF. Generally prefer 
+          CSS file to apply to "Galaxy Page" exports to PDF. Generally prefer
           markdown_export_css, but this is here for deployments that
           would like to tailor different kinds of exports.
 
@@ -2737,7 +2744,7 @@ mapping:
         default: markdown_export_invocation_reports.css
         required: false
         desc: |
-          CSS file to apply to invocation report exports to PDF. Generally prefer 
+          CSS file to apply to invocation report exports to PDF. Generally prefer
           markdown_export_css, but this is here for deployments that
           would like to tailor different kinds of exports.
 

From 494c15cb9d622c3c75bbcf72b880ee386cbe01f5 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Tue, 3 Mar 2020 18:31:06 +0100
Subject: [PATCH 49/71] Be more explcit about starting cache monitor thread

---
 lib/galaxy/objectstore/s3.py | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py
index 30d5f1100e1..c6049441a03 100644
--- a/lib/galaxy/objectstore/s3.py
+++ b/lib/galaxy/objectstore/s3.py
@@ -123,9 +123,10 @@ class CloudConfigMixin(object):
                 'conn_path': self.conn_path,
             },
             'cache': {
-                'size': -1,  # disable cache cleaning when starting an exported object store in set_metadata.py
+                'size': self.cache_size,
                 'path': self.staging_path,
-            }
+            },
+            'enable_cache_monitor': False,
         }
 
 
@@ -146,6 +147,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
         bucket_dict = config_dict['bucket']
         connection_dict = config_dict.get('connection', {})
         cache_dict = config_dict['cache']
+        self.enable_cache_monitor = config_dict.get('enable_cache_monitor', True)
 
         self.access_key = auth_dict.get('access_key')
         self.secret_key = auth_dict.get('secret_key')
@@ -190,8 +192,16 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
 
         self._configure_connection()
         self._bucket = self._get_bucket(self.bucket)
+        self.start_cache_monitor()
+        # Test if 'axel' is available for parallel download and pull the key into cache
+        if which('axel'):
+            self.use_axel = True
+        else:
+            self.use_axel = False
+
+    def start_cache_monitor(self):
         # Clean cache only if value is set in galaxy.ini
-        if self.cache_size != -1:
+        if self.cache_size != -1 and self.enable_cache_monitor:
             # Convert GBs to bytes for comparison
             self.cache_size = self.cache_size * 1073741824
             # Helper for interruptable sleep
@@ -199,11 +209,6 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
             self.cache_monitor_thread = threading.Thread(target=self.__cache_monitor)
             self.cache_monitor_thread.start()
             log.info("Cache cleaner manager started")
-        # Test if 'axel' is available for parallel download and pull the key into cache
-        if which('axel'):
-            self.use_axel = True
-        else:
-            self.use_axel = False
 
     def _configure_connection(self):
         log.debug("Configuring S3 Connection")

From 6d2e2ef10bde3812dfe292ae1074b08fd501033d Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Tue, 3 Mar 2020 18:31:58 +0100
Subject: [PATCH 50/71] Drop redundant store_by attribute

---
 lib/galaxy/objectstore/s3.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py
index c6049441a03..9c53c0aed29 100644
--- a/lib/galaxy/objectstore/s3.py
+++ b/lib/galaxy/objectstore/s3.py
@@ -164,8 +164,6 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
 
         self.cache_size = cache_dict.get('size', -1)
         self.staging_path = cache_dict.get('path') or self.config.object_store_cache_path
-        self.store_by = config_dict.get("store_by", None) or getattr(config, "object_store_store_by", "id")
-        assert self.store_by in ["id", "uuid"]
 
         extra_dirs = dict(
             (e['type'], e['path']) for e in config_dict.get('extra_dirs', []))

From 3623f226f1474cb233e74bf6ae7e42a71012d300 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Tue, 3 Mar 2020 18:32:10 +0100
Subject: [PATCH 51/71] Drop badly formatted logging

---
 lib/galaxy/objectstore/s3.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py
index 9c53c0aed29..327988be7be 100644
--- a/lib/galaxy/objectstore/s3.py
+++ b/lib/galaxy/objectstore/s3.py
@@ -169,9 +169,6 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin):
             (e['type'], e['path']) for e in config_dict.get('extra_dirs', []))
         self.extra_dirs.update(extra_dirs)
 
-        log.debug("Object cache dir:    %s", self.staging_path)
-        log.debug("       job work dir: %s", self.extra_dirs['job_work'])
-
         self._initialize()
 
     def _initialize(self):

From 12f8c78e344a948be6bb1b52bd56add610e7a8b9 Mon Sep 17 00:00:00 2001
From: Ralf Weber <RJMW@users.noreply.github.com>
Date: Tue, 3 Mar 2020 21:09:16 +0000
Subject: [PATCH 52/71] Add rdata.xcms.raw for older versions of XCMS

---
 lib/galaxy/config/sample/datatypes_conf.xml.sample | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/galaxy/config/sample/datatypes_conf.xml.sample b/lib/galaxy/config/sample/datatypes_conf.xml.sample
index 8a92bd8e970..43a5f031d74 100644
--- a/lib/galaxy/config/sample/datatypes_conf.xml.sample
+++ b/lib/galaxy/config/sample/datatypes_conf.xml.sample
@@ -563,6 +563,7 @@
     <datatype extension="gal" type="galaxy.datatypes.microarrays:Gal" display_in_upload="true"/>
     <datatype extension="rdata" type="galaxy.datatypes.binary:RData" display_in_upload="true" description="Stored data from an R session"/>
     <datatype extension="rdata.sce" type="galaxy.datatypes.binary:RData" description="Stored RDS from a SingleCellObject" subclass="true" display_in_upload="true"/>
+    <datatype extension="rdata.xcms.raw" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
     <datatype extension="rdata.msnbase.raw" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
     <datatype extension="rdata.xcms.findchrompeaks" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
     <datatype extension="rdata.xcms.group" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />

From 33c963487b7e0d320081773ac6f8dcfe22ac8721 Mon Sep 17 00:00:00 2001
From: Ralf Weber <RJMW@users.noreply.github.com>
Date: Tue, 3 Mar 2020 21:29:15 +0000
Subject: [PATCH 53/71] Add missing mimetype and remove spaces

---
 .../config/sample/datatypes_conf.xml.sample   | 22 +++++++++----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/galaxy/config/sample/datatypes_conf.xml.sample b/lib/galaxy/config/sample/datatypes_conf.xml.sample
index 43a5f031d74..433b799404b 100644
--- a/lib/galaxy/config/sample/datatypes_conf.xml.sample
+++ b/lib/galaxy/config/sample/datatypes_conf.xml.sample
@@ -561,17 +561,17 @@
     <datatype extension="cel" type="galaxy.datatypes.microarrays:Cel" display_in_upload="true"/>
     <datatype extension="gpr" type="galaxy.datatypes.microarrays:Gpr" display_in_upload="true"/>
     <datatype extension="gal" type="galaxy.datatypes.microarrays:Gal" display_in_upload="true"/>
-    <datatype extension="rdata" type="galaxy.datatypes.binary:RData" display_in_upload="true" description="Stored data from an R session"/>
-    <datatype extension="rdata.sce" type="galaxy.datatypes.binary:RData" description="Stored RDS from a SingleCellObject" subclass="true" display_in_upload="true"/>
-    <datatype extension="rdata.xcms.raw" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
-    <datatype extension="rdata.msnbase.raw" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
-    <datatype extension="rdata.xcms.findchrompeaks" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
-    <datatype extension="rdata.xcms.group" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
-    <datatype extension="rdata.xcms.retcor" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
-    <datatype extension="rdata.xcms.fillpeaks" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
-    <datatype extension="rdata.camera.positive" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
-    <datatype extension="rdata.camera.negative" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
-    <datatype extension="rdata.camera.quick" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" />
+    <datatype extension="rdata" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" display_in_upload="true" description="Stored data from an R session"/>
+    <datatype extension="rdata.sce" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true" description="Stored RDS from a SingleCellObject"/>
+    <datatype extension="rdata.xcms.raw" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true"/>
+    <datatype extension="rdata.msnbase.raw" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true"/>
+    <datatype extension="rdata.xcms.findchrompeaks" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true"/>
+    <datatype extension="rdata.xcms.group" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true"/>
+    <datatype extension="rdata.xcms.retcor" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true"/>
+    <datatype extension="rdata.xcms.fillpeaks" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true"/>
+    <datatype extension="rdata.camera.positive" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true"/>
+    <datatype extension="rdata.camera.negative" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true"/>
+    <datatype extension="rdata.camera.quick" type="galaxy.datatypes.binary:RData" mimetype="application/x-gzip" subclass="true" display_in_upload="true"/>
     <datatype extension="rdock_as" type="galaxy.datatypes.binary:Binary" description="rDock active site format" subclass="true" display_in_upload="true"/>
     <datatype extension="oxlicg" type="galaxy.datatypes.binary:OxliCountGraph" mimetype="application/octet-stream" display_in_upload="true"/>
     <datatype extension="oxling" type="galaxy.datatypes.binary:OxliNodeGraph" mimetype="application/octet-stream" display_in_upload="true"/>

From ae5e3ea54d10891de362601ac223fa25fb76bd8d Mon Sep 17 00:00:00 2001
From: Helena Rasche <hxr@hx42.org>
Date: Wed, 4 Mar 2020 08:57:42 +0100
Subject: [PATCH 54/71] Update lib/galaxy/webapps/galaxy/config_schema.yml

Co-Authored-By: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
---
 lib/galaxy/webapps/galaxy/config_schema.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml
index 72db2811e08..bd62b5d1072 100644
--- a/lib/galaxy/webapps/galaxy/config_schema.yml
+++ b/lib/galaxy/webapps/galaxy/config_schema.yml
@@ -2419,7 +2419,7 @@ mapping:
 
       job_metrics_config_file:
         type: str
-        default: config/job_metrics_conf.xml
+        default: job_metrics_conf.xml
         required: false
         desc: |
           XML config file that contains the job metric collection configuration.

From 787620dc24fafa4e6b480466f444effa4223131b Mon Sep 17 00:00:00 2001
From: Helena Rasche <hxr@hx42.org>
Date: Wed, 4 Mar 2020 08:57:50 +0100
Subject: [PATCH 55/71] Update lib/galaxy/webapps/galaxy/config_schema.yml

Co-Authored-By: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
---
 lib/galaxy/webapps/galaxy/config_schema.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml
index bd62b5d1072..dc974f0ae1d 100644
--- a/lib/galaxy/webapps/galaxy/config_schema.yml
+++ b/lib/galaxy/webapps/galaxy/config_schema.yml
@@ -2423,6 +2423,8 @@ mapping:
         required: false
         desc: |
           XML config file that contains the job metric collection configuration.
+          
+          The value of this option will be resolved with respect to <config_dir>.
 
       expose_potentially_sensitive_job_metrics:
         type: bool

From 89b8e62f1f5009c047f39d0a091148db4232f525 Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Wed, 4 Mar 2020 09:23:39 -0500
Subject: [PATCH 56/71] Do not test defaults for job_metrics_config_file

---
 test/integration/test_config_defaults.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/integration/test_config_defaults.py b/test/integration/test_config_defaults.py
index c8d2ff5f26e..be1f15ea947 100644
--- a/test/integration/test_config_defaults.py
+++ b/test/integration/test_config_defaults.py
@@ -178,6 +178,7 @@ DO_NOT_TEST = [
     'id_secret',  # broken: default overridden
     'job_config',  # no obvious testable defaults
     'job_config_file',  # broken: remove 'config/' prefix from schema
+    'job_metrics_config_file',
     'job_working_directory',  # broken; may or may not be able to test
     'library_import_dir',  # broken: default overridden
     'logging',  # mapping loaded in config/

From 39ac5dbd7a439bd62f2ff7fec65a1db5c5f05089 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Thu, 5 Mar 2020 12:36:10 +0100
Subject: [PATCH 57/71] Skip migration if uninitialzed database exists

As we do when Galaxy created the database. This is useful if the
database was pre-created because the Galaxy user doesn't have
permissions to create new databases.
---
 lib/galaxy/model/migrate/check.py | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/lib/galaxy/model/migrate/check.py b/lib/galaxy/model/migrate/check.py
index 0feaaefa789..a5c9388dd54 100644
--- a/lib/galaxy/model/migrate/check.py
+++ b/lib/galaxy/model/migrate/check.py
@@ -61,8 +61,7 @@ def create_or_verify_database(url, galaxy_config_file, engine_options={}, app=No
         # Apply all scripts to get to current version
         migrate_to_current_version(engine, db_schema)
 
-    meta = MetaData(bind=engine)
-    if new_database:
+    def migrate_from_scratch():
         log.info("Creating new database from scratch, skipping migrations")
         current_version = migrate_repository.version().version
         mapping.init(file_path='/tmp', url=url, map_install_models=map_install_models, create_tables=True)
@@ -70,6 +69,13 @@ def create_or_verify_database(url, galaxy_config_file, engine_options={}, app=No
         db_schema = schema.ControlledSchema(engine, migrate_repository)
         assert db_schema.version == current_version
         migrate()
+        if app:
+            # skips the tool migration process.
+            app.new_installation = True
+
+    meta = MetaData(bind=engine)
+    if new_database:
+        migrate_from_scratch()
         return
     elif app and getattr(app.config, 'database_auto_migrate', False):
         migrate()
@@ -79,12 +85,9 @@ def create_or_verify_database(url, galaxy_config_file, engine_options={}, app=No
     try:
         Table("dataset", meta, autoload=True)
     except NoSuchTableError:
-        # No 'dataset' table means a completely uninitialized database.  If we have an app, we'll
-        # set its new_installation setting to True so the tool migration process will be skipped.
-        if app:
-            app.new_installation = True
+        # No 'dataset' table means a completely uninitialized database.
         log.info("No database, initializing")
-        migrate()
+        migrate_from_scratch()
         return
     try:
         hda_table = Table("history_dataset_association", meta, autoload=True)

From fde103db24063ac01cc12d7f24c85950c65ca0a2 Mon Sep 17 00:00:00 2001
From: MiguelJulia <migueljuliamolina@gmail.com>
Date: Fri, 6 Mar 2020 16:07:37 +0100
Subject: [PATCH 58/71] Added whitelisting option for user registration

---
 lib/galaxy/config/__init__.py              | 10 ++++++++++
 lib/galaxy/security/validate_user_input.py |  7 +++++++
 lib/tool_shed/webapp/config_schema.yml     | 10 ++++++++++
 3 files changed, 27 insertions(+)

diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py
index 3fdc7f9af3a..021e7a7d710 100644
--- a/lib/galaxy/config/__init__.py
+++ b/lib/galaxy/config/__init__.py
@@ -465,6 +465,16 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
             except IOError:
                 log.error("CONFIGURATION ERROR: Can't open supplied blacklist file from path: %s", self.blacklist_file)
 
+        #  Create whitelist file to accept only certain email domains
+        self.whitelist_content = None
+        if self.whitelist_file:
+            self.whitelist_file = os.path.join(self.root, self.whitelist_file)
+            try:
+                with open(self.whitelist_file) as f:
+                    self.whitelist_content = [line.rstrip() for line in f]
+            except IOError:
+                log.error("CONFIGURATION ERROR: Can't open supplied whitelist file from path: %s", self.whitelist_file)
+
         self.persistent_communication_rooms = listify(self.persistent_communication_rooms, do_strip=True)
         # The transfer manager and deferred job queue
         self.enable_beta_job_managers = string_as_bool(kwargs.get('enable_beta_job_managers', 'False'))
diff --git a/lib/galaxy/security/validate_user_input.py b/lib/galaxy/security/validate_user_input.py
index ae5dc840910..12bba4c06f5 100644
--- a/lib/galaxy/security/validate_user_input.py
+++ b/lib/galaxy/security/validate_user_input.py
@@ -46,6 +46,13 @@ def validate_email(trans, email, user=None, check_dup=True, allow_empty=False):
             domain = ('.').join(domain.split('.')[-2:])
         if domain in trans.app.config.blacklist_content:
             message = "Please enter your permanent email address."
+    #  If the whitelist is not empty filter out any domain not in the list.
+    elif trans.app.config.whitelist_content is not None:
+        domain = email.split('@')[1]
+        if len(domain.split('.')) > 2:
+            domain = ('.').join(domain.split('.')[-2:])
+        if domain not in trans.app.config.whitelist_content:
+            message = "Please enter an allowed domain email address for this server."
     return message
 
 
diff --git a/lib/tool_shed/webapp/config_schema.yml b/lib/tool_shed/webapp/config_schema.yml
index cc014e2c630..3a6b836292e 100644
--- a/lib/tool_shed/webapp/config_schema.yml
+++ b/lib/tool_shed/webapp/config_schema.yml
@@ -482,6 +482,16 @@ mapping:
           disposable email address during the registration.  If their address domain
           matches any domain in the blacklist, they are refused the registration.
 
+      whitelist_file:
+        type: str
+        default: config/disposable_email_whitelist.conf
+        required: false
+        desc: |
+          E-mail domains whitelist is used for filtering out users that are using
+          email address not belonging to the instition/company domains during the
+          registration. If their address domain matches any domain in the whitelist,
+          they are refused the registration.
+
       brand:
         type: str
         default: null

From 7538b3eb5c7d6deb532ae1f00bf2bb323b093251 Mon Sep 17 00:00:00 2001
From: MiguelJulia <migueljuliamolina@gmail.com>
Date: Fri, 6 Mar 2020 16:37:26 +0100
Subject: [PATCH 59/71] Move to correct configuration

---
 lib/galaxy/webapps/galaxy/config_schema.yml | 12 +++++++++++-
 lib/tool_shed/webapp/config_schema.yml      | 10 ----------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml
index a312531459f..1a8a688d660 100644
--- a/lib/galaxy/webapps/galaxy/config_schema.yml
+++ b/lib/galaxy/webapps/galaxy/config_schema.yml
@@ -902,6 +902,16 @@ mapping:
 
           Example value 'config/disposable_email_blacklist.conf'
 
+      whitelist_file:
+        type: str
+        default: config/disposable_email_whitelist.conf
+        required: false
+        desc: |
+          E-mail domains whitelist is used for filtering out users that are using
+          email address not belonging to the instition/company domains during the
+          registration. If their address domain matches any domain in the whitelist,
+          they are refused the registration.
+
       registration_warning_message:
         type: str
         default: >-
@@ -2414,7 +2424,7 @@ mapping:
         required: false
         desc: |
           XML config file that contains the job metric collection configuration.
-          
+
           The value of this option will be resolved with respect to <config_dir>.
 
       expose_potentially_sensitive_job_metrics:
diff --git a/lib/tool_shed/webapp/config_schema.yml b/lib/tool_shed/webapp/config_schema.yml
index 3a6b836292e..cc014e2c630 100644
--- a/lib/tool_shed/webapp/config_schema.yml
+++ b/lib/tool_shed/webapp/config_schema.yml
@@ -482,16 +482,6 @@ mapping:
           disposable email address during the registration.  If their address domain
           matches any domain in the blacklist, they are refused the registration.
 
-      whitelist_file:
-        type: str
-        default: config/disposable_email_whitelist.conf
-        required: false
-        desc: |
-          E-mail domains whitelist is used for filtering out users that are using
-          email address not belonging to the instition/company domains during the
-          registration. If their address domain matches any domain in the whitelist,
-          they are refused the registration.
-
       brand:
         type: str
         default: null

From 75d26ab896cde9b92077e1994ac33530e60879d5 Mon Sep 17 00:00:00 2001
From: MiguelJulia <migueljuliamolina@gmail.com>
Date: Fri, 6 Mar 2020 16:39:26 +0100
Subject: [PATCH 60/71] Explanation about whitelist/blacklist priority

---
 lib/galaxy/webapps/galaxy/config_schema.yml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml
index 1a8a688d660..a0e5f760d4b 100644
--- a/lib/galaxy/webapps/galaxy/config_schema.yml
+++ b/lib/galaxy/webapps/galaxy/config_schema.yml
@@ -910,7 +910,9 @@ mapping:
           E-mail domains whitelist is used for filtering out users that are using
           email address not belonging to the instition/company domains during the
           registration. If their address domain matches any domain in the whitelist,
-          they are refused the registration.
+          they are refused the registration. This is a more restrictive option than
+          blacklist, and therefore, in case the previous one is defined this one
+          will be ignored.
 
       registration_warning_message:
         type: str

From e7f086c33431e463f02c2e3089fc7e2b23553e34 Mon Sep 17 00:00:00 2001
From: MiguelJulia <migueljuliamolina@gmail.com>
Date: Fri, 6 Mar 2020 16:45:35 +0100
Subject: [PATCH 61/71] Added documentation generated with make

---
 doc/source/admin/galaxy_options.rst        | 19 ++++++++++++++++++-
 lib/galaxy/config/sample/galaxy.yml.sample | 12 +++++++++++-
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/doc/source/admin/galaxy_options.rst b/doc/source/admin/galaxy_options.rst
index 1c78cc612b5..3f8728d28f1 100644
--- a/doc/source/admin/galaxy_options.rst
+++ b/doc/source/admin/galaxy_options.rst
@@ -1221,6 +1221,21 @@
 :Type: str
 
 
+~~~~~~~~~~~~~~~~~~
+``whitelist_file``
+~~~~~~~~~~~~~~~~~~
+
+:Description:
+    E-mail domains whitelist is used for filtering out users that are
+    using email address not belonging to the instition/company domains
+    during the registration. If their address domain matches any
+    domain in the whitelist, they are refused the registration. This
+    is a more restrictive option than blacklist, and therefore, in
+    case the previous one is defined this one will be ignored.
+:Default: ``config/disposable_email_whitelist.conf``
+:Type: str
+
+
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 ``registration_warning_message``
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -3265,7 +3280,9 @@
 :Description:
     XML config file that contains the job metric collection
     configuration.
-:Default: ``config/job_metrics_conf.xml``
+    The value of this option will be resolved with respect to
+    <config_dir>.
+:Default: ``job_metrics_conf.xml``
 :Type: str
 
 
diff --git a/lib/galaxy/config/sample/galaxy.yml.sample b/lib/galaxy/config/sample/galaxy.yml.sample
index dea2580aeb7..9054cf7f2d7 100644
--- a/lib/galaxy/config/sample/galaxy.yml.sample
+++ b/lib/galaxy/config/sample/galaxy.yml.sample
@@ -687,6 +687,14 @@ galaxy:
   # Example value 'config/disposable_email_blacklist.conf'
   #blacklist_file: null
 
+  # E-mail domains whitelist is used for filtering out users that are
+  # using email address not belonging to the instition/company domains
+  # during the registration. If their address domain matches any domain
+  # in the whitelist, they are refused the registration. This is a more
+  # restrictive option than blacklist, and therefore, in case the
+  # previous one is defined this one will be ignored.
+  #whitelist_file: config/disposable_email_whitelist.conf
+
   # Registration warning message is used to discourage people from
   # registering multiple accounts.  Applies mostly for the main Galaxy
   # instance. If no message specified the warning box will not be shown.
@@ -1607,7 +1615,9 @@ galaxy:
 
   # XML config file that contains the job metric collection
   # configuration.
-  #job_metrics_config_file: config/job_metrics_conf.xml
+  # The value of this option will be resolved with respect to
+  # <config_dir>.
+  #job_metrics_config_file: job_metrics_conf.xml
 
   # This option allows users to see the job metrics (except for
   # environment variables).

From a16bd19f685f481b06661dcaa7364587fd393630 Mon Sep 17 00:00:00 2001
From: MiguelJulia <migueljuliamolina@gmail.com>
Date: Fri, 6 Mar 2020 18:18:09 +0100
Subject: [PATCH 62/71] Reworded documentation as suggested by ic4f

---
 lib/galaxy/webapps/galaxy/config_schema.yml | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml
index a0e5f760d4b..375fdf5e275 100644
--- a/lib/galaxy/webapps/galaxy/config_schema.yml
+++ b/lib/galaxy/webapps/galaxy/config_schema.yml
@@ -907,12 +907,11 @@ mapping:
         default: config/disposable_email_whitelist.conf
         required: false
         desc: |
-          E-mail domains whitelist is used for filtering out users that are using
-          email address not belonging to the instition/company domains during the
-          registration. If their address domain matches any domain in the whitelist,
-          they are refused the registration. This is a more restrictive option than
-          blacklist, and therefore, in case the previous one is defined this one
-          will be ignored.
+          E-mail domains whitelist is used to specify allowed email address domains.
+          If the list is non-empty and a user attempts registration using an email
+          address belonging to a domain that is not on the list, registration will be
+          enied. This is a more restrictive option than <blacklist_file>, and therefore,
+          in case <blacklist_file> is defined, <whitelist_file> will be ignored.
 
       registration_warning_message:
         type: str

From 2eeb542b79653f465370a541614240959252080c Mon Sep 17 00:00:00 2001
From: MiguelJulia <migueljuliamolina@gmail.com>
Date: Fri, 6 Mar 2020 18:31:21 +0100
Subject: [PATCH 63/71] removed hardcoded path

---
 lib/galaxy/webapps/galaxy/config_schema.yml |  3 ++-
 lib/tool_shed/webapp/config_schema.yml      | 12 ++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml
index 375fdf5e275..4733fd2b23e 100644
--- a/lib/galaxy/webapps/galaxy/config_schema.yml
+++ b/lib/galaxy/webapps/galaxy/config_schema.yml
@@ -904,7 +904,8 @@ mapping:
 
       whitelist_file:
         type: str
-        default: config/disposable_email_whitelist.conf
+        default: disposable_email_whitelist.conf
+        path_resolves_to: config_dir
         required: false
         desc: |
           E-mail domains whitelist is used to specify allowed email address domains.
diff --git a/lib/tool_shed/webapp/config_schema.yml b/lib/tool_shed/webapp/config_schema.yml
index cc014e2c630..79fb569a79e 100644
--- a/lib/tool_shed/webapp/config_schema.yml
+++ b/lib/tool_shed/webapp/config_schema.yml
@@ -482,6 +482,18 @@ mapping:
           disposable email address during the registration.  If their address domain
           matches any domain in the blacklist, they are refused the registration.
 
+      whitelist_file:
+        type: str
+        default: disposable_email_whitelist.conf
+        path_resolves_to: config_dir
+        required: false
+        desc: |
+          E-mail domains whitelist is used to specify allowed email address domains.
+          If the list is non-empty and a user attempts registration using an email
+          address belonging to a domain that is not on the list, registration will be
+          enied. This is a more restrictive option than <blacklist_file>, and therefore,
+          in case <blacklist_file> is defined, <whitelist_file> will be ignored.
+
       brand:
         type: str
         default: null

From d2c0ddddfbe12b19ad49afa4e35ed8b375ea3990 Mon Sep 17 00:00:00 2001
From: MiguelJulia <migueljuliamolina@gmail.com>
Date: Fri, 6 Mar 2020 18:33:44 +0100
Subject: [PATCH 64/71] removed line unnecessary 471 for whitelist_file
 location

---
 lib/galaxy/config/__init__.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py
index 021e7a7d710..3a8c60a35e5 100644
--- a/lib/galaxy/config/__init__.py
+++ b/lib/galaxy/config/__init__.py
@@ -468,7 +468,6 @@ class GalaxyAppConfiguration(BaseAppConfiguration):
         #  Create whitelist file to accept only certain email domains
         self.whitelist_content = None
         if self.whitelist_file:
-            self.whitelist_file = os.path.join(self.root, self.whitelist_file)
             try:
                 with open(self.whitelist_file) as f:
                     self.whitelist_content = [line.rstrip() for line in f]

From 70f0a4dc896543b4672f9dd0c248ec8aa530b6d7 Mon Sep 17 00:00:00 2001
From: MiguelJulia <migueljuliamolina@gmail.com>
Date: Fri, 6 Mar 2020 18:37:23 +0100
Subject: [PATCH 65/71] make config-rebuild

---
 doc/source/admin/galaxy_options.rst           | 14 +++++++-------
 lib/galaxy/config/sample/galaxy.yml.sample    | 14 +++++++-------
 lib/galaxy/config/sample/tool_shed.yml.sample |  8 ++++++++
 3 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/doc/source/admin/galaxy_options.rst b/doc/source/admin/galaxy_options.rst
index 3f8728d28f1..ed7c6319cac 100644
--- a/doc/source/admin/galaxy_options.rst
+++ b/doc/source/admin/galaxy_options.rst
@@ -1226,13 +1226,13 @@
 ~~~~~~~~~~~~~~~~~~
 
 :Description:
-    E-mail domains whitelist is used for filtering out users that are
-    using email address not belonging to the instition/company domains
-    during the registration. If their address domain matches any
-    domain in the whitelist, they are refused the registration. This
-    is a more restrictive option than blacklist, and therefore, in
-    case the previous one is defined this one will be ignored.
-:Default: ``config/disposable_email_whitelist.conf``
+    E-mail domains whitelist is used to specify allowed email address
+    domains. If the list is non-empty and a user attempts registration
+    using an email address belonging to a domain that is not on the
+    list, registration will be enied. This is a more restrictive
+    option than <blacklist_file>, and therefore, in case
+    <blacklist_file> is defined, <whitelist_file> will be ignored.
+:Default: ``disposable_email_whitelist.conf``
 :Type: str
 
 
diff --git a/lib/galaxy/config/sample/galaxy.yml.sample b/lib/galaxy/config/sample/galaxy.yml.sample
index 9054cf7f2d7..e1e71076dfb 100644
--- a/lib/galaxy/config/sample/galaxy.yml.sample
+++ b/lib/galaxy/config/sample/galaxy.yml.sample
@@ -687,13 +687,13 @@ galaxy:
   # Example value 'config/disposable_email_blacklist.conf'
   #blacklist_file: null
 
-  # E-mail domains whitelist is used for filtering out users that are
-  # using email address not belonging to the instition/company domains
-  # during the registration. If their address domain matches any domain
-  # in the whitelist, they are refused the registration. This is a more
-  # restrictive option than blacklist, and therefore, in case the
-  # previous one is defined this one will be ignored.
-  #whitelist_file: config/disposable_email_whitelist.conf
+  # E-mail domains whitelist is used to specify allowed email address
+  # domains. If the list is non-empty and a user attempts registration
+  # using an email address belonging to a domain that is not on the
+  # list, registration will be enied. This is a more restrictive option
+  # than <blacklist_file>, and therefore, in case <blacklist_file> is
+  # defined, <whitelist_file> will be ignored.
+  #whitelist_file: disposable_email_whitelist.conf
 
   # Registration warning message is used to discourage people from
   # registering multiple accounts.  Applies mostly for the main Galaxy
diff --git a/lib/galaxy/config/sample/tool_shed.yml.sample b/lib/galaxy/config/sample/tool_shed.yml.sample
index 2e1934d55fa..52d6994638e 100644
--- a/lib/galaxy/config/sample/tool_shed.yml.sample
+++ b/lib/galaxy/config/sample/tool_shed.yml.sample
@@ -344,6 +344,14 @@ tool_shed:
   # the registration.
   #blacklist_file: config/disposable_email_blacklist.conf
 
+  # E-mail domains whitelist is used to specify allowed email address
+  # domains. If the list is non-empty and a user attempts registration
+  # using an email address belonging to a domain that is not on the
+  # list, registration will be enied. This is a more restrictive option
+  # than <blacklist_file>, and therefore, in case <blacklist_file> is
+  # defined, <whitelist_file> will be ignored.
+  #whitelist_file: disposable_email_whitelist.conf
+
   # Append "/{brand}" to the "Galaxy" text in the masthead.
   #brand: null
 

From 6a0eb4d746faf13bcb8e2bb11ca7576ac2df6f26 Mon Sep 17 00:00:00 2001
From: MiguelJulia <migueljuliamolina@gmail.com>
Date: Fri, 6 Mar 2020 19:57:41 +0100
Subject: [PATCH 66/71] changed priority: ignoring blacklist when whitelist
 exists

---
 lib/galaxy/security/validate_user_input.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/galaxy/security/validate_user_input.py b/lib/galaxy/security/validate_user_input.py
index 12bba4c06f5..d81f7ef5398 100644
--- a/lib/galaxy/security/validate_user_input.py
+++ b/lib/galaxy/security/validate_user_input.py
@@ -39,6 +39,13 @@ def validate_email(trans, email, user=None, check_dup=True, allow_empty=False):
         message = "Email address cannot be more than %d characters in length." % EMAIL_MAX_LEN
     elif check_dup and trans.sa_session.query(trans.app.model.User).filter(func.lower(trans.app.model.User.table.c.email) == email.lower()).first():
         message = "User with email '%s' already exists." % email
+    #  If the whitelist is not empty filter out any domain not in the list and ignore blacklist.
+    elif trans.app.config.whitelist_content is not None:
+        domain = email.split('@')[1]
+        if len(domain.split('.')) > 2:
+            domain = ('.').join(domain.split('.')[-2:])
+        if domain not in trans.app.config.whitelist_content:
+            message = "Please enter an allowed domain email address for this server."
     #  If the blacklist is not empty filter out the disposable domains.
     elif trans.app.config.blacklist_content is not None:
         domain = email.split('@')[1]
@@ -46,13 +53,6 @@ def validate_email(trans, email, user=None, check_dup=True, allow_empty=False):
             domain = ('.').join(domain.split('.')[-2:])
         if domain in trans.app.config.blacklist_content:
             message = "Please enter your permanent email address."
-    #  If the whitelist is not empty filter out any domain not in the list.
-    elif trans.app.config.whitelist_content is not None:
-        domain = email.split('@')[1]
-        if len(domain.split('.')) > 2:
-            domain = ('.').join(domain.split('.')[-2:])
-        if domain not in trans.app.config.whitelist_content:
-            message = "Please enter an allowed domain email address for this server."
     return message
 
 

From 64e4cfe19e7fd8efa54350c259629ddeb0c4628f Mon Sep 17 00:00:00 2001
From: MiguelJulia <migueljuliamolina@gmail.com>
Date: Fri, 6 Mar 2020 21:21:34 +0100
Subject: [PATCH 67/71] updated documentation with new priority over lists

---
 doc/source/admin/galaxy_options.rst           | 2 +-
 lib/galaxy/config/sample/galaxy.yml.sample    | 4 ++--
 lib/galaxy/config/sample/tool_shed.yml.sample | 4 ++--
 lib/galaxy/webapps/galaxy/config_schema.yml   | 2 +-
 lib/tool_shed/webapp/config_schema.yml        | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/doc/source/admin/galaxy_options.rst b/doc/source/admin/galaxy_options.rst
index ed7c6319cac..41abe433853 100644
--- a/doc/source/admin/galaxy_options.rst
+++ b/doc/source/admin/galaxy_options.rst
@@ -1231,7 +1231,7 @@
     using an email address belonging to a domain that is not on the
     list, registration will be enied. This is a more restrictive
     option than <blacklist_file>, and therefore, in case
-    <blacklist_file> is defined, <whitelist_file> will be ignored.
+    <whitelist_file> is defined, <blacklist_file> will be ignored.
 :Default: ``disposable_email_whitelist.conf``
 :Type: str
 
diff --git a/lib/galaxy/config/sample/galaxy.yml.sample b/lib/galaxy/config/sample/galaxy.yml.sample
index e1e71076dfb..dc1e81c10ed 100644
--- a/lib/galaxy/config/sample/galaxy.yml.sample
+++ b/lib/galaxy/config/sample/galaxy.yml.sample
@@ -691,8 +691,8 @@ galaxy:
   # domains. If the list is non-empty and a user attempts registration
   # using an email address belonging to a domain that is not on the
   # list, registration will be enied. This is a more restrictive option
-  # than <blacklist_file>, and therefore, in case <blacklist_file> is
-  # defined, <whitelist_file> will be ignored.
+  # than <blacklist_file>, and therefore, in case <whitelist_file> is
+  # defined, <blacklist_file> will be ignored.
   #whitelist_file: disposable_email_whitelist.conf
 
   # Registration warning message is used to discourage people from
diff --git a/lib/galaxy/config/sample/tool_shed.yml.sample b/lib/galaxy/config/sample/tool_shed.yml.sample
index 52d6994638e..0c4c9907021 100644
--- a/lib/galaxy/config/sample/tool_shed.yml.sample
+++ b/lib/galaxy/config/sample/tool_shed.yml.sample
@@ -348,8 +348,8 @@ tool_shed:
   # domains. If the list is non-empty and a user attempts registration
   # using an email address belonging to a domain that is not on the
   # list, registration will be enied. This is a more restrictive option
-  # than <blacklist_file>, and therefore, in case <blacklist_file> is
-  # defined, <whitelist_file> will be ignored.
+  # than <blacklist_file>, and therefore, in case <whitelist_file> is
+  # defined, <blacklist_file> will be ignored.
   #whitelist_file: disposable_email_whitelist.conf
 
   # Append "/{brand}" to the "Galaxy" text in the masthead.
diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml
index 4733fd2b23e..d8529ef43fd 100644
--- a/lib/galaxy/webapps/galaxy/config_schema.yml
+++ b/lib/galaxy/webapps/galaxy/config_schema.yml
@@ -912,7 +912,7 @@ mapping:
           If the list is non-empty and a user attempts registration using an email
           address belonging to a domain that is not on the list, registration will be
           enied. This is a more restrictive option than <blacklist_file>, and therefore,
-          in case <blacklist_file> is defined, <whitelist_file> will be ignored.
+          in case <whitelist_file> is defined, <blacklist_file> will be ignored.
 
       registration_warning_message:
         type: str
diff --git a/lib/tool_shed/webapp/config_schema.yml b/lib/tool_shed/webapp/config_schema.yml
index 79fb569a79e..9de65d6ef68 100644
--- a/lib/tool_shed/webapp/config_schema.yml
+++ b/lib/tool_shed/webapp/config_schema.yml
@@ -492,7 +492,7 @@ mapping:
           If the list is non-empty and a user attempts registration using an email
           address belonging to a domain that is not on the list, registration will be
           enied. This is a more restrictive option than <blacklist_file>, and therefore,
-          in case <blacklist_file> is defined, <whitelist_file> will be ignored.
+          in case <whitelist_file> is defined, <blacklist_file> will be ignored.
 
       brand:
         type: str

From 486d633cdef61a976a53a79cfb0080fe186d6619 Mon Sep 17 00:00:00 2001
From: Daniel Blankenberg <dan.blankenberg@gmail.com>
Date: Fri, 6 Mar 2020 16:46:05 -0500
Subject: [PATCH 68/71] run make config-rebuild

---
 doc/source/admin/galaxy_options.rst        | 4 +++-
 lib/galaxy/config/sample/galaxy.yml.sample | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/doc/source/admin/galaxy_options.rst b/doc/source/admin/galaxy_options.rst
index 1c78cc612b5..b7a8d9d6ad3 100644
--- a/doc/source/admin/galaxy_options.rst
+++ b/doc/source/admin/galaxy_options.rst
@@ -3265,7 +3265,9 @@
 :Description:
     XML config file that contains the job metric collection
     configuration.
-:Default: ``config/job_metrics_conf.xml``
+    The value of this option will be resolved with respect to
+    <config_dir>.
+:Default: ``job_metrics_conf.xml``
 :Type: str
 
 
diff --git a/lib/galaxy/config/sample/galaxy.yml.sample b/lib/galaxy/config/sample/galaxy.yml.sample
index dea2580aeb7..a2818be4173 100644
--- a/lib/galaxy/config/sample/galaxy.yml.sample
+++ b/lib/galaxy/config/sample/galaxy.yml.sample
@@ -1607,7 +1607,9 @@ galaxy:
 
   # XML config file that contains the job metric collection
   # configuration.
-  #job_metrics_config_file: config/job_metrics_conf.xml
+  # The value of this option will be resolved with respect to
+  # <config_dir>.
+  #job_metrics_config_file: job_metrics_conf.xml
 
   # This option allows users to see the job metrics (except for
   # environment variables).

From 89ba43f8472550cddf5e77c934eff0434a88d9fe Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Mon, 9 Mar 2020 19:27:35 -0400
Subject: [PATCH 69/71] Raise exception when trying to overwrite existing attr

Except base config attributes set in `_set_config_base()`
---
 lib/galaxy/config/__init__.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py
index 982fa59666c..c3a8bf5af59 100644
--- a/lib/galaxy/config/__init__.py
+++ b/lib/galaxy/config/__init__.py
@@ -212,11 +212,14 @@ class BaseAppConfiguration(object):
                 self._raw_config[key] = value
 
     def _create_attributes_from_raw_config(self):
+        # `base_configs` are a special case: these attributes have been created and will be ignored
+        # by the code below. Trying to overwrite any other existing attributes will raise an error.
+        base_configs = {'config_dir', 'data_dir', 'managed_config_dir'}
         for key, value in self._raw_config.items():
             if not hasattr(self, key):
                 setattr(self, key, value)
-            else:
-                log.debug("Attribute '%s' is set and cannot be overwritten with value '%s'" % (key, value))
+            elif key not in base_configs:
+                raise ConfigurationError("Attempting to override existing attribute '%s'" % key)
 
     def _resolve_paths(self):
 

From 7ac4d8301000939048cb6648b6d32471098d661b Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Mon, 9 Mar 2020 19:45:36 -0400
Subject: [PATCH 70/71] Use new schema.defaults property

Instead of app_schema['foo'].get('default') - as per code review
---
 lib/galaxy/tools/toolbox/base.py | 2 +-
 test/unit/tools/test_toolbox.py  | 6 +-----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/lib/galaxy/tools/toolbox/base.py b/lib/galaxy/tools/toolbox/base.py
index 08a4697564a..070791cf08f 100644
--- a/lib/galaxy/tools/toolbox/base.py
+++ b/lib/galaxy/tools/toolbox/base.py
@@ -1218,7 +1218,7 @@ class BaseGalaxyToolBox(AbstractToolBox):
         app_config_dict = self.app.config.config_dict
         conf_file = app_config_dict.get("dependency_resolvers_config_file")
         default_tool_dependency_dir = os.path.join(
-            self.app.config.data_dir, self.app.config.schema.app_schema['tool_dependency_dir'].get('default'))
+            self.app.config.data_dir, self.app.config.schema.defaults['tool_dependency_dir'])
         self.dependency_manager = build_dependency_manager(
             app_config_dict=app_config_dict, conf_file=conf_file, default_tool_dependency_dir=default_tool_dependency_dir)
 
diff --git a/test/unit/tools/test_toolbox.py b/test/unit/tools/test_toolbox.py
index d2bfc6caaab..974b30bfb41 100644
--- a/test/unit/tools/test_toolbox.py
+++ b/test/unit/tools/test_toolbox.py
@@ -564,11 +564,7 @@ class SimplifiedToolBox(ToolBox):
         app.tool_cache = ToolCache() if not hasattr(app, 'tool_cache') else app.tool_cache
         app.job_config.get_tool_resource_parameters = lambda tool_id: None
         app.config.update_integrated_tool_panel = True
-        app.config.schema.app_schema = {
-            'tool_dependency_dir': {
-                'default': 'dependencies',
-            }
-        }
+        app.config.schema.defaults = {'tool_dependency_dir': 'dependencies'}
         config_files = test_case.config_files
         tool_root_dir = test_case.test_directory
         super(SimplifiedToolBox, self).__init__(

From 76d52f40cd871a70894272cb1fa780ee964afc23 Mon Sep 17 00:00:00 2001
From: Nicola Soranzo <nicola.soranzo@earlham.ac.uk>
Date: Tue, 10 Mar 2020 04:18:41 +0000
Subject: [PATCH 71/71] Use auto_create functionality of
 `get_private_user_role()`

Address https://github.com/galaxyproject/galaxy/pull/9168/files#r361814891
---
 lib/galaxy/model/security.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/galaxy/model/security.py b/lib/galaxy/model/security.py
index ec728dc6143..a73e4967f51 100644
--- a/lib/galaxy/model/security.py
+++ b/lib/galaxy/model/security.py
@@ -658,8 +658,7 @@ class GalaxyRBACAgent(RBACAgent):
 
     def create_user_role(self, user, app):
         # Create private user role if necessary
-        if not self.get_private_user_role(user):
-            self.create_private_user_role(user)
+        self.get_private_user_role(user, auto_create=True)
         # Create default user permissions if necessary
         if not user.default_permissions:
             if hasattr(app.config, "new_user_dataset_access_role_default_private"):