public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ashesh Vashi <[email protected]>
To: Joao De Almeida Pereira <[email protected]>
Cc: Dave Page <[email protected]>
Cc: Khushboo Vashi <[email protected]>
Cc: Murtuza Zabuawala <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree
Date: Mon, 30 Apr 2018 20:33:03 +0530
Message-ID: <CAG7mmoy=5W3scnPs-TVpcu49F7gZU2qySPaakwfEdcg6rg2RMQ@mail.gmail.com> (raw)
In-Reply-To: <CAG7mmoym9838PVKdDD=469Bsx6AFBV1ThMK_dyitUmG26147Qg@mail.gmail.com>
References: <CAE+jja=Gdd032H7tpoZD2C0m2R7SnTZpHX_oPx2K2zGbaaW9yg@mail.gmail.com>
<CAFOhELcrZsRkwV4beOC7UT_HYvi9Rk=HFJ355g8Bj4bqyzMLog@mail.gmail.com>
<CAE+jjaneGOR_R7QvYnZt=khumRsyhnrzFRhpMG5-Q+rR9guLzQ@mail.gmail.com>
<CAKKotZROcoO_rtrrJUTvpfgSh1w4ig1ogQrkDrKSQHuKEL7CdQ@mail.gmail.com>
<CAE+jjakEZtn0ct128Xd+es5xCZ5eZLwoNZw6X0ue7oFB+a83oQ@mail.gmail.com>
<CAFOhELe_t_QGBbcYYm3Zrfwf3z9674PY5sy2QS46wHDTf-8DHA@mail.gmail.com>
<CAE+jja=u+W+FNaWGVs1KPZm1eYA_F=7ktomSbmsxOo-cjuOdBw@mail.gmail.com>
<CAE+jja=TZzMXOq4+t=dA+LfTGkW66urB37Tt9sEJ_UjAiymbbQ@mail.gmail.com>
<CAE+jjamSM_pBLNmYLn6-hWMiQvToceV2iPuME-eH5c+tgaYf7A@mail.gmail.com>
<CA+OCxowN15jg_DV4o+R+OSczyGHHccnJ1sPXE_XukYugaTKyyw@mail.gmail.com>
<CA+OCxozTMcEDKZEu9YCOsSBSed23Ka8hHiO5s6VF76KopbipNQ@mail.gmail.com>
<CAG7mmowrM3b8cTY+zcCAcVma8CKWHwXSF3=BUDcSFLpjH=jQBg@mail.gmail.com>
<CAE+jjanjQ295x0DgffT8OARRv8fge5KOPJpMCnLWe+j0jdqGJQ@mail.gmail.com>
<CAG7mmoym9838PVKdDD=469Bsx6AFBV1ThMK_dyitUmG26147Qg@mail.gmail.com>
On Mon, Apr 30, 2018 at 8:30 PM, Ashesh Vashi <[email protected]
> wrote:
> On Sat, Apr 28, 2018 at 3:55 AM, Joao De Almeida Pereira <
> [email protected]> wrote:
>
>> Hi Hackers,
>> As you are aware we kept on working on the patch, so we are attaching to
>> this email a new version of the patch.
>> This new version contains all the changes in the previous one plus more
>> extractions of functions and refactoring of code.
>>
>> The objective of this patch is to create a separation between pgAdmin and
>> the ACI Tree. We are doing this because we realized that at this point in
>> time we have the ACI Tree all over the code of pgAdmin. I found a very
>> interesting article that really talks about this:
>> https://medium.freecodecamp.org/code-dependencies-are-the-de
>> vil-35ed28b556d
>>
>> In this patch there are some visions and ideas about the location of the
>> code, the way to organize it and also try to pave the future for a
>> application that is stable, easy to develop on and that can be release at a
>> times notice.
>>
>> We are investing a big chunk of our time in doing this refactoring, but
>> while doing that we also try to respond to the patches sent to the mailing
>> list. We would like the feedback from the community because we believe this
>> is a changing point for the application. The idea is to change the way we
>> develop this application, instead of only correcting a bug of developing a
>> feature, with every commit we should correct the bug or develop a feature
>> but leave the code a little better than we found it (Refactoring,
>> refactoring, refactoring). This is hard work but that is what the users
>> from pgAdmin expect from this community of developers.
>>
>>
>> ======
>>
>>
>>
>> It is a huge patch
>> 86 files changed, 5492 inserts, 1840 deletions
>> and we would like to get your feedback as soon as possible, because we
>> are continuing to work on it which means it is going to grow in size.
>>
>>
>> At this point in time we still have 124 of 176 calls to the function
>> itemData from ACITree.
>>
>> What does each patch contain:
>> 0001:
>> Very simple patch, we found out that the linter was not looking into
>> all the javascript test files, so this patch will ensure it is
>>
> Committed the patch along with the regression introduced because of this
> patch.
>
>>
>> 0002:
>> New Tree abstraction. This patch contains the new Tree that works as an
>> adaptor for ACI Tree and is going to be used on all the extractions that we
>> are doing.
>>
>
> I was expecting a separate layer between the tree implementation, and
> aciTree adaptor.
> Please find the patch for the example.
>
> It will separate the two layers, and easy to replace with the new
> implemenation in future.
>
Oops forgot to attach the patch.
Please find the patch attached.
-- Thanks, Ashesh
>
>> 0003:
>> Code that extracts, wrap with tests and replace ACI Tree invocations.
>>
> There are many small cases left in the patches.
> Hence - I would like to know the TODO list created by you.
>
> e.g. When we remove any of the object from the database server, we're not
> yet removing the respective node from the new implementation, and its
> children.
>
>>
>> We start creating new pattern for the location of Javascript files and
>> their structure.
>>
> I would not like to see that changes in this patch.
> I would like us to come up with the actual design about the hot
> pluggability before going in this direction.
>
>> Create patterns for creation of dialogs (backup and restore)
>>
> It's better - we don't change the directory structure at the moment.
>
> I am not against dividing the big javascript files in small chunks, but -
> I would like us to discuss first about the hot plugins design first.
>
> -- Thanks, Ashesh
>
>>
>>
>
>>
>> Thanks
>> Joao
>>
>>
>> On Fri, Apr 27, 2018 at 5:34 AM Ashesh Vashi <
>> [email protected]> wrote:
>>
>>> I have quite a few comments for the patch.
>>> I will send them soon.
>>>
>>> On Fri, Apr 27, 2018, 14:45 Dave Page <[email protected]> wrote:
>>>
>>>> How is your work on this going Ashesh? Will you be committing today?
>>>>
>>>> On Wed, Apr 25, 2018 at 8:52 AM, Dave Page <[email protected]> wrote:
>>>>
>>>>> Ashesh; you had agreed to work on this early this week. Please ensure
>>>>> you do so today.
>>>>>
>>>>> Thanks.
>>>>>
>>>>> On Tue, Apr 24, 2018 at 8:13 PM, Joao De Almeida Pereira <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Hi Hackers,
>>>>>>
>>>>>> Can someone review and merge this patch?
>>>>>>
>>>>>> Thanks
>>>>>> Joao
>>>>>>
>>>>>> On Wed, Apr 18, 2018 at 10:23 AM Joao De Almeida Pereira <
>>>>>> [email protected]> wrote:
>>>>>>
>>>>>>> Hi Hackers,
>>>>>>> Any other comment about this patch?
>>>>>>>
>>>>>>> Thanks
>>>>>>> Joao
>>>>>>>
>>>>>>> On Tue, Apr 10, 2018 at 12:00 PM Joao De Almeida Pereira <
>>>>>>> [email protected]> wrote:
>>>>>>>
>>>>>>>> Hello Khushboo
>>>>>>>>
>>>>>>>> On Mon, Apr 9, 2018 at 1:59 AM Khushboo Vashi <
>>>>>>>> [email protected]> wrote:
>>>>>>>>
>>>>>>>>> Hi Joao,
>>>>>>>>>
>>>>>>>>> I have reviewed your patch and have some suggestions.
>>>>>>>>>
>>>>>>>>> On Sat, Apr 7, 2018 at 12:43 AM, Joao De Almeida Pereira <
>>>>>>>>> [email protected]> wrote:
>>>>>>>>>
>>>>>>>>>> Hello Murtuza/Dave,
>>>>>>>>>> Yes now the extracted functions are spread into different files.
>>>>>>>>>> The intent would be to make the files as small as possible, and also to
>>>>>>>>>> group and name them in a way that would be easy to understand what each
>>>>>>>>>> file is doing without the need of opening it.
>>>>>>>>>> As a example:
>>>>>>>>>> static/js/backup will contain all the backup related
>>>>>>>>>> functionality inside of this folder we can see the file:
>>>>>>>>>>
>>>>>>>>> menu_utils.js At this moment in time we decided to group all the
>>>>>>>>>> functions that are related to the menu, but we can split that also if we
>>>>>>>>>> believe it is easier to see.
>>>>>>>>>>
>>>>>>>>> It's really very good to see the separated code for backup module.
>>>>>>>>> As we have done for backup, we would like do it for other PG utilities like
>>>>>>>>> restore, maintenance etc.
>>>>>>>>> Considering this, we should separate the code in a way that some
>>>>>>>>> of the common functionalities can be used for other modules like menu (as
>>>>>>>>> you have mentioned above), dialogue factory etc.
>>>>>>>>> Also, I think these functionalities should be in their respective
>>>>>>>>> static folder instead of pgadmin/static.
>>>>>>>>>
>>>>>>>>
>>>>>>>> About the location of the files. The move of the files to
>>>>>>>> pgadmin/static/js was made on purpose in order to clearly separate
>>>>>>>> Javascript from python code.
>>>>>>>> The rational behind it was
>>>>>>>> - Create a clear separation between the backend and frontend
>>>>>>>> - Having Javascript code concentrated in a single place, hopefully,
>>>>>>>> will encourage to developers to look for a functionality, that is already
>>>>>>>> implemented in another modules, because they are right there. (When we
>>>>>>>> started this journey we realized that the 'nodes' have a big groups of code
>>>>>>>> that could be shared, but because the Javascript is spread everywhere it is
>>>>>>>> much harder to look for it)
>>>>>>>>
>>>>>>>>
>>>>>>>> There are some drawbacks of this separation:
>>>>>>>> - When creating a new module we will need to put the javascript in
>>>>>>>> a separate location from the backend code
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> static/js/datagrid folder contains all the datagrid related
>>>>>>>>>> functionality
>>>>>>>>>>
>>>>>>>>> Same as backup module, this should be in it's respective
>>>>>>>>> static/js folder.
>>>>>>>>>
>>>>>>>>>> Inside of the folder we can see the files:
>>>>>>>>>> get_panel_title.js is responsible for retrieving the name of the
>>>>>>>>>> panel
>>>>>>>>>> show_data.js is responsible for showing the datagrid
>>>>>>>>>> show_query_tool.js is responsible for showing the query tool
>>>>>>>>>>
>>>>>>>>>> Does this structure make sense?
>>>>>>>>>> Can you give an example of a comment that you think is missing
>>>>>>>>>> and that could help?
>>>>>>>>>>
>>>>>>>>>> As a personal note, unless the algorithm is very obscure or very
>>>>>>>>>> complicated, I believe that if the code needs comments it is a signal that
>>>>>>>>>> something needs to change in terms of naming, structure of the part in
>>>>>>>>>> question. This being said, I am open to add some comments that might help
>>>>>>>>>> people.
>>>>>>>>>>
>>>>>>>>> You are right, with the help of naming convention and structure of
>>>>>>>>> the code, any one can get the idea about the code. But it is very useful to
>>>>>>>>> understand the code
>>>>>>>>> very easily with the proper comments especially when there are
>>>>>>>>> multiple developers working on a single project.
>>>>>>>>>
>>>>>>>>> I found some of the places where it would be great to have
>>>>>>>>> comments.
>>>>>>>>>
>>>>>>>>> - treeMenu: new tree.Tree() in a browser.js
>>>>>>>>> - tree.js (especially Tree class)
>>>>>>>>>
>>>>>>>> About the comment point I need a more clear understanding on what
>>>>>>>> kind of comments you are looking for. Because when you read the function
>>>>>>>> names you understand the intent, what they are doing. The parameters also
>>>>>>>> explain what you need to pass into them.
>>>>>>>>
>>>>>>>> If what you are looking for in these comments is the reasoning
>>>>>>>> being the change itself, then that should be present in the commit message.
>>>>>>>> Specially because this is going to be a very big patch with a very big
>>>>>>>> number of changes.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>>> Joao
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>> Khushboo
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Fri, Apr 6, 2018 at 4:48 AM Murtuza Zabuawala <
>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Joao,
>>>>>>>>>>>
>>>>>>>>>>> Patch looks good and working as expected.
>>>>>>>>>>>
>>>>>>>>>>> I also agree with Dave, Can we please add some comments in each
>>>>>>>>>>> file which can help us to understand the flow, I'm saying because now the
>>>>>>>>>>> code is segregated in so many separate files it will be hard to keep track
>>>>>>>>>>> of the flow from one file to another when debugging.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> Regards,
>>>>>>>>>>> Murtuza Zabuawala
>>>>>>>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Thu, Apr 5, 2018 at 7:08 PM, Joao De Almeida Pereira <
>>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Khushboo,
>>>>>>>>>>>> Attached you can find both patches rebased
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Thu, Apr 5, 2018 at 6:31 AM Khushboo Vashi <
>>>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Joao,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Can you please rebase the second patch?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Khushboo
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Tue, Apr 3, 2018 at 12:15 AM, Joao De Almeida Pereira <
>>>>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Hackers,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Attached you can find the patch that will start to decouple
>>>>>>>>>>>>>> pgAdmin from ACITree library.
>>>>>>>>>>>>>> This patch is intended to be merged after 3.0, because we do
>>>>>>>>>>>>>> not want to cause any entropy or delay the release, but we want to start
>>>>>>>>>>>>>> the discussion and show some code.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This job that we started is a massive tech debt chore that
>>>>>>>>>>>>>> will take some time to finalize and we would love the help of the community
>>>>>>>>>>>>>> to do it.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> *Summary of the patch:*
>>>>>>>>>>>>>> 0001 patch:
>>>>>>>>>>>>>> - Creates a new tree that will allow us to create a
>>>>>>>>>>>>>> separation between the application and ACI Tree
>>>>>>>>>>>>>> - Creates a Fake Tree (Test double, for reference on the
>>>>>>>>>>>>>> available test doubles: https://martinfowler.
>>>>>>>>>>>>>> com/bliki/TestDouble.html) that can be used to inplace to
>>>>>>>>>>>>>> replace the ACITree and also encapsulate the new tree behavior on our tests
>>>>>>>>>>>>>> - Adds tests for all the tree functionalities
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 0002 patch:
>>>>>>>>>>>>>> - Extracts, refactors, adds tests and remove dependency from
>>>>>>>>>>>>>> ACI Tree on:
>>>>>>>>>>>>>> - getTreeNodeHierarchy
>>>>>>>>>>>>>> - on backup.js: menu_enabled, menu_enabled_server,
>>>>>>>>>>>>>> start_backup_global_server, backup_objects
>>>>>>>>>>>>>> - on datagrid.js: show_data_grid, get_panel_title,
>>>>>>>>>>>>>> show_query_tool
>>>>>>>>>>>>>> - Start using sprintf-js as Underscore.String is deprecating
>>>>>>>>>>>>>> sprintf function
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This patch represents only 10 calls to ACITree.itemData out
>>>>>>>>>>>>>> of 176 that are spread around our code
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> *In Depth look on the process behind the patch:*
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> We started writing this patch with the idea that we need to
>>>>>>>>>>>>>> decouple pgAdmin4 from ACITree, because ACITree is no longer supported, the
>>>>>>>>>>>>>> documentation is non existent and ACITree is no longer being actively
>>>>>>>>>>>>>> developed.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Our process:
>>>>>>>>>>>>>> 1. We "randomly" selected a function that is part of the
>>>>>>>>>>>>>> ACITree. From this point we decided to replace that function with our own
>>>>>>>>>>>>>> version. The function that we choose was "itemData".
>>>>>>>>>>>>>> The function gives us all the "data" that a specific node of
>>>>>>>>>>>>>> the tree find.
>>>>>>>>>>>>>> Given in order to replace the tree we would need to have a
>>>>>>>>>>>>>> function that would give us the same information. We had 2 options:
>>>>>>>>>>>>>> a) Create a tree with a function called itemData
>>>>>>>>>>>>>> Pros:
>>>>>>>>>>>>>> - At first view this was the simpler solution
>>>>>>>>>>>>>> - Would keep the status quo
>>>>>>>>>>>>>> Cons:
>>>>>>>>>>>>>> - Not a OOP approach
>>>>>>>>>>>>>> - Not very flexible
>>>>>>>>>>>>>> b) Create a tree that would return a node given an ID and
>>>>>>>>>>>>>> then the node would be responsible for giving it's data.
>>>>>>>>>>>>>> Pros:
>>>>>>>>>>>>>> - OOP Approach
>>>>>>>>>>>>>> - More flexible and we do not need to bring the tree around,
>>>>>>>>>>>>>> just a node
>>>>>>>>>>>>>> Cons:
>>>>>>>>>>>>>> - Break the current status quo
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Given these 2 options we decided to go for a more OOP
>>>>>>>>>>>>>> approach creating a Tree and a TreeNode classes, that in the future will be
>>>>>>>>>>>>>> renamed to ACITreeWrapper and TreeNode.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 2. After we decided on the starting point we searched for
>>>>>>>>>>>>>> occurrences of the function "itemData" and we found out that there were 303
>>>>>>>>>>>>>> occurrences of "itemData" in the code and roughly 176 calls to the function
>>>>>>>>>>>>>> itself (some of the hits were variable names).
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 3. We selected the first file on the search and found the
>>>>>>>>>>>>>> function that was responsible for calling the itemData function.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 4. Extracted the function to a separate file
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 5. Wrap this function with tests
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 6. Refactor the function to ES6, give more declarative names
>>>>>>>>>>>>>> to variables and break the functions into smaller chunks
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 7. When all the tests were passing we replaced ACITree with
>>>>>>>>>>>>>> our Tree
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 8. We ensured that all tests were passing
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 9. Remove function from the original file and use the new
>>>>>>>>>>>>>> function
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 10. Ensure everything still works
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 11. Find the next function and execute from step 4 until all
>>>>>>>>>>>>>> the functions are replaced, refactored and tested.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> As you can see by the process this is a pretty huge
>>>>>>>>>>>>>> undertake, because of the number of calls to the function. This is just the
>>>>>>>>>>>>>> first step on the direction of completely isolating the ACITree so that we
>>>>>>>>>>>>>> can solve the problem with a large number of elements on the tree.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> *What is on our radar that we need to address:*
>>>>>>>>>>>>>> - Finish the complete decoupling of the ACITree
>>>>>>>>>>>>>> - Performance of the current tree implementation
>>>>>>>>>>>>>> - Tweak the naming of the Tree class to explicitly tell us
>>>>>>>>>>>>>> this is to use only with ACITree.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>> Joao
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Dave Page
>>>>> Blog: http://pgsnake.blogspot.com
>>>>> Twitter: @pgsnake
>>>>>
>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>> The Enterprise PostgreSQL Company
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>
>
Attachments:
[application/octet-stream] 0002-New-tree-implemenation.patch (23.7K, 3-0002-New-tree-implemenation.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/static/js/browser.js b/web/pgadmin/browser/static/js/browser.js
index 897d270..fc68089 100644
--- a/web/pgadmin/browser/static/js/browser.js
+++ b/web/pgadmin/browser/static/js/browser.js
@@ -1,4 +1,5 @@
define('pgadmin.browser', [
+ 'sources/tree/tree',
'sources/gettext', 'sources/url_for', 'require', 'jquery', 'underscore', 'underscore.string',
'bootstrap', 'sources/pgadmin', 'pgadmin.alertifyjs', 'bundled_codemirror',
'sources/check_node_visibility', 'sources/modify_animation', 'pgadmin.browser.utils', 'wcdocker',
@@ -10,6 +11,7 @@ define('pgadmin.browser', [
'sources/codemirror/addon/fold/pgadmin-sqlfoldcode',
'pgadmin.browser.keyboard',
], function(
+ tree,
gettext, url_for, require, $, _, S, Bootstrap, pgAdmin, Alertify,
codemirror, checkNodeVisibility, modifyAnimation
) {
@@ -86,6 +88,7 @@ define('pgadmin.browser', [
});
b.tree = $('#tree').aciTree('api');
+ b.treeMenu.register($('#tree'));
};
// Extend the browser class attributes
@@ -100,6 +103,7 @@ define('pgadmin.browser', [
editor:null,
// Left hand browser tree
tree:null,
+ treeMenu: new tree.Tree(),
// list of script to be loaded, when a certain type of node is loaded
// It will be used to register extensions, tools, child node scripts,
// etc.
diff --git a/web/pgadmin/static/js/tree/tree.js b/web/pgadmin/static/js/tree/tree.js
new file mode 100644
index 0000000..8048771
--- /dev/null
+++ b/web/pgadmin/static/js/tree/tree.js
@@ -0,0 +1,275 @@
+//////////////////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////////////////
+
+
+class BaseTreeNode {
+ constructor(id, data, domNode, parent) {
+ this.id = id;
+ this.data = data;
+ this.setParent(parent);
+ this.children = [];
+ this.domNode = domNode;
+ }
+
+ hasParent() {
+ return isNotNullOrUndefined(this.parentNode);
+ }
+
+ parent() {
+ return this.parentNode;
+ }
+
+ setParent(parent) {
+ this.parentNode = parent;
+ if (isNotNullOrUndefined(parent)) {
+ this.path = this.id;
+ if (isNotNullOrUndefined(parent) && isNotNullOrUndefined(parent.path)) {
+ this.path = parent.path + '.' + this.id;
+ }
+ } else {
+ this.path = null;
+ }
+ }
+
+ getData() {
+ if (this.data === undefined) {
+ return undefined;
+ } else if (this.data === null) {
+ return null;
+ }
+ return Object.assign({}, this.data);
+ }
+
+ getHtmlIdentifier() {
+ return this.domNode;
+ }
+
+ reload(tree) {
+ // Implement it in the actual implementation
+ // This is more of a shell object.
+ throw 'Not Implemented', tree;
+ }
+
+ unload(tree) {
+ this.children.forEach(function(child) {
+ child.unload(tree);
+ child.setParent(null);
+ });
+ this.children = [];
+ }
+
+ anyParent(condition) {
+ let node = this;
+
+ while (node.hasParent()) {
+ node = node.parent();
+ if (condition(node)) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ anyFamilyMember(condition) {
+ if(condition(this)) {
+ return true;
+ }
+
+ return this.anyParent(condition);
+ }
+}
+
+
+class BaseTree {
+ constructor() {
+ this.rootNode = this._createNewTreeNode(undefined, {});
+ }
+
+ addNewNode(id, data, domNode, parentPath) {
+ const parent = this.findNode(parentPath);
+ return this.createOrUpdateNode(id, data, parent, domNode);
+ }
+
+ findNode(path) {
+ if (path === null || path === undefined || path.length === 0) {
+ return this.rootNode;
+ }
+ return findInTree(this.rootNode, path.join('.'));
+ }
+
+ findNodeByDomElement(/* domElement */) {
+ // TODO::
+ // Think through the new implementation - How is it going to work with real
+ // domElement?
+ //
+ // NOTE: We already have implementation for the aciTree.
+ throw 'Not Implemented';
+ }
+
+ selected() {
+ throw 'Not Implemented';
+ }
+
+ selectNode(/* node */) {
+ throw 'Not Implemented';
+ }
+
+ register(/* $treeJQuery */) {
+ throw 'Not Implemented';
+ }
+
+ _createNewTreeNode(/* id, data, domNode, parent */) {
+ throw 'Not Implemented';
+ }
+
+ createOrUpdateNode(id, data, parent, domNode) {
+ let oldNodePath = [id];
+ if(parent !== null && parent !== undefined) {
+ oldNodePath = [parent.path, id];
+ }
+ const oldNode = this.findNode(oldNodePath);
+ if (oldNode !== null) {
+ oldNode.data = Object.assign({}, data);
+ return oldNode;
+ }
+
+ const node = this._createNewTreeNode(id, data, domNode, parent);
+ if (parent === this.rootNode) {
+ node.parentNode = null;
+ }
+ if (isNotNullOrUndefined(parent)) {
+ parent.children.push(node);
+ }
+ return node;
+ }
+}
+
+
+function findInTree(rootNode, path) {
+ if (path === null) {
+ return rootNode;
+ }
+ return (function findInNode(currentNode) {
+ for (let i = 0, length = currentNode.children.length; i < length; i++) {
+ const calculatedNode = findInNode(currentNode.children[i]);
+ if (calculatedNode !== null) {
+ return calculatedNode;
+ }
+ }
+
+ if (currentNode.path === path) {
+ return currentNode;
+ } else {
+ return null;
+ }
+ })(rootNode);
+}
+
+
+class ACITreeNode extends BaseTreeNode {
+ constructor(id, data, domNode, parent) {
+ super(id, data, domNode, parent);
+ }
+
+ reload(tree) {
+ this.unload(tree);
+ tree.aciTreeApi.setInode(this.domNode);
+ tree.aciTreeApi.deselect(this.domNode);
+
+ setTimeout(() => {
+ tree.selectNode(this.domNode);
+ }, 0);
+ }
+
+ unload(tree) {
+ // Do not pass on tree object to 'super' object to avoid unloading the
+ // children recursively for performance reason.
+ //
+ // Any way - unloading using 'aciTreeApi.unload(...)' will any way unload
+ // all children.
+ super.unload(null);
+
+ if (isNotNullOrUndefined(tree) && isNotNullOrUndefined(tree.aciTreeApi)) {
+ tree.aciTreeApi.unload(this.domNode);
+ } else {
+ this.domNode = null;
+ }
+ }
+}
+
+
+function isNotNullOrUndefined(val) {
+ return (val !== undefined && val !== null);
+}
+
+
+
+class ACITree extends BaseTree {
+ constructor() {
+ super(arguments);
+ this.aciTreeApi = null;
+ }
+
+ _createNewTreeNode(id, data, domNode, parent) {
+ let node = new ACITreeNode(id, data, domNode, parent);
+ return node;
+ }
+
+ findNodeByDomElement(domElement) {
+ const path = this.translateTreeNodeIdFromACITree(domElement);
+ if(!path || !path[0]) {
+ return undefined;
+ }
+
+ return this.findNode(path);
+ }
+
+ selected() {
+ return this.aciTreeApi.selected();
+ }
+
+ selectNode(aciTreeIdentifier) {
+ this.aciTreeApi.select(aciTreeIdentifier);
+ }
+
+ register($treeJQuery) {
+ $treeJQuery.on('acitree', function (event, api, item, eventName) {
+ if (api.isItem(item)) {
+ if (eventName === 'added') {
+ const id = api.getId(item);
+ const data = api.itemData(item);
+ const parentId = this.translateTreeNodeIdFromACITree(api.parent(item));
+ this.addNewNode(id, data, item, parentId);
+ }
+ }
+ }.bind(this));
+ this.aciTreeApi = $treeJQuery.aciTree('api');
+ }
+
+ translateTreeNodeIdFromACITree(aciTreeNode) {
+ let currentTreeNode = aciTreeNode;
+ let path = [];
+ while (currentTreeNode !== null && currentTreeNode !== undefined && currentTreeNode.length > 0) {
+ path.unshift(this.aciTreeApi.getId(currentTreeNode));
+ if (this.aciTreeApi.hasParent(currentTreeNode)) {
+ currentTreeNode = this.aciTreeApi.parent(currentTreeNode);
+ } else {
+ break;
+ }
+ }
+ return path;
+ }
+
+ static test() {
+ return 'ACITree';
+ }
+}
+
+export { ACITree as Tree, ACITreeNode as TreeNode };
diff --git a/web/regression/javascript/tree/tree_fake.js b/web/regression/javascript/tree/tree_fake.js
new file mode 100644
index 0000000..b285a45
--- /dev/null
+++ b/web/regression/javascript/tree/tree_fake.js
@@ -0,0 +1,69 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+
+import {Tree} from '../../../pgadmin/static/js/tree/tree';
+
+export class TreeFake extends Tree {
+ constructor() {
+ super();
+ this.aciTreeToOurTreeTranslator = {};
+ this.aciTreeApi = jasmine.createSpyObj(
+ ['ACITreeApi'], ['setInode', 'unload', 'deselect', 'select']);
+ }
+
+ addNewNode(id, data, domNode, path) {
+ this.aciTreeToOurTreeTranslator[id] = [id];
+ if (path !== null && path !== undefined) {
+ this.aciTreeToOurTreeTranslator[id] = path.concat(id);
+ }
+ return super.addNewNode(id, data, domNode, path);
+ }
+
+ addChild(parent, child) {
+ child.setParent(parent);
+ this.aciTreeToOurTreeTranslator[child.id] = this.aciTreeToOurTreeTranslator[parent.id].concat(child.id);
+ parent.children.push(child);
+ }
+
+ hasParent(aciTreeNode) {
+ return this.translateTreeNodeIdFromACITree(aciTreeNode).length > 1;
+ }
+
+ parent(aciTreeNode) {
+ if (this.hasParent(aciTreeNode)) {
+ let path = this.translateTreeNodeIdFromACITree(aciTreeNode);
+ return [{id: this.findNode(path).parent().id}];
+ }
+
+ return null;
+ }
+
+ translateTreeNodeIdFromACITree(aciTreeNode) {
+ if(aciTreeNode === undefined || aciTreeNode[0] === undefined) {
+ return null;
+ }
+ return this.aciTreeToOurTreeTranslator[aciTreeNode[0].id];
+ }
+
+ itemData(aciTreeNode) {
+ let node = this.findNodeByDomElement(aciTreeNode);
+ if (node === undefined || node === null) {
+ return undefined;
+ }
+ return node.getData();
+ }
+
+ selected() {
+ return this.selectedNode;
+ }
+
+ selectNode(selectedNode) {
+ this.selectedNode = selectedNode;
+ }
+}
diff --git a/web/regression/javascript/tree/tree_spec.js b/web/regression/javascript/tree/tree_spec.js
new file mode 100644
index 0000000..164ceb5
--- /dev/null
+++ b/web/regression/javascript/tree/tree_spec.js
@@ -0,0 +1,421 @@
+//////////////////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////////////////
+
+import {Tree, TreeNode} from '../../../pgadmin/static/js/tree/tree';
+import {TreeFake} from './tree_fake';
+
+const context = describe;
+
+const treeTests = (treeClass, setDefaultCallBack) => {
+ let tree;
+ beforeEach(() => {
+ tree = new treeClass();
+ });
+
+ describe('#addNewNode', () => {
+ describe('when add a new root element', () => {
+ context('using [] as the parent', () => {
+ beforeEach(() => {
+ tree.addNewNode('some new node', {data: 'interesting'}, undefined, []);
+ });
+
+ it('can be retrieved', () => {
+ const node = tree.findNode(['some new node']);
+ expect(node.data).toEqual({data: 'interesting'});
+ });
+
+ it('return false for #hasParent()', () => {
+ const node = tree.findNode(['some new node']);
+ expect(node.hasParent()).toBe(false);
+ });
+
+ it('return null for #parent()', () => {
+ const node = tree.findNode(['some new node']);
+ expect(node.parent()).toBeNull();
+ });
+ });
+
+ context('using null as the parent', () => {
+ beforeEach(() => {
+ tree.addNewNode('some new node', {data: 'interesting'}, undefined, null);
+ });
+
+ it('can be retrieved', () => {
+ const node = tree.findNode(['some new node']);
+ expect(node.data).toEqual({data: 'interesting'});
+ });
+
+ it('return false for #hasParent()', () => {
+ const node = tree.findNode(['some new node']);
+ expect(node.hasParent()).toBe(false);
+ });
+
+ it('return null for #parent()', () => {
+ const node = tree.findNode(['some new node']);
+ expect(node.parent()).toBeNull();
+ });
+ });
+
+ context('using undefined as the parent', () => {
+ beforeEach(() => {
+ tree.addNewNode('some new node', {data: 'interesting'});
+ });
+
+ it('can be retrieved', () => {
+ const node = tree.findNode(['some new node']);
+ expect(node.data).toEqual({data: 'interesting'});
+ });
+
+ it('return false for #hasParent()', () => {
+ const node = tree.findNode(['some new node']);
+ expect(node.hasParent()).toBe(false);
+ });
+
+ it('return null for #parent()', () => {
+ const node = tree.findNode(['some new node']);
+ expect(node.parent()).toBeNull();
+ });
+ });
+ });
+
+ describe('when add a new element as a child', () => {
+ let parentNode;
+ beforeEach(() => {
+ parentNode = tree.addNewNode('parent node', {data: 'parent data'}, undefined, []);
+ tree.addNewNode('some new node', {data: 'interesting'}, undefined, ['parent' +
+ ' node']);
+ });
+
+ it('can be retrieved', () => {
+ const node = tree.findNode(['parent node', 'some new node']);
+ expect(node.data).toEqual({data: 'interesting'});
+ });
+
+ it('return true for #hasParent()', () => {
+ const node = tree.findNode(['parent node', 'some new node']);
+ expect(node.hasParent()).toBe(true);
+ });
+
+ it('return "parent node" object for #parent()', () => {
+ const node = tree.findNode(['parent node', 'some new node']);
+ expect(node.parent()).toEqual(parentNode);
+ });
+ });
+
+ describe('when add an element that already exists under a parent', () => {
+ beforeEach(() => {
+ tree.addNewNode('parent node', {data: 'parent data'}, undefined, []);
+ tree.addNewNode('some new node', {data: 'interesting'}, undefined, ['parent' +
+ ' node']);
+ });
+
+ it('does not add a new child', () => {
+ tree.addNewNode('some new node', {data: 'interesting 1'}, undefined, ['parent' +
+ ' node']);
+ const parentNode = tree.findNode(['parent node']);
+ expect(parentNode.children.length).toBe(1);
+ });
+
+ it('updates the existing node data', () => {
+ tree.addNewNode('some new node', {data: 'interesting 1'}, undefined, ['parent' +
+ ' node']);
+ const node = tree.findNode(['parent node', 'some new node']);
+ expect(node.data).toEqual({data: 'interesting 1'});
+ });
+ });
+ });
+
+ describe('#translateTreeNodeIdFromACITree', () => {
+ let aciTreeApi;
+ beforeEach(() => {
+ aciTreeApi = jasmine.createSpyObj('ACITreeApi', [
+ 'hasParent',
+ 'parent',
+ 'getId',
+ ]);
+
+ aciTreeApi.getId.and.callFake((node) => {
+ return node[0].id;
+ });
+ tree.aciTreeApi = aciTreeApi;
+ });
+
+ describe('When tree as a single level', () => {
+ beforeEach(() => {
+ aciTreeApi.hasParent.and.returnValue(false);
+ });
+
+ it('returns an array with the ID of the first level', () => {
+ let node = [{
+ id: 'some id',
+ }];
+ tree.addNewNode('some id', {}, undefined, []);
+
+ expect(tree.translateTreeNodeIdFromACITree(node)).toEqual(['some id']);
+ });
+ });
+
+ describe('When tree as a 2 levels', () => {
+ describe('When we try to retrieve the node in the second level', () => {
+ it('returns an array with the ID of the first level and second level', () => {
+ aciTreeApi.hasParent.and.returnValues(true, false);
+ aciTreeApi.parent.and.returnValue([{
+ id: 'parent id',
+ }]);
+ let node = [{
+ id: 'some id',
+ }];
+
+ tree.addNewNode('parent id', {}, undefined, []);
+ tree.addNewNode('some id', {}, undefined, ['parent id']);
+
+ expect(tree.translateTreeNodeIdFromACITree(node))
+ .toEqual(['parent id', 'some id']);
+ });
+ });
+ });
+ });
+
+ describe('#selected', () => {
+ context('a node in the tree is selected', () => {
+ it('returns that node object', () => {
+ let selectedNode = new TreeNode('bamm', {}, []);
+ setDefaultCallBack(tree, selectedNode);
+ expect(tree.selected()).toEqual(selectedNode);
+ });
+ });
+ });
+
+ describe('#findNodeByTreeElement', () => {
+ context('retrieve data from node not found', () => {
+ it('return undefined', () => {
+ let aciTreeApi = jasmine.createSpyObj('ACITreeApi', [
+ 'hasParent',
+ 'parent',
+ 'getId',
+ ]);
+
+ aciTreeApi.getId.and.callFake((node) => {
+ return node[0].id;
+ });
+ tree.aciTreeApi = aciTreeApi;
+ expect(tree.findNodeByDomElement(['<li>something</li>'])).toBeUndefined();
+ });
+ });
+ });
+};
+
+describe('tree tests', () => {
+ describe('TreeNode', () => {
+ describe('#hasParent', () => {
+ context('parent is null', () => {
+ it('returns false', () => {
+ let treeNode = new TreeNode('123', {}, [], null);
+ expect(treeNode.hasParent()).toBe(false);
+ });
+ });
+ context('parent is undefined', () => {
+ it('returns false', () => {
+ let treeNode = new TreeNode('123', {}, [], undefined);
+ expect(treeNode.hasParent()).toBe(false);
+ });
+ });
+ context('parent exists', () => {
+ it('returns true', () => {
+ let parentNode = new TreeNode('456', {}, []);
+ let treeNode = new TreeNode('123', {}, [], parentNode);
+ expect(treeNode.hasParent()).toBe(true);
+ });
+ });
+ });
+
+ describe('#reload', () => {
+ let tree;
+ let level2;
+ beforeEach(() => {
+ tree = new TreeFake();
+ tree.addNewNode('level1', {data: 'interesting'}, [{id: 'level1'}], []);
+ level2 = tree.addNewNode('level2', {data: 'data'}, [{id: 'level2'}], ['level1']);
+ tree.addNewNode('level3', {data: 'more data'}, [{id: 'level3'}], ['level1', 'level2']);
+
+ tree.aciTreeApi = jasmine.createSpyObj(
+ 'ACITreeApi', ['setInode', 'unload', 'deselect', 'select']);
+ });
+
+ it('reloads the node and its children', () => {
+ level2.reload(tree);
+ expect(tree.findNodeByDomElement([{id: 'level2'}])).toEqual(level2);
+ });
+
+ it('does not reload the children of node', () => {
+ level2.reload(tree);
+ expect(tree.findNodeByDomElement([{id: 'level3'}])).toBeNull();
+ });
+
+ it('select the node', (done) => {
+ level2.reload(tree);
+ setTimeout(() => {
+ expect(tree.selected()).toEqual([{id: 'level2'}]);
+ done();
+ }, 20);
+ });
+
+ describe('ACITree specific', () => {
+ it('sets the current node as a Inode, changing the Icon back to +', () => {
+ level2.reload(tree);
+ expect(tree.aciTreeApi.setInode).toHaveBeenCalledWith([{id: 'level2'}]);
+ });
+
+ it('deselect the node and selects it again to trigger ACI tree' +
+ ' events', (done) => {
+ level2.reload(tree);
+ setTimeout(() => {
+ expect(tree.aciTreeApi.deselect).toHaveBeenCalledWith([{id: 'level2'}]);
+ done();
+ }, 20);
+ });
+ });
+ });
+
+ describe('#unload', () => {
+ let tree;
+ let level2;
+ beforeEach(() => {
+ tree = new TreeFake();
+ tree.addNewNode('level1', {data: 'interesting'}, ['<li>level1</li>'], []);
+ level2 = tree.addNewNode('level2', {data: 'data'}, ['<li>level2</li>'], ['level1']);
+ tree.addNewNode('level3', {data: 'more data'}, ['<li>level3</li>'], ['level1', 'level2']);
+ tree.aciTreeApi = jasmine.createSpyObj('ACITreeApi', ['unload']);
+ });
+
+ it('unloads the children of the current node', () => {
+ level2.unload(tree);
+ expect(tree.findNodeByDomElement([{id: 'level2'}])).toEqual(level2);
+ expect(tree.findNodeByDomElement([{id: 'level3'}])).toBeNull();
+ });
+
+ it('calls unload on the ACI Tree', () => {
+ level2.unload(tree);
+ expect(tree.aciTreeApi.unload).toHaveBeenCalledWith(['<li>level2</li>']);
+ });
+ });
+ });
+
+ describe('Tree', () => {
+ function realTreeSelectNode(tree, selectedNode) {
+ let aciTreeApi = jasmine.createSpyObj('ACITreeApi', [
+ 'selected',
+ ]);
+ tree.aciTreeApi = aciTreeApi;
+ aciTreeApi.selected.and.returnValue(selectedNode);
+ }
+
+ treeTests(Tree, realTreeSelectNode);
+ });
+
+ describe('TreeFake', () => {
+ function fakeTreeSelectNode(tree, selectedNode) {
+ tree.selectNode(selectedNode);
+ }
+
+ treeTests(TreeFake, fakeTreeSelectNode);
+
+ describe('#hasParent', () => {
+ context('tree contains multiple levels', () => {
+ let tree;
+ beforeEach(() => {
+ tree = new TreeFake();
+ tree.addNewNode('level1', {data: 'interesting'}, undefined, []);
+ tree.addNewNode('level2', {data: 'interesting'}, undefined, ['level1']);
+ });
+
+ context('node is at the first level', () => {
+ it('returns false', () => {
+ expect(tree.hasParent([{id: 'level1'}])).toBe(false);
+ });
+ });
+
+ context('node is at the second level', () => {
+ it('returns true', () => {
+ expect(tree.hasParent([{id: 'level2'}])).toBe(true);
+ });
+ });
+ });
+ });
+
+ describe('#parent', () => {
+ let tree;
+ beforeEach(() => {
+ tree = new TreeFake();
+ tree.addNewNode('level1', {data: 'interesting'}, undefined, []);
+ tree.addNewNode('level2', {data: 'interesting'}, undefined, ['level1']);
+ });
+
+ context('node is the root', () => {
+ it('returns null', () => {
+ expect(tree.parent([{id: 'level1'}])).toBeNull();
+ });
+ });
+
+ context('node is not root', () => {
+ it('returns root element', () => {
+ expect(tree.parent([{id: 'level2'}])).toEqual([{id: 'level1'}]);
+ });
+ });
+ });
+
+ describe('#itemData', () => {
+ let tree;
+ beforeEach(() => {
+ tree = new TreeFake();
+ tree.addNewNode('level1', {data: 'interesting'}, undefined, []);
+ tree.addNewNode('level2', {data: 'expected data'}, undefined, ['level1']);
+ });
+
+ context('retrieve data from the node', () => {
+ it('return the node data', () => {
+ expect(tree.itemData([{id: 'level2'}])).toEqual({
+ data: 'expected' +
+ ' data',
+ });
+ });
+ });
+
+ context('retrieve data from node not found', () => {
+ it('return undefined', () => {
+ expect(tree.itemData([{id: 'bamm'}])).toBeUndefined();
+ });
+ });
+ });
+
+ describe('#addChild', () => {
+ let root, child;
+ beforeEach(() => {
+ let tree = new TreeFake();
+ root = tree.addNewNode('root', {}, [{id: 'root'}]);
+ child = new TreeNode('node.1', {}, [{id: 'node.1'}]);
+ tree.addChild(root, child);
+ });
+
+ it('adds a new child to a node', () => {
+ expect(root.children).toEqual([child]);
+ });
+
+ it('changes the parent of the child node', () => {
+ expect(root.children[0].parentNode).toEqual(root);
+ expect(child.parentNode).toEqual(root);
+ });
+
+ it('changes the path of the child', () => {
+ expect(child.path).toEqual('root.node.1');
+ });
+ });
+ });
+});
+
view thread (69+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree
In-Reply-To: <CAG7mmoy=5W3scnPs-TVpcu49F7gZU2qySPaakwfEdcg6rg2RMQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox