guacamole-client icon indicating copy to clipboard operation
guacamole-client copied to clipboard

GUACAMOLE-1085: Migrate frontend to angular

Open leonard2901 opened this issue 1 year ago • 18 comments

This PR includes the current progress for the migration of the AngularJS frontend to Angular. There is still a lot of work left to do, but I think now would be a good time to get some feedback on the code.

Overview

The new frontend consists of one Angular app and two Angular libraries:

  • guacamole-frontend

    • contains the main application
  • guacamole-frontend-lib

    • contains components that could be useful when building a custom Guacamole-Angular application
  • guacamole-frontend-ext-lib

    • contains services and classes for extensions that provide a clear interface for registering additional routes and field types

The application and both libraries are included in the maven build.

TODOs:

  • [ ] any changes and new features after revision 466476412a8a93440bea1121f374e17021380f25 (2023-06-02)
  • [x] migrate import module
  • [x] migrate index/services/iconService.js
  • [x] migrate manage/directives/connectionPermissionEditor.js
  • [x] migrate manage/controllers/manageSharingProfileController.js
  • [ ] caching of API responses
  • [ ] refactor the managed client so that reusable code can be moved to the guacamole-frontend-lib
  • [ ] some documentation in the code is missing
  • [ ] TODOs in the code
  • [x] code style might not be correct all the time
  • [x] some styles are not applied properly since Angular's components introduce additional DOM elements
  • [ ] the functionality to serve complex extensions from the Java code
  • [ ] guacamole-common-js should be copied to the frontend in the build process. At the moment I just included a copy at projects/guacamole-frontend-lib/src/assets/. Are there any plans to release an official NPM package for guacamole-common-js?
  • [x] type declarations for guacamole-common-js (projects/guacamole-frontend-lib/src/lib/types/Guacamole.ts). They will be replaced by the package @types/guacamole-common-js once it is updated.
  • [ ] automatic generation of the LICENSE and NOTICE files in the maven build. The angular build creates a 3rdpartylicenses.txt file, but I don't really know what to do with it. Any comments on this will be appreciated.
  • [ ] Do not assume that the application is always deployed at /guacamole/

Extensions

The biggest challenge in the migration was to build an extension system in Angular that was as flexible as the one in AngularJS.

  • Adding and modifying translation strings was mostly handled by the REST API and should work like before.
  • Inject additional HTML and CSS should work just like in AngularJS.
    • See apply-patches.service.ts and style-loader.service.ts in projects/guacamole-frontend/src/app/index/services/
  • More complex extensions can now be implemented as a separate Angular application
    • Extensions are loaded as a remote module via webpack module federation and the module federation plugin for Angular. Routes and form field can be added by implementing a bootstrap function. Adding custom Angular components to arbitrary locations will also be possible.
    • See doc/guacamole-frontend-extension-example for an example.
    • Note: the module federation configuration file (at the moment hard coded as projects/guacamole-frontend/src/moduleFederation/mf.manifest.json ) should be generated by the Java server in the future

