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

Snapshots api #3833

Merged
merged 12 commits into from
May 8, 2024
Merged

Snapshots api #3833

merged 12 commits into from
May 8, 2024

Conversation

Steve-Mcl
Copy link
Contributor

@Steve-Mcl Steve-Mcl commented May 7, 2024

closes #3828

Description

This PR lays foundation for exporting device snapshots (part of #3617) for visualising snapshots (part of #3798) and goes some way to squaring up the snapshot vue pages differences as part of #3815 by providing a means to export a device snapshot

Details

  • A new API with 4 routes are added creating a top-level Snapshots API. The routes are:

    • GET /snapshots/:id - returns only the meta data of a snapshot
    • GET /snapshots/:id/full - returns the full data of a snapshot (without credentials)
    • DELETE /snapshots/:id - deletes a snapshot
    • POST /snapshots/:id/export - exports a snapshot
  • The pre-handler for these routes check if the user has the necessary RBAC permissions to perform the operations. These are:

    • snapshot:meta - 'View a Snapshot' - Viewer
    • snapshot:full - 'Export a Snapshot' - Member
    • snapshot:export - 'Export a Snapshot' - Member
    • snapshot:delete - 'Delete a Snapshot' - Owner
  • Tests are added to cover the new routes and the permissions they require. (see below)

  • A new controller Snapshots is added to handle these routes. Controller methods include:

    • getSnapshot - returns only the meta data of a snapshot
    • getFullSnapshot - returns the full data of a snapshot
    • deleteSnapshot - deletes a snapshot
  • Schemas in the ProjectSnapshot view are updated to support the new API:

    original schema new schema description what changed
    SnapshotSummary SnapshotSummary contains only meta data no changes
    Snapshot Snapshot contains a view of a full snapshot no changes but does now inherit the SnapshotSummary instead of duplicating fields
    SnapshotAndSettings contains full snapshot without flows or credentials. It is a base for FullSnapshot and ExportedSnapshot new schema
    FullSnapshot contains all of SnapshotAndSettings and adds flows new schema
    ExportedSnapshot ExportedSnapshot contains all of SnapshotAndSettings but adds flows and credentials Modified to inherit a base schema SnapshotAndSettings
  • Existing calls to export snapshot and delete snapshot updated to utelise the new snapshot controller as a single source of truth.

NOTE WORTHY

This PR includes a minor pattern change to how forge/db/views/ProjectSnapshot.js is exported/imported.
In essence, the module now exports the functions and has a new init routine to setup the app object.

This new pattern is a first step in improving type inference and ultimately DX
throughout our codebase. Essentially, the start of some technical debt cleanup.

Most importantly, this is NOT a breaking change and can be done incrementally.
It is my hope this is accepted and adopted to all views eventually.

Amongst the benefits, it provides accurate function signatures (no app object), better code
completion, instant code navigation (by CTRL+clicking on the function), code peeking (on hover), and more.

For now, this is a first step in the right direction. Local variables can be decorated with JSDoc
to provide accurate typings. e.g. /** @type {import('path-to-view.js')} */ const snapshotView = app.db.views.Snapshot.

Eventually, we will be able to properly decorate the app and the app.db.views objects so that offers
the dev accurate typings & completions to reduces context switching (no need to hunt down the function
when its signature is accurately offered as a suggestion).

Once all views are updated to this pattern, I want to apply the same pattern to the controllers.

The eventual goal is to have a reference-able objects that can be used to provide better
static analysis & simple refactoring possibilities.

Missing tests added (opportunistic/working in that area/quick wins)

  Audit Log > Application
    ✔ Provides a logger for application device snapshot created
    ✔ Provides a logger for application device snapshot deleted

Tests Added

  Audit Log > Application
    ✔ Provides a logger for application device snapshot exported

  Snapshots API
    Get snapshot meta
      instance
        ✔ Returns 404 for non-existent snapshot
        ✔ Non-member cannot get snapshot (43ms)
        ✔ Snapshot contains only meta data
        ✔ Owner can get snapshot
        ✔ Member can get snapshot
        ✔ Viewer can get snapshot
      device
        ✔ Returns 404 for non-existent snapshot
        ✔ Non-member cannot get snapshot
        ✔ Snapshot contains only meta data
        ✔ Owner can get snapshot
        ✔ Member can get snapshot
        ✔ Viewer can get snapshot
    Get full snapshot
      instance
        ✔ Returns 404 for non-existent snapshot
        ✔ Non-member cannot get snapshot
        ✔ Snapshot contains full data
        ✔ Owner can get snapshot
        ✔ Member can get snapshot
        ✔ Viewer cannot get snapshot
      device
        ✔ Returns 404 for non-existent snapshot
        ✔ Non-member cannot get snapshot
        ✔ Snapshot contains full data
        ✔ Owner can get snapshot
        ✔ Member can get snapshot
        ✔ Viewer cannot get snapshot
    Export snapshot
      instance
        ✔ Returns 404 for non-existent snapshot
        ✔ Non-member cannot export snapshot (39ms)
        ✔ Cannot export snapshot without new credentialSecret
        ✔ Snapshot contains export data
        ✔ Owner can export snapshot
        ✔ Member can export snapshot (39ms)
        ✔ Viewer cannot export snapshot
        ✔ Exports provided credentials (43ms)
      device
        ✔ Returns 404 for non-existent snapshot
        ✔ Non-member cannot export snapshot
        ✔ Cannot export snapshot without new credentialSecret
        ✔ Snapshot contains export data
        ✔ Owner can export snapshot
        ✔ Member can export snapshot
        ✔ Viewer cannot export snapshot
        ✔ Exports provided credentials
    Delete snapshot
      instance
        ✔ Returns 404 for non-existent snapshot
        ✔ Non-member cannot delete snapshot (40ms)
        ✔ Owner can delete snapshot
        ✔ Member cannot delete snapshot
        ✔ Viewer cannot delete snapshot
      device
        ✔ Returns 404 for non-existent snapshot
        ✔ Non-member cannot delete snapshot
        ✔ Owner can delete snapshot
        ✔ Member cannot delete snapshot
        ✔ Viewer cannot delete snapshot

Related Issue(s)

#3828

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

  • Includes a DB migration? -> add the area:migration label

@Steve-Mcl Steve-Mcl requested a review from knolleary May 7, 2024 14:48
Copy link

codecov bot commented May 7, 2024

Codecov Report

Attention: Patch coverage is 85.61151% with 20 lines in your changes are missing coverage. Please review.

Project coverage is 79.22%. Comparing base (fe2e18f) to head (10d55af).

Files Patch % Lines
forge/db/controllers/Snapshot.js 66.66% 14 Missing ⚠️
forge/routes/api/snapshot.js 90.32% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3833      +/-   ##
==========================================
+ Coverage   79.16%   79.22%   +0.05%     
==========================================
  Files         279      281       +2     
  Lines       12567    12660      +93     
  Branches     2792     2813      +21     
==========================================
+ Hits         9949    10030      +81     
- Misses       2618     2630      +12     
Flag Coverage Δ
backend 79.22% <85.61%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@knolleary knolleary left a comment

Choose a reason for hiding this comment

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

One minor tweak to the permissions descriptions to clarify the difference between the two - will self merge.

forge/lib/permissions.js Outdated Show resolved Hide resolved
@Steve-Mcl Steve-Mcl merged commit 9395ad5 into main May 8, 2024
13 checks passed
@Steve-Mcl Steve-Mcl deleted the 3828-snapshots-api branch May 8, 2024 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate a top level API for snapshots
2 participants