Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: extension support #743

Open
1 of 4 tasks
bpatrik opened this issue Oct 14, 2023 · 12 comments
Open
1 of 4 tasks

Feature: extension support #743

bpatrik opened this issue Oct 14, 2023 · 12 comments
Assignees
Labels
enhancement help wanted mian-milestone This a main feature of a future release

Comments

@bpatrik
Copy link
Owner

bpatrik commented Oct 14, 2023

This a tracking bug and "design doc" for the next mayor update that will be about extension support.

Requirement:

  1. Users should be able extend the app without rebuilding the application
    • app restart after a new extension install is acceptable
    • "extending the app" mean business logic changes on both server and client-side
    • extensions should also support at least some UI modifications
  2. Extensions should be configurable (should show in the settings)
  3. There should be a repository where users can easily download extensions from. Preferably integrated in the app.

Design ideas

Extension will live in their own folder, next to the config.json file. config folder is by default writable from the docker setup, so it should be easy to add there more files.
It will be written in Javascript (I hope its not a question :) ).

server side extension

Creating a server side extension is the easy part. The app will assume a presence of an server.js that has a function init(app) function. App will call the init function of all extensions on startup.
If the folder has package.json, the app will run an npm isntall so that the extension developers can use external libraries (like if someone wants to develop an extension to support bmp files).
The appobject in the init function will represent an interface to the app, where extension developers can subscribe to certain events of the app. These events will be, like: before directory scanning or after directory scanning. With this developers can override the input and the output of the directory scanning event. (So things like also indexing doc files or skipping files that start with _ can be added).
Furthermore app object will also export low level api. Low level API will be less stable than the exported events, but with that developers can override any aspect of the app without restriction.
app object will also export the currently used config.

client side extensions

This is the tricky one as the client side is built and minified and localized AOT with webpack. Therefore there is one build app for all language. Exposing the full client app to the extension is not an option as its is not possible to know in extension development time how webapck will minify the function names.
I also could not find a way to add new angular component to the app after compile. Eg.: So I do not see a way to add UI for supporting docx files on UI through extensions.

I was thinking about exposing callback for the client extension, where developers can add icons or buttons in a very restrictive way to the app. And also here could the developer inject some business logic to the client.

repository

I do not want to host a custom repository. I think I will just "hack" it into a readme file. Developers will maintain extensions on github and add a link to their repo to to a readme file in the repository. This way the app will be able to auto discover the list of extensions.

Future work

Security can be implemented in the future where the app makes it explicit what resources an extension uses. Like it changes the config or touches low-level APIs.

Open questions

I don't really know how to solve the client side extension support properly

Sub tasks

@bpatrik bpatrik added this to the Next (probably v2.5) milestone Oct 14, 2023
@bpatrik bpatrik self-assigned this Oct 14, 2023
@bpatrik bpatrik added help wanted mian-milestone This a main feature of a future release labels Oct 14, 2023
@bpatrik bpatrik pinned this issue Oct 14, 2023
@mblythe86
Copy link
Contributor

I like this idea a lot, and I think it has a lot of potential to serve as a bridge between what you're developing pigallery2 for (as you lay out in #738) and what other users are wanting.

As far as I can tell, the biggest philosophical difference I have is with this concept in #738:

Source of truth is the disk: DB is only a cache

In my particular case, there is an additional source of truth: the DigiKam DB. This is where I mark individual directories to be "Public" (i.e. displayed on pigallery2), and where I manually select directory covers/previews/thumbnails/highlights.

Allowing me to code up a custom extension that intervenes in the "Directory Scanning" and "Preview Filling" jobs would be pretty much a perfect solution.

bpatrik added a commit that referenced this issue Nov 19, 2023
@bpatrik
Copy link
Owner Author

bpatrik commented Nov 19, 2023

Added backend extension support (in alpha version). So far the app looks for extension subfolders in the /extensions folder.
to add an extension you can:

<pigallery src root>/extensions/myextension/package.js <- this is optional. You can add extra npm packages here
<pigallery src root>/extensions/myextension/server.js <- this is needed

sample server js:

/* eslint-disable @typescript-eslint/no-inferrable-types */
import {IExtensionObject} from '../../src/backend/model/extension/IExtension';
import {PhotoMetadata} from '../../src/common/entities/PhotoDTO';
import {Column, Entity, Index, PrimaryGeneratedColumn} from 'typeorm';
import {SubConfigClass} from 'typeconfig/src/decorators/class/SubConfigClass';
import {ConfigProperty} from 'typeconfig/src/decorators/property/ConfigPropoerty';
@Entity()
class TestLoggerEntity {
  @Index()
  @PrimaryGeneratedColumn({unsigned: true})
  id: number;

  @Column()
  text: string;
}


@SubConfigClass({softReadonly: true})
export class TestConfig {
  @ConfigProperty({
    tags: {
      name: `Test config value`,
    },
    description: `This is just a description`,
  })
  cleanUpUnusedTables: boolean = true;
}

export const init = async (extension: IExtensionObject<TestConfig>): Promise<void> => {
  extension.Logger.debug('Setting up');

// override metadata loader example
  extension.events.gallery.MetadataLoader
    .loadPhotoMetadata.before(async (input, event) => {
    extension.Logger.debug('skipping');
    event.stopPropagation = true;
    return {
      size: {width: 1, height: 1},
      fileSize: 1,
      creationDate: 0
    } as PhotoMetadata;
  });
  extension.config.setTemplate(TestConfig);
  console.log(JSON.stringify(extension._app.config.Extensions));

  // add new res API example
  extension.RESTApi.get.jsonResponse(['/hw'], null, async () => {
    const conn = await extension.db.getSQLConnection();
    conn.getRepository(TestLoggerEntity).save({text: 'logger entry' + Date.now()});


    return await conn.getRepository(TestLoggerEntity).find();
  });

// set own tables
  await extension.db.setExtensionTables([TestLoggerEntity]);
  
  
  // add messenger example
  extension.messengers.addMessenger('loggerMsg',
    [{
      id: 'text',
      type: 'string',
      name: 'just a text',
      description: 'nothing to mention here',
      defaultValue: 'I hand picked these photos just for you:',
    }],
    {
      sendMedia: async (c, m) => {
        console.log(m);
      }
    });
};

