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

next round of refactoring #35

Merged
merged 17 commits into from
Oct 12, 2023
Merged

next round of refactoring #35

merged 17 commits into from
Oct 12, 2023

Conversation

line-o
Copy link
Member

@line-o line-o commented Oct 4, 2023

fixes #12

Features

  • Bump package version to 2.0.0 to reflect breaking changes and declare max processor version 6.x.x
  • rename blacklist to ignore in configuration and throughout the code 1229819
  • Backup tuttle configuration on deinstall and restore on installation if found. Will resort to commented example configuration if no backup is found. This is usually the case on clean installs. This feature has only been tested locally yet. But it's very reliable as my local testing workflow relied on it. cbeaa09
  • A lot of behind the curtains optimisations to further reduce duplicated code.

Fixes

  • Module namespaces all start with http://e-editiones.org
  • Tests have been adapted, improved and extended
  • drop unused dependency on shared-resources

fixes #12

When tuttle is updated the configuration file,
`/db/apps/tuttle/data/tuttle.xml` is backed up to
`/db/tuttle-backup/tuttle.xml`.
After the installation is finished either
- a backup is found an restored and the backup collection removed
- no old configuration was found and the example is used (copied from
  `/db/apps/tuttle/data/tuttle-example-config.xml`)
- added cleanup.xq (runs before deinstallation)
- post-install.xql was renamed to finish.xq (runs after installation)
- declare max processor version exist-db 6.x.x
- drop unused dependency on shared-resources
- formatting
- fetching repository state to staging collection
- deploying state from staging collection to production collection
- incremental update of production collection
- add return types
- add collection module
- rename github:clone and gitlab:clone to :get-archive as that
  describes what is happening
- move duplicate logic from the above functions to the api module
- minor reformatting in config module
- improve, adapt and extend tests
- replace dbutil:scan (shared-resources) with collection:scan
- set permissions to resources as well
- resources that belong to `nobody/nogroup` are invisible to `admin/dba`
  the example permissions are therefore set to `admin/dba` and
  mode is `rw-r----` which is quite restrictive but safe.
    The comment in the example configuration changed to reflect that.
    To test that permissions are applied `guest/guest` or `SYSTEM/dba`
    are options.
@line-o line-o requested review from duncdrum and JoernT October 5, 2023 20:33
Copy link
Contributor

@duncdrum duncdrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we come up with something a bit more flexibel for the collection name of the backup files? some minor comments. All in all really good stuff. Can you also add a note on the readme about the v7 known incompatibility. We could now set allow-fail on CI for as long as we restrict installation on exist v7
Is this still a TODO ? https://github.com/eeditiones/tuttle#db-to-git


(: TODO: $target is not set in cleanup phase :)
declare variable $configuration-collection := "/db/apps/tuttle/data/";
declare variable $backup-collection := "/db/tuttle-backup/";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comment on mattermost, I d really prefer a collection name that is more flexible for other apps as well, and won't lead to one collection per app with similar demands. so far we only have system and app following the db root collection. Lets create a third location for this kind of backup files

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do favour a broader solution as well. Please understand that this is not the right moment to set de-facto standards and the collection in this case is very short-lived. The entire /db/tuttle-backup is removed after installation.
And I would also prefer to discuss a general solution within the community not just among us.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happy to discuss a general solution with the community. I just don't see what we gain there, by adopting a single use solution now, instead of one that is open to extension

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we choose a collection name that is
a) the one other apps will be using then it needs extra protection from removal and will have to stay there even if unused
b) not the one we are choosing here, the code has to be changed anyway

src/modules/api.xql Outdated Show resolved Hide resolved
src/modules/api.xql Outdated Show resolved Hide resolved
src/modules/app.xql Outdated Show resolved Hide resolved
@line-o
Copy link
Member Author

line-o commented Oct 5, 2023

@duncdrum Please be aware that there are much much more pressing issues than comments at this point. We will get to those eventually. I want to first get the main issues done.

- hide `index.html` from URL
- allow only get requests for most non-api routes
- prevent exposure of the configuration file as it might contain
   sensitive data
- move all static resources to `resources`-collection
- requests to static resources (css/js/images) are routed to the
   `resources`-collection, which cannot be accessed otherwise to
   prevent directory-listings
- remove unused /jwt route
@line-o line-o requested a review from duncdrum October 5, 2023 22:55
src/controller.xql Outdated Show resolved Hide resolved
@line-o line-o merged commit 8bbe9fd into master Oct 12, 2023
4 of 6 checks passed
@line-o line-o deleted the fix/12 branch October 12, 2023 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updating tuttle overwrites configuration
2 participants