Refactoring of WebTerminal.Engine class

[eduard93]

I propose refactoring of WebTerminal.Engine class into following classes:

• WebTerminal.Parameters - contains parameters
• WebTerminal.Redirects - extends %Library.Routine and contains methods working with redirects
• WebTerminal.Actions - contains user actions (ExecuteCommand, ExecuteSQL Update, Tracking etc)
• WebTerminal.Autocomplete - contains autocomplete logic
• WebTerminal.Engine - extends all previous classes, contains OnMethods, Server, ClientLoop

I'm not sure about exact class structure, but that's the gist of it. Would make for an easier reading.

[eduard93]

Also WebTerminal.Actions would be testable in terminal (usual) context. Maybe.

[nikitaeverywhere]

WebTerminal.Engine - extends all previous classes

WebTerminal.Engine extends %CSP.WebSocket, which is an important point - all previous classes should be encapsulated inside Engine class to have ability to interact with the client.

Refactoring is always a good idea, Edward!

But there is another point actual for now - renaming classes (or creating a new ones) has a downbeat effect on the terminal's /update command right now. Somewhere in the heaven the RemoveProjection method of WebTerminal.Installer class should be aware of all the terminal's classes to remove (OK, let it be the WebTerminal package), but also has to be aware of the namespace CWT installed to by user, which is not implemented for now. In case of user remove classes from the namespace classes were once mapped to, WebTerminal.Installer won't know the "original" namespace, and will skip removing original package. Any ideas how to deal with all of this?

[eduard93]

has to be aware of the namespace CWT installed to by user

Follow the mappings :) They point to database associated with namespace we need

[nikitaeverywhere]

They point to database associated with namespace we need

The only thing mappings tell us is the database where package is stored. But how to get the namespace? Well, it is a database property to have a default namespace, and not vice versa, isn't it?

[eduard93]

You can use Config.Namespaces:List query and iterate over it till you find first namespace pointing to this database. Since Namespace count does not usually go higher than lower hundreds it would not be a time-consuming operation.

[nikitaeverywhere]

You can use Config.Namespaces:List query and iterate over it till you find first namespace pointing to this database.

But what if two namespaces point to the same database?

[eduard93]

But what if two namespaces point to the same database?

So? Any will do.

[nikitaeverywhere]

Does the class unique for a namespace or a database? Your answer likely tells the last variant correct, right?

[eduard93]

Namespace stores classes in routines database. So the class is unique for a namespace. Class is also unique for a database, because class, like everything else is a global (Class definition = subscript node in ^oddDEF global). So both are correct. Class is unique for a namespace and a database

[nikitaeverywhere]

Config.Namespaces:List query

OK, I'll try.

[eduard93]

Do you want to focus on other issues? I can take this one.

[nikitaeverywhere]

Do you want to take the last issue discussed (always correct deleting and unmapping) issue, or a full refactoring phase? In case of the last one, I think it is better for me first to redefine how classes inside repository are stored with a simple gulp task to collect them into one project.

[eduard93]

The second one. Yes, please do redefine how classes inside repository are stored.

[eduard93]

About first. Get db via w ##class(%SYS.Namespace).GetPackageDest($Namespace,"WebTerminal")

[eduard93]

About first. Here's get original namespace method

[nikitaeverywhere]

Yes, please do redefine how classes inside repository are stored.

Ready with 1bded7c11b4f82540648a2bd58fd90e50d5c5e2c. The primary concepts and "how to"s I have placed inside DEVELOPMENT.md file.

[nikitaeverywhere]

About first. Get db via w ##class(%SYS.Namespace).GetPackageDest($Namespace,"WebTerminal")

,

About first. Here's get original namespace method

Which one? :)

[eduard93]

The second one. ##class(%SYS.Namespace).GetPackageDest($Namespace,"WebTerminal")``` only returns db path. The second one returns namespace

[nikitaeverywhere]

@eduard93, I was sure that I left a comment here saying that everything is ready but I cannot find it again, so:

I did the repository refactoring by 1bded7c11b4f82540648a2bd58fd90e50d5c5e2c, so now it is OK to go forward with WebTerminal.Engine class refactoring. Read the DEVELOPMENT.md notes also.

Thank you!

[eduard93]

I'm refactoring all right, see https://github.com/intersystems-ru/webterminal/tree/Refactoring branch. It's just currently refactoring broke my local webterminal, so I'm not commiting.

[eduard93]

Encountering clean install compilation problems

[eduard93]

Use DependsOn instead of CompileAfter seems to solve compilation errors on import See https://github.com/intersystems-ru/webterminal/commit/6d8de5f71c845fd3097df3505bb7a7cc7320c1a5

[nikitaeverywhere]

I have tested these changes and everything seem to be OK. The installer issue now has it's place here: #46. Thanks Edward!