Decouple projects from storage API#5831
Open
beasteers wants to merge 4 commits into
Open
Conversation
565b9a4 to
2a36203
Compare
Member
|
Hey @beasteers - thanks for this. As discussed in slack, I can't promise how soon we'll be able to properly review this given the scale of change. It's a good proposal for sure - just a lot to work through. |
Pure file moves ahead of the projects/storage decoupling, updating the require paths/references that point at the relocated modules: storage/localfilesystem/projects/* -> projects/git/* No behaviour change.
Pull the repeated per-method runtime mocking into createRuntime() / rejectedError() / assertRejects() helpers and a data-driven table for the pass-through methods. Behaviour-preserving: same coverage, still exercising the storage.projects API.
Extract project versioning out of the localfilesystem storage module into a runtime-level service driven by a pluggable storage seam: - runtime/lib/projects: type-agnostic project service; storage declares its project type + flow layout via getProjectType()/getFlowLayout(); api, flows and nodes talk to runtime.projects instead of storage.projects. - storage/localfilesystem/flows.js: the single-file flow layout (read/save + namespaced layout.project.* write path) the built-in git type drives. - projects/git: the git project type and Project model, write path delegated to the layout. api/projects tests: createRuntime() switches storage.projects -> projects and available() moves to a delegated call (prior commit's helper refactor makes this a small change).
2a36203 to
253159a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tl;dr - Projects can now work with custom storage providers; the important changes are in the third commit; the design choices are concentrated in
storage/localfilesystem/flows.jsandprojects/git/Project.js- the rest is just scaffolding around that.Proposed changes
This PR moves project orchestration out of the
localfilesystemstorage implementation and makes it a runtime service (runtime.projects), so project management is no longer tied to the default single-file flow storage layout. Storage APIs can now provide Project lifecycle methods that perform project storage operations such as initializing or migrating to a project, recording path settings in package.json, and listing tracked files. In addition, it is also implemented with swappable "Project Types" so that git could be substituted for something else (e.g. object storage).Previously,
storage/localfilesystem/projectsmixed together three separate concerns:This change separates those responsibilities into dedicated modules:
runtime/lib/projects/index.jsThe top-level runtime projects service. similar API to before but has pluggable project types (e.g. git)
runtime/lib/projects/git/index.jsThe built-in
gitproject type implementation. It owns project lifecycle behavior and delegates git operations toProject.js.runtime/lib/storage/localfilesystem/flows.jsThe canonical single-file flow layout. It owns
getFlows,saveFlows,getCredentials,saveCredentials, and the opt-in project flow-file lifecycle operations that depend on how this layout stores flows.As part of that split,
runtime/lib/projects/git/Project.jsno longer needs to know how flow files are structured on disk. Instead, it delegates flow-file semantics to the active storage layout viastorage.getProjectLayout().In order to enable projects for your storage provider, you need to define:
storage.getProjectType() -> 'git'storage.getProjectLayout() -> layoutThat layout now provides the project file lifecycle hooks the built-in git project type needs:
flowFileExists- does a (non-project) flow file exist on disk - used for the create project formflowsDeclared- are flows declared in the project settingsexportFilesForEditor- exports flow/credentials paths that are used in the editorlistAutosaveFiles- the files that will auto-commit when enabledcreateFiles- create new project filesmigrateFiles- migrate existing project filesupdateFiles- update project on PUT /projects/:idbuildPackageSettings- populate package settings with file pathsCommit structure
This is easiest to review commit-by-commit:
9ac2fcc9bPure move/path-change commit - not much to see here.
runtime/lib/storage/localfilesystem/projects/*->runtime/lib/projects/*runtime/lib/storage/localfilesystem/util.js->runtime/lib/storage/util.js1ac551c3aNo real changes - behaviour-preserving refactor of
api/projectsunit tests onto shared helpers, so the runtime service move in the next commit is easier to review.95f736cd9Main refactor that decouples projects from storage and introduces the runtime service / git project type / flow layout split described above.
The third commit is the one I would focus on in isolation.
Review focus
Primary review targets are:
runtime/lib/projects/index.jsruntime/lib/projects/git/index.jsruntime/lib/projects/git/Project.js**runtime/lib/storage/localfilesystem/flows.js**Other runtime-facing updates are mostly mechanical call-site changes from
runtime.storage.projectstoruntime.projects, including:runtime/lib/api/projects.jsruntime/lib/api/settings.jsruntime/lib/flows/index.jsruntime/lib/nodes/credentials.jsruntime/lib/index.jsThere are also small supporting changes in:
runtime/lib/storage/index.js- addsflows.getProjectTypeandflows.getFlowLayoutto configure Projectsruntime/lib/storage/localfilesystem/index.js- initsflows.jsinstead ofprojects.jsruntime/lib/projects/git/defaultFileSet.js-project.files.flowassignment now inprojects/git/Project.js: buildDefaultFileAnd then there's just moves and updated import paths:
runtime/lib/storage/localfilesystem/projects/*->runtime/lib/projects/git/*runtime/lib/storage/localfilesystem/library.jsruntime/lib/storage/localfilesystem/sessions.jsruntime/lib/storage/localfilesystem/settings.jsPossible additions
RED.settings.filesso that the editor is agnosticruntime.storage.projectsfor custom storage modules that implement their own project management.I figured 3 could be a follow-up PR, 1 and 2 are drafted if you want me to include them in this PR.
Tests
New test files:
test/unit/@node-red/runtime/lib/projects/index_spec.jstest/unit/@node-red/runtime/lib/projects/git/index_spec.jstest/unit/@node-red/runtime/lib/projects/git/Project_spec.jstest/unit/@node-red/runtime/lib/projects/git/Project_writepath_spec.jstest/unit/@node-red/runtime/lib/storage/localfilesystem/flows_spec.jsUpdated test files (basically just doing
runtime.settings.projectstoruntime.projects):test/unit/@node-red/runtime/lib/projects/git/defaultFileSet_spec.jstest/unit/@node-red/runtime/lib/api/projects_spec.jstest/unit/@node-red/runtime/lib/api/settings_spec.jsMoves/updated import paths:
test/unit/@node-red/runtime/lib/storage/util_spec.jstest/unit/@node-red/runtime/lib/projects/git/**/*_spec.jsChecklist
npm run testto verify the unit tests pass