Plugin

[AndiLavera]

Description of the Change

Plugin support for Amber. 90% of the code is from @damianham so credits go to him. This version is very different than his though, so it felt justified making a PR directly on Amber instead of making a PR to his version.

This version does not fetch repos like the recipe command. This searches locally. We can let "shards" handle fetching while we just have to install some files.

Plugin authors are expected to have a "plugin" folder at the top level (maybe 'src/' would be better?). The CLI takes a command like mochi:setup. The Fetcher will look for the directory "./lib/mochi/plugin/setup". Doing this allows for plugin "stages"/"modules". Multiple plugins can sit in a single repo. This works well for big shards that have multiple modules.

Note on "stages": The goal is not to encourage multiple different plug ins under one repo. It's to stage installation for big shards. For example, with my authentication gem Mochi, we have modules such as authenticable, invitable, and lockable. With such a set up, users of Mochi can install only the modules they want to use.

My approach comes from different perspective. I believe a plugin author should have the choice between a monolithic or modular repo structure & plugins code should be modified to fit user needs.

This implementation does support both monolithic & modular repos. Authors can make a monolithic structure by putting the "plugin" folder at the top level of their shard. Or they can make the plugin modular by creating a separate repo with that plugin shard having the main shard as a dependency.

I also disagree with the idea that users would not be modifying plugin code. For example, the files I generate for the user in my shard Mochi is made to be configured. I generate an initializer which exposes several settings for the user. I also generate views & controllers which I expect to be modified at least a little bit. For this reason alone, I don't believe a "plugins" folder and separation of concerns is necessary. I feel plugins should install into the users project under src/ or config/ or where ever those files would belong.

Usage

Install a shard like you normally would in the shard.yml file, run amber plugin install shard_name:stage_name.

Alternate Designs

1197

Benefits

This section more of a compare & contrast of #1197 - @damianham if i get anything wrong, please let me know

• 1197 only supports plugins on github & the master branch

  • Shards does not have this limitation. Changing the fetcher to local circumvents this limitation

• 1197 does not support stages. Authors would be required to make multiple repos to 'stage' installs

• Lighter implementation, easier to debug

• Personally, I don't like the whole symbolic linking & hiding folders. I now understand why it looks like infinitely recurses. However, I think that will confuse a lot of new comers. I imagine a FAQ would have to be made just for that. This implementation does not use any symbolic linking.

Possible Drawbacks

Should be the same drawbacks as #1197 . I will copy & paste for easy viewing:

A plugin author may not namespace their plugin routes and classes, although this should be a mandatory requirement in the plugin documentation (coming soon).

A plugin route/class namespace may clash with an application's existing namespaces. An Amber application develop should read the plugin documentation in the plugin README.md to ensure they are aware of the namespace used in the plugin to ensure installing it does not clash with their existing namespaces.

Things to do

• [ ] Write Specs - Skipped for now just to see what people think of this implementation
• [ ] Write Docs - Would be written if this was going to be merged

[drujensen]

Thanks for this @andrewc910! I always disliked fetching the repository instead of using the shards built-in support. Also happy to see the recursive symlink removed. 💯

[damianham]

There are certainly merits to this approach and in particular importing the shard in top level shard.yml will allow the developer to use different branch or specific version other than master. So in the cases where this might be required I can see the advantage of that. As a web application developer and potential consumer of future plugins I would be concerned that a plugin that generates artifacts in the regular 'src' tree may conflict with existing code artifacts so I made the decision to use a specific 'plugins' folder for imported plugins to ensure there is no conflict. Given that in my implementation a plugin generates artifacts into it's own folder within 'plugins' there is no conflict with artifacts in src or with other plugins.

Maybe I was being to rigid when I said that users would not be modifying plugin code. Since it is easily accessible it is easily modifiable and of course I am modifying the mailer code between my different projects that use the plugins. I provide for hooks and initializers in the 'src' tree for user modifications which may suffice - otherwise a developer would delve into the plugin code.

My implementation uses a shard.yml in .plugins and then runs shards install to install the plugin. Any symbolic link is created by the shards command. I am not sure thee is a need to explain the hidden folders in a FAQ. It is black box that does the job and just like the .git folder I don't think developers need to be that concerned with the contents of hidden folders once the plugin has generated the code.