export const cleanUp = async (extension: IExtensionObject<TestConfig>): Promise<void> => {
  extension.Logger.debug('Cleaning up');
};

See https://github.com/bpatrik/pigallery2/blob/master/src/backend/model/extension/IExtension.ts for full extension API

@bpatrik
Copy link
Owner Author

bpatrik commented Nov 19, 2023

a more in depth documentation and easier usage comes later

@lukx
Copy link

lukx commented Nov 20, 2023

Kudos @bpatrik for this extensive new feature, I am loving it. Thank you for staying innovative!

@bpatrik
Copy link
Owner Author

bpatrik commented Nov 25, 2023

bpatrik added a commit that referenced this issue Nov 26, 2023
bpatrik added a commit that referenced this issue Nov 26, 2023
@mblythe86
Copy link
Contributor

This is great, thanks so much!

I've written a 'DigiKam Connector' plugin that (mostly) suits my needs: https://github.com/mblythe86/pigallery2-extension-digikam-connector

I'm relatively inexperienced at JavaScript/TypeScript development (my day job mostly uses shell/ruby/perl). If you would be willing, I'd really appreciate it if you could take a look over my code and give me some feedback. Am I following good TypeScript conventions? Do you think I'm using the extension API in a sane way? etc...

While creating this extension, I did hit a few speedbumps. I'll file separate issues (or pull requests) for the places where I think pigallery2 extension support could be improved, and with a question I have.

Thanks again!

@bpatrik
Copy link
Owner Author

bpatrik commented Dec 2, 2023

Had a quick look, this is very impressive @mblythe86! When I was creating the extension, I was not thinking about calling an external DB and doing decisions in the app :)

I think the code looks good. Probably I would have had the same approach. (In my work I also do not user Typescript, so 🤷‍♂️)

To answer you readme, the plan is to have a nice config surface (also unified one with the jobs/messenger), but that I run into some issues.

I'm curious hear about you dev exp. here or in an other bug.

bpatrik added a commit that referenced this issue Dec 2, 2023
@mblythe86
Copy link
Contributor

Thanks for taking a look!

the plan is to have a nice config surface (also unified one with the jobs/messenger), but that I run into some issues.

Yeah, implementing that sounds tedious. It's definitely something that I'd put off.

An quick-and-dirty improvement might be to just have the JSON pretty-printed in the text box, and have the text box auto-scale to the contents.

I'm curious hear about you dev exp

Overall, it was good!

I really like the before/after hooks that you have in the extension API. It makes it very convenient to either replace the implementation of the wrapped function (as I did with getCoverForDirectory), or just tweak the return value of the function a little without re-implementing the whole thing (as I did with scanDirectory).

There are a couple ways that the dev experience could improve, but none are showstoppers:

  • Extension re-loading
    • I found myself restarting my docker container over & over as I made gradual tweaks to my extension. It would be nice if there was a 'reload extensions' button on the admin panel. Or better yet, if it could auto-detect changes to extension files, and automatically re-load the extension (probably only in NODE_ENV=debug mode to avoid a performance impact)
  • Backtraces
    • When my code errored out, the backtrace was kind of cryptic due to the TypeScript -> JavaScript transpile:
      at Array.<anonymous> (/app/data/config/extensions/sample-extension/server.js:324:14)
      I think that the *.js.map file is supposed to allow for debuggers to map back to the original TypeScript code location. It would be nice if the backtrace mechanism could use that.

bpatrik added a commit that referenced this issue Dec 3, 2023
@bpatrik
Copy link
Owner Author

bpatrik commented Dec 3, 2023

Backtraces

Source file support is a good idea. Added js.map files to the release. Using them is disabled by default as random google searches suggests that it would slow down the app in prod: nodejs/node#41541, https://nodejs.org/docs/latest-v18.x/api/cli.html#--enable-source-maps

It can be enabled with env NODE_OPTIONS=--enable-source-maps

@bpatrik
Copy link
Owner Author

bpatrik commented Dec 3, 2023

Yeah, implementing that sounds tedious. It's definitely something that I'd put off.

The app config is one of the most complex part of the app. I might have overcomplicated it. But now it supports declarative config creation and UI generataion which is awesome, but it has some blind spots.

I'm long thinkg about rewriting some part of it, but I can't find the will poiwer to do so as its not the fun part of the app :)

I really like the before/after hooks

Note: after hooks parameter list slightly changed yesterday in a beaking way. see aa4c8a2

Extension re-loading

I see waht you mean. changing any config through the UI reloads the extensions. That would be a short term workaround. In longer term, I was not thinking about a reload button, but an enable disable button that would basically also do a relaod (because of the config change aspect of it)

@Tofee
Copy link

Tofee commented Jun 23, 2024

Hi!
Is the status of this feature up-to-date? Looking around, it looks like most of it is now implemented. Or are there still remaining blocking points before issuing a new release?

@bpatrik
Copy link
Owner Author

bpatrik commented Jun 27, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted mian-milestone This a main feature of a future release
Projects
None yet
Development

No branches or pull requests

4 participants