Implementation notes

  • AngularJS services that mostly serve as data containers are replaced by simple classes with instance or static methods. Some of the more complex services are replaced by Angular services.
  • Named functions are mostly replaced by arrow functions.
  • Functions that return a promise with data from REST API are mostly replaced by functions that return an RxJS observable.
  • AngularJS's directives are replaced by Angular...
    • components if they are configured with restrict: 'E',
    • directives if they are configured with restrict: 'A'.
  • All styles are defined globally so that they can be overritten by extension styles
  • The authentication service method to perform HTTP request with the current token (AuthenticationService.request()) is replaced by a HTTP interceptor (auth/interceptor/authentication.interceptor.ts).
  • The error handling of the HTTP requests (rest/services/requestService.js) is replaced by a HTTP interceptor (rest/interceptor/error-handling.interceptor.ts).
  • The configuration of the $http service (httpDefaults.js) is replaced by an HTTP interceptor (index/config/default-headers.interceptor.ts).
  • To disable certain interceptors for specific requests, the HttpContextTokens in the InterceptorService can be used.
  • angular-translate is replaced by transloco (https://ngneat.github.io/transloco/).
    • A noticeable difference is that transloco uses double curly braces by default to link to other translations keys instead of the @: prefix. However, I change it to @:...:@ to prevent transcloco from removing parts of ICU messages (https://github.com/ngneat/transloco/issues/621).
  • I refactored the GuacFileBrowser to simply use an *ngFor loop. In my tests I didn't notice any performance issues.
  • I added Cypress for E2E Tests in the cypress folder. The tests are not integrated in the build process, and I did not include a proper configuration file because of the various possible testing setups.
  • I removed workarounds for IE since Angular itself does no longer support IE. Should these workarounds still be kept?
    • TunnelService~uploadToStream
    • TunnelService~downloadStream
    • UserCredentials.getLink
    • UserCredentialService~getLink
  • To replace the $parse function of AngularJS I used the NPM package angular-expressions which is a copy of the AngularJS code as standalone module.
  • There are some more questions directly in the code marked as a TODO.

Running the code

For my setup the easiest way to run the code was to build the frontend with maven and use the war file in a docker-compose setup. Since the module federation config is hard coded for now there will be some errors when the example extension (doc/guacamole-frontend-extension-example) is not running (ng serve) at localhost:4202. If the extension is running, the content will be available at /extension-example.

It is also possible to run the frontend using ng serve and benefit from live reloading with a way to work around the same origin policy (modify HTTP headers/proxy/disable web security). Therefore, the libraries need to be built manually: ng build guacamole-frontend-lib and ng build guacamole-frontend-ext-lib. Additionally, the following code might be necessary to redirect the requests to the REST API:

// projects/guacamole-frontend/src/app/auth/interceptor/authentication.interceptor.ts
if(!request.url.startsWith('assets'))
    request = request.clone({url: `http://localhost:8080/guacamole/${request.url}`});
// projects/guacamole-frontend/src/app/client/services/managed-client.service.ts getInstance()
  tunnel = new Guacamole.ChainedTunnel(
      new Guacamole.WebSocketTunnel('ws://localhost:8080/guacamole/websocket-tunnel'),
      new Guacamole.HTTPTunnel('http://localhost:8080/guacamole/tunnel')

When using ng serve, the REST API from this branch should still be used.

To run the Cypress test I used the following configuration to disable web security for Chrome:

// guacamole/src/main/guacamole-frontend/cypress.config.ts
e2e: {
  setupNodeEvents(on, config) {
      on('before:browser:launch', (browser, launchOptions) => {

          if (browser.family === 'chromium') {
              launchOptions.args.push('--disable-web-security');
          }

          return launchOptions;
      })
  },
  baseUrl: "http://localhost:4200"
}

leonard2901 avatar Jun 30 '23 23:06 leonard2901

:open_mouth:

Thanks, @leonard2901!

mike-jumper avatar Jul 01 '23 00:07 mike-jumper

Are there any plans to release an official NPM package for guacamole-common-js?

If it should be done or needs to be done, then yes - absolutely.

mike-jumper avatar Jul 01 '23 00:07 mike-jumper

Are there any plans to release an official NPM package for guacamole-common-js?

If it should be done or needs to be done, then yes - absolutely.

That would be great. Then it would be possible to also publish the Angular libraries to NPM and further simplify the creation of extensions and other applications.

leonard2901 avatar Jul 03 '23 08:07 leonard2901

Any feedback is appreciated - not limited to Mike of course. If anyone gave it a try (build/use/look-into), please post your comments.

everflux avatar Jul 12 '23 20:07 everflux

I tried using ng serve to deploy the webapp but it seems to have problems loading a module. Is this expected? image

jmuehlner avatar Jul 28 '23 17:07 jmuehlner

I deployed the app at /guacamole/ and most things seem to run well, with a few exceptions (e.g. share link dropdown is broken, broken translations on text input buttons).

Super cool!

jmuehlner avatar Jul 28 '23 18:07 jmuehlner

I tried using ng serve to deploy the webapp but it seems to have problems loading a module. Is this expected? image

Thanks for taking the time to look at this! From the screenshot it looks like there are two problems with your setup:

  • The Angular app can't find the example extension (doc/guacamole-frontend-extension-example) which is expected to be running on port 4202. This is currently hard-coded to show how extensions can be loaded in Angular without having to touch the Java code of the server.
  • If you run the app with ng serve, all requests will be made against localhost:4200 by default. Please see the last section of the PR description for a workaround for this.

leonard2901 avatar Aug 02 '23 12:08 leonard2901

Hi! Thanks for your feedback so far. I’m working on finishing the remaining todos, and I wanted to ask if, once they’re done, it’s realistic to aim for a merge. Appreciate your thoughts!

leonard2901 avatar Oct 27 '23 17:10 leonard2901

I think so. It's quite a huge change, to be sure, and all the more reason to avoid too much long-term divergence.

Moving to Angular from AngularJS will mean a full version bump to 2.0.0, but that's OK. We can figure out a branching scheme that won't make maintenance of 1.5.x, upcoming 1.6.0, etc. impossible.

mike-jumper avatar Oct 27 '23 17:10 mike-jumper

That sounds great.

Currently guacamole-common-js is provided here as an npm package. Type declarations for TypeScript are provided here.

With the transition to Angular and TypeScript, it would be nice if guacamole-common-js was offered as an "official" package at npm. @mike-jumper was already positive about this idea. What could be the procedure for realizing this?

leonard2901 avatar Nov 04 '23 12:11 leonard2901

With the transition to Angular and TypeScript, it would be nice if guacamole-common-js was offered as an "official" package at npm. @mike-jumper was already positive about this idea.

Ah, yes - that guy. ;)

What could be the procedure for realizing this?

The build would need to be altered however necessary to produce that package and consume it.

The current Maven-based process is:

  1. The guacamole-common-js Maven project produces a .zip artifact consumable by other Maven projects.

  2. The guacamole Maven project consumes the guacamole-common-js artifact as part of the overall guacamole-client build.

  3. When a release candidate is being produced, the guacamole-common-js artifact is pushed to a staging Maven repository.

    NOTE: It's important here that the artifact for a release candidate is the same as the artifact for the actual release. Its version number doesn't change, the binary doesn't get rebuilt; it gets staged, tested, and when the release passes vote its promoted directly without any modification/regeneration/rebuilding of any kind.

  4. When a release candidate is being promoted to release, all staged artifacts (including guacamole-common-js) are released and get automatically synced to Maven Central.

An NPM-based process would need to be:

  1. The guacamole-common-js Maven project produces an NPM artifact consumable by other NPM projects (and Maven projects that use NPM via plugins). If possible, we should also still produce a Maven artifact.

  2. The guacamole Maven project consumes the guacamole-common-js artifact as part of the overall guacamole-client build, whether via an NPM artifact or a pure Maven artifact.

  3. When a release candidate is being produced, the guacamole-common-js artifact is staged to NPM (somehow) as a release candidate. If we still produce a Maven artifact, that would be pushed to staging, too. As with the pure Maven process, it would be important that the staged artifact(s) are always the artifact(s) that are released - the release of a release candidate shouldn't involve rebuilding and potentially producing something that isn't what was tested as the release candidate.

  4. When a release candidate is being promoted to release, all staged artifacts (including guacamole-common-js) are released and get automatically synced to Maven Central, NPM, etc.

mike-jumper avatar Nov 04 '23 18:11 mike-jumper

In addition to that: Perhaps it would be a good idea to reserve a good name as 'guacamole' as package seems to be taken https://www.npmjs.com/package/guacamole There is an organization level naming schema as well, so "@guacamole" or "@apache-guacamole" might be useful to save for this use case.

everflux avatar Nov 05 '23 19:11 everflux

@everflux Sadly, it looks like the guacamole name is already taken at the org level, so apache-guacamole or a similar variant might be all that's possible (unless the owner of that org/username is willing to relinquish it):

https://www.npmjs.com/org/guacamole

As for the package itself, I think a reasonable path forward might be to get in contact with @padarom, the individual that produced and maintains an NPM-ified fork of guacamole-common-js:

https://www.npmjs.com/package/guacamole-common-js

If we can take over ownership and maintenance of that package within NPM, then existing users of the established NPM package would be able to continue using it without interruption, and former users of the original pure-Maven package looking to migrate to NPM would have the benefit of familiar package naming.

@padarom, any thoughts?

mike-jumper avatar Nov 06 '23 17:11 mike-jumper

The intention of my fork was to make it easier for people to use Guacamole in their JS projects, so if there ends up being an official Apache package doing the same, then my fork can be seen as obsolete.

As long as the official npm package is compatible, I'm fine with transferring ownership. If I understood correctly the plan seems to include a major version bump, so even if it isn't quite compatible some breakage could be expected by the package's current user base anyways.

GitHub's diff viewer isn't being nice to me with this large PR and I'm currently on the go, but from the PR description it seems like the regular Guacamole frontend is rewritten in Typescript, whereas guacamole-common-js is not. Are type definitions still expected to be provided via @types/guacamole-common-js? If I could make the recommendation to also think about including (and bundling) types directly in the official package, rather than having users manually add another package that has to be separately maintained?

padarom avatar Nov 06 '23 19:11 padarom

If I could make the recommendation to also think about including (and bundling) types directly in the official package, rather than having users manually add another package that has to be separately maintained?

I agree.

mike-jumper avatar Nov 07 '23 23:11 mike-jumper

If I understood correctly the plan seems to include a major version bump, so even if it isn't quite compatible some breakage could be expected by the package's current user base anyways.

I see no immediate reason why my changes to the frontend should lead to breaking changes in the JS lib.

If I could make the recommendation to also think about including (and bundling) types directly in the official package, rather than having users manually add another package that has to be separately maintained?

I also agree. However, to avoid making this PR even bigger, I would suggest that this should be done as a separate task after this PR has been merged

leonard2901 avatar Nov 08 '23 09:11 leonard2901

@leonard2901: when you are planning to release your guacamole-client-angular as a npm package?

suncase avatar Mar 14 '24 21:03 suncase

@suncase Thank you for you interest in the Angular client. As you can see in the description of the PR, there are still a few things I need to work on before this could possibly be merged. Unfortunately, I currently have less time for the project than I would have liked. So I can't tell you when this will be merged.

That being said, I don't think the frontend of the Guacamole Client will be released as an npm package. My idea is to publish the packages "guacamole-frontend-ext-lib" and "guacamole-frontend-lib" on npm to facilitate 1) the creation of extensions for the frontend and 2) the creation of custom Guacamole-Angular applications. If I have overlooked a use case, please feel free to describe it.

leonard2901 avatar Mar 15 '24 07:03 leonard2901