I don't think I am a fan of amber plugin install shard_name:stage_name. This implies that plugins have to have stages which will not be true for many if not most plugins. We don't know that yet until there are many more plugins developed. The authorize plugin I developed generates the confirmable, invitable, lockable, recoverable, trackable stages but if you don't include them in src/models/user.cr and remove the routes then they don't get compiled into the binary.

Like anything else we need to fiddle with this until we are happy with the implementation so kudos for creating an alternative to carry on the discussion.

[AndiLavera]

Yeah, that is one problem putting files directly into the src/ folder. What I am thinking is the plugin folder will ultimately become another src/ file. You install 3 plugins, this one uses a model, the second uses a controller & the third uses some views. You now just recreated the src hierarchy in another top level folder.

What if we inserted the files into src under a folder that's named after the plugin? This is closer to how devise looks after generation. src/controllers/devise/devise_controllers_here or src/views/devise/devise_views_here. It would prevent the recreation of src under a new folder but also safeguard against overwriting files. To safeguard, in the Installer we ensure controller files go into controllers/plugin_name/plugin_controller_files. If we handle adding the additional folder, plugin authors wouldn't have to do it & we prevent files from being generated in places they shouldn't. The conversion of files would look like:

Before install: plugin_repo/plugin/controllers/my_controller.cr

After install: amber_project/src/controllers/plugin_name/my_controller.cr

I agree with you that people shouldn't go mucking about in the .plugins folder but i believe they will. At the very least, the question "What is this .plugins folder" will come up in the gitter quite a bit. I am always on the side of implementing something that won't raise any questions.

You are totally correct on the plugin_name:stage. That was definitely an oversight. I will change that. A plugin can be installed plugin_name or plugin_name:stage is the goal.

[damianham]

Yeah, that is one problem putting files directly into the src/ folder. What I am thinking is the plugin folder will ultimately become another src/ file. You install 3 plugins, this one uses a model, the second uses a controller & the third uses some views. You now just recreated the src hierarchy in another top level folder.

This isn't an issue for me as I have organised plugins into a modular structure so for example

plugins/authorize/invitable/invitable_controller.cr
plugins/authorize/invitable/invitable.cr
plugins/authorize/invitable/new.slang
plugins/authorize/invitable/accept.slang
plugins/authorize/invitable/invitations.slang
plugins/authorize/invitable/_hooks.cr

So the model, controller and views are in the same folder (that's the way it rolls with the modular structure) and it is the same for the other features of the authorize plugin.

What if we inserted the files into src under a folder that's named after the plugin?

That would be OK except that you would have to add a line to config/application.cr for each plugin to load it in. Currently config/application.cr loads specific folders in src. My PR has the same issue though, you have to add a line to root/plugins/plugins.cr to load each plugin. But fundamentally I prefer this approach to generating fles all over the source tree.

Before install: plugin_repo/plugin/controllers/my_controller.cr

After install: amber_project/src/controllers/plugin_name/my_controller.cr

I am a modular fanboy so I don't like this approach. I prefer amber_project/src/plugin_name/my_controller.cr

I agree with you that people shouldn't go mucking about in the .plugins folder but i believe they will. At the very least, the question "What is this .plugins folder" will come up in the gitter quite a bit. I am always on the side of implementing something that won't raise any questions.

Well we could have a root/.plugins/Readme.md which says;

this is a temporary folder used during the installation of plugins - you don't need to worry about it's contents

:)

[AndiLavera]

I think at this point it would be best to have the amber core team chime in on some of the friction before we can go about implmenting a proper solution.

The two biggest questions I see @damianham & I discussing are:

• Should the plugin CLI fetch a repo or should the files be installed locally?

• What should the file hierarchy be? Should plugin files be integrated into the main project or should they be contained in a modular fashion?

@damianham I think we could go back and forth on this one. I see the merits in your design on the file hierarchy but i do prefer mine. At this point, I think a core member should decide the spec. I personally dont have enough experience in web dev to have an authoritive opinion. I like my ideas but i am not sure if they are whats best. My PR was just to open the dicussion on some ideas!

[drujensen]

I prefer that all the source code exists in the src folder.

what if we use src/plugins/{plugin-name}/...?

In theory, this would support both modular and non-modular plugins. I prefer modular when the product is going to have hundreds of very discrete and separate components. Otherwise, I prefer the Rails layout better. would be nice if we could support both.

[eliasjpr]

Should the plugin CLI fetch a repo or should the files be installed locally?

Here are my two cents:

• Use shards as we normally do for fetching the plugin
• Use amber install plugin or amber generate plugin_name for plugin code installation and/or generation

What should the file hierarchy be? Should plugin files be integrated into the main project or should they be contained in a modular fashion?

I believe the self contained plugin directory is best at the moment, with that said I am okay with this approach src/plugins/{plugin-name}/

[AndiLavera]

Just to put everything in one place, this is what I have gathered:

1. Install Command: amber install plugin_name
2. Location: src/plugins/{plugin_name}/
3. Fetching: Shards will fetch the shard/plugin, amber CLI only moves/processes files

Questions:

1. For install, do we want to drop the : for submodules? I think amber install plugin & amber install plugin:submodule would be more flexible but I will drop it from the code base if we don't want it.
2. This one is less about code & more about documentation Will the file hierarchy in the plugin be ...plugins/{plugin_name}/{all_files_here}? I.E. Dumping all controllers, views, models, etc in the same folder. Or do we want {plugin_name}/controllers/, {plugin_name}/views/, {plugin_name}/models/. I don't think it matters to the Amber framework. I think Amber would just find the files regardless if they are in subfolders of the plugin. However, I believe we should encourage plugin authors to follow a specific hierarchy. This would just be in the documentation on how to create a plugin.

[drujensen]

1. For install, do we want to drop the : for submodules?

Yes. Crystal will only compile what is included so it's better to use that mechanism instead of retrieving only submodules.

2. Will the file hierarchy in the plugin be ...plugins/{plugin_name}/{all_files_here}?

I think we need this to be flexible. I can see some plugins that don't require the file structure and others that want it laid out in other ways. For the documentation, I would recommend it uses some type of SOC file structure. This may be in reverse order i.e. users/Controller.cr instead of controllers\User.cr but it's a good practice if you have a lot of files with different concerns.

[AndiLavera]

Okay, it should be ready to merge. Please review and let me know what you think.

What has changed: I removed the submodules. Now the installer pretty much just clones shard/plugins into my-amber-project/src/plugins. I will write documentation encouraging authors to follow a certain hierarchy that includes SOC, however, the installer doesn't enforce anything. I am not sure how we would, TBH.

I added the one spec we needed from @damianham PR. I would like to test Amber::Plugins::Installer but IDK how to test a Teeplate class. I will let the core dev's decide what to do on this one.

Current flow for users: Add shard to shard.yml $ shards install $ amber plugin install shard_folder_name (I say shard folder name because sometimes the shard name doesn't match folder name that gets cloned. IE: Some shards are named with .cr but the folder excludes this)

How to manually test:

1. Clone this branch
2. Build
3. Create a new amber project
4. Copy some folder with a plugins folder into new projects lib folder
5. Run amber plugin install folder_you_added_to_lib It should clone the files in your_shard/plugins into src/plugins/. This currently works on my machine!

Requirements not currently met: @eliasjpr wanted the command to be amber install plugin_name. It is currently amber plugin install plugin_name. plugin being the cli command, install being an argument. I prefer this because we can eventually add an uninstall to remove those files. If this is not satisfactory, lmk and i will remove it thus making it amber plugin plugin_name. I can also change the cli command to install instead of plugin if that is preferable however I don't believe the class name Install is self-explanatory. Just lmk how to move forward!

[eliasjpr]

@andrewc910 I would prefer for the simplicity and usability of amber plugin {plugin_name} to use a flag amber plugin -i {plugin_name} or maybe simply default to install amber plugin {name} in the case of needing to support multiple flags. Note that there cloud be a flag for uninstall -u or --uninstall

@amberframework/core-team what do you think?

[AndiLavera]

@eliasjpr I like that better. amber plugin plugin_name, defaults to install and eventually add a -u flag for uninstall. I can get that done tomorrow!

[AndiLavera]

The command is now amber plugin plugin_name. -u is the flag to uninstall. I plan to build uninstalling functionality in a separate PR after this gets merged.

[eliasjpr]

Ping! @amberframework/core-team

[AndiLavera]

Any update on this? If there is a problem, please let me know and changes will be made.

There is a feature I would like to add but would prefer doing so in a different PR to not hold this up. I am thinking allowing plugin authors to create yml file for configuration or extra behavior. For example, a plugin author could defined routes which we would automatically generate and insert into the users route file. I imagine a lot of time, discussion and thought should go into this hence why I think a separate PR would be best.