Merge pull request #199 from pulsar-edit/use-controllers

Modularity Refactor - Stop treating HTTP like it's special

.codacy.yaml

@@ -4,4 +4,4 @@ engines:
     minTokenMatch: 80
 exclude_paths:
   - "./docs/*"
-  - "./scripts/**"
+  - "./scripts/**/**"

.compactignore

@@ -14,6 +14,8 @@ coverage/
 >   gcloudignore
 .gcloudignore
 test/
+tests/
+scripts/
 docs/reference
 docs/resources/
 docs/build.md

.gcloudignore

@@ -5,13 +5,13 @@ node_modules/
 data/
 .gcloudignore
 test/
+tests/
+scripts/
 docs/reference
 docs/resources/
 docs/build.md
 docs/host_your_own.md
 docs/writingIntegrationTests.md
-scripts/
-health-check-output.json # Just in case it's output and not deleted
 # Git Tooling / NPM Tooling
 .git/
 .github/

codeql-config.yml

@@ -6,4 +6,5 @@ queries:
 
 paths-ignore:
   - ./test
+  - ./tests
 # https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors

docs/reference/API_Definition.md

@@ -1,3 +1,5 @@
+# WARNING This file is deprecated. And will soon be removed. Leaving our swagger instance to be the complete standard in the Pulsar backend API documentation.
+
 # **[GET]** /
 A non-essential endpoint, returning a status message, and the server version.
 
@@ -25,7 +27,7 @@ Parameters:
 
 
 ---
-* page _(optional)_ `[integer]` | Location: `query` | Defaults: `1` 
+* page _(optional)_ `[integer]` | Location: `query` | Defaults: `1`
   - Indicate the page number to return.
 
 

docs/resources/refactor-docs.md

@@ -0,0 +1,80 @@
+# DON'T KEEP THIS FILE
+
+Alright, so lets do a big time refactor, because it's fun, and I get bothered looking at the same code for too long or something.
+
+Essentially here's the new idea:
+
+## HTTP Handling
+
+Stop treating it as if it was special. HTTP handling is essentially only a utility function that's easily **replicable** and should be treated as such.
+
+The only part of an HTTP handling process that matters is the logic that's preformed. The logic of returning the data depending on states of SSO's or adding pagination or even erroring out is insanely easily replicable.
+
+So we should abstract away from hardcoding endless functions for HTTP handling as much as possible. So here's my idea:
+
+Every endpoint is it's own tiny module. This module should export at least two things:
+
+* `logic()` This function will be called to handle the actual logic of the endpoint, passing all relevant data to it
+* `params()` This function will return a parameter object consisting of all query parameters that this endpoint expects to receive
+* `endpoint` The endpoint object will then provide the endpoint logic with everything else it needs to define any given endpoint.
+
+From here the `main.js` module should instead import all modules needed, and iterate through them to create every single endpoint as needed. This may result in a slightly longer startup time, but overall I hope the increase in code readability and less duplication will be worth it.
+
+So this means that every module is imported, the `endpoint` object is read to setup the endpoint, and from there, it's made available as an endpoint via express, which can then, once hit, use the `params()` function to prepare the query parameters, and then pass those off to the `logic()` function.
+
+### `endpoint` Structure
+
+The `path` here is an array since in some instances, we want to accept multiple paths, such as `POST /api/packages` and `POST /api/themes`.
+
+```javascript
+const endpoint = {
+  // Can be "GET", "POST", "DELETE"
+  method: "GET",
+  paths: [ "/api/themes" ],
+  // Can be "generic" or "auth"
+  rate_limit: "generic",
+  options: {
+    // This would be the headers to return for `HTTP OPTIONS` req:
+    Allow: "GET",
+    "X-Content-Type-Options": "nosniff"
+  }
+};
+```
+
+## Returning HTTP handling
+
+Again, the logic here is easily replicable. So we shouldn't make it special. And if finally doing a proper rewrite, we can incorporate a proper SSO, and have a different type for every single return. This way the actual handling of any return, can instead be a factor of a `httpReturn()` function of the SSO itself, rather than baked in logic. So that way we can keep the return logic as unique as needed, as the uniqueness depends solely on the uniqueness of the SSO being returned.
+
+## Tests
+
+(As always the bane of existence)
+
+With this refactor, we no longer need true "integration" tests. As integration can be tested on if the proper endpoints being hit call the proper endpoint.logic() function. Beyond that the majority of "integration" testing would be relegated to interactions with external services working as expected.
+
+Meaning the only tests we would likely need are:
+
+* `tests` This would be the vast majority of tests, able to be generic, and not needing any fancy setup
+* `database` This suite of tests should purely test if DB calls do what we expect
+* `integration` A small suite of full integration tests is never a bad idea. To test that API calls have the intended effects on the DB. With a huge focus on having the intended effects. As we are seeing some examples where the expected data is not appearing or being applied to the DB as we want.
+* `external` We don't do this currently. But a suite of external tests that are run on a maybe monthly basis is not a bad idea. This could allow us to ensure external APIs are returning data as expected.
+
+---
+
+I think this is plenty to focus on now. At the very least the changes described here would likely mean a rewrite of about or over half the entire codebase. But if all goes to plan, would mean that every single piece of logic is more modular, keeping logic related to itself within the same file, and if tests are effected as hoped, would mean a much more robust testing solution, that who knows, may actually be able to achieve near 100% testing coverage.
+
+One side effect of all this change, means the possibility of generating documentation of the API based totally on the documentation itself, where we no longer would be reliant on my own `@confused-techie/quick-webserver-docs` module, nor having to ensure comments are updated.
+
+## Documentation
+
+Alright, so some cool ideas about documentation.
+
+Within `./src/parameters` we have modules named after the common name of any given parameter.
+
+Then to ensure our OpenAPI docs are ALWAYS up to date with the actual code, we take the `params` object of every single endpoint module, and we actually iterate through the `params` there and match those against these modules within `parameters`.
+
+So that the parameters within our docs will always match exactly with what's actually used. This does mean the name we use for the parameter within code has to match up with the names of the files.
+
+Additionally, for the content, we can reference a fileName from ./tests/models so that we can accomplish the following:
+
+* Use that object's `schema` and `example` to generate an object
+* We can also possibly use this to ensure the return matches what we expect.

jest.config.js

@@ -1,53 +1,43 @@
 const config = {
-  setupFilesAfterEnv: ["<rootDir>/test/global.setup.jest.js"],
+  setupFilesAfterEnv: [
+    "<rootDir>/tests/helpers/global.setup.jest.js"
+  ],
   verbose: true,
   collectCoverage: true,
-  coverageReporters: ["text", "clover"],
+  coverageReporters: [
+    "text",
+    "clover"
+  ],
   coveragePathIgnorePatterns: [
-    "<rootDir>/src/tests_integration/fixtures/**",
-    "<rootDir>/test/fixtures/**",
-    "<rootDir>/node_modules/**",
+    "<rootDir>/tests/",
+    "<rootDir>/test/",
+    "<rootDir>/node_modules/"
   ],
   projects: [
     {
       displayName: "Integration-Tests",
       globalSetup: "<rootDir>/node_modules/@databases/pg-test/jest/globalSetup",
-      globalTeardown:
-        "<rootDir>/node_modules/@databases/pg-test/jest/globalTeardown",
+      globalTeardown: "<rootDir>/node_modules/@databases/pg-test/jest/globalTeardown",
       setupFilesAfterEnv: [
-        "<rootDir>/test/handlers.setup.jest.js",
-        "<rootDir>/test/global.setup.jest.js",
+        "<rootDir>/tests/helpers/handlers.setup.jest.js",
+        "<rootDir>/tests/helpers/global.setup.jest.js"
       ],
       testMatch: [
-        "<rootDir>/test/*.integration.test.js",
-        "<rootDir>/test/database/**/**.js",
-      ],
+        "<rootDir>/tests/database/**.test.js",
+        "<rootDir>/tests/http/**.test.js",
+        "<rootDir>/tests/vcs/**.test.js"
+      ]
     },
     {
       displayName: "Unit-Tests",
-      setupFilesAfterEnv: ["<rootDir>/test/global.setup.jest.js"],
-      testMatch: [
-        "<rootDir>/test/*.unit.test.js",
-        "<rootDir>/test/handlers/**/**.test.js",
-      ],
-    },
-    {
-      displayName: "VCS-Tests",
-      setupFilesAfterEnv: ["<rootDir>/test/global.setup.jest.js"],
-      testMatch: ["<rootDir>/test/*.vcs.test.js"],
-    },
-    {
-      displayName: "Handler-Tests",
-      globalSetup: "<rootDir>/node_modules/@databases/pg-test/jest/globalSetup",
-      globalTeardown:
-        "<rootDir>/node_modules/@databases/pg-test/jest/globalTeardown",
       setupFilesAfterEnv: [
-        "<rootDir>/test/handlers.setup.jest.js",
-        "<rootDir>/test/global.setup.jest.js",
+        "<rootDir>/tests/helpers/global.setup.jest.js"
       ],
-      testMatch: ["<rootDir>/test/*.handler.integration.test.js"],
-    },
-  ],
+      testMatch: [
+        "<rootDir>/tests/unit/**/**.test.js"
+      ]
+    }
+  ]
 };
 
 module.exports = config;

package.json

@@ -8,18 +8,14 @@
   },
   "scripts": {
     "start": "node ./src/server.js",
+    "test": "cross-env NODE_ENV=test PULSAR_STATUS=dev jest --selectProjects Integration-Tests Unit-Tests --runInBand",
     "test:unit": "cross-env NODE_ENV=test PULSAR_STATUS=dev jest --selectProjects Unit-Tests",
     "test:integration": "cross-env NODE_ENV=test PULSAR_STATUS=dev jest --selectProjects Integration-Tests",
     "start:dev": "cross-env PULSAR_STATUS=dev node ./src/dev_server.js",
-    "test": "cross-env NODE_ENV=test PULSAR_STATUS=dev jest --selectProjects Integration-Tests Unit-Tests VCS-Tests",
-    "test:vcs": "cross-env NODE_ENV=test PULSAR_STATUS=dev MOCK_GH=false jest --selectProjects VCS-Tests",
-    "test:handlers": "cross-env NODE_ENV=test PULSAR_STATUS=dev jest --selectProjects Handler-Tests",
-    "api-docs": "quick-webserver-docs -i ./src/main.js -o ./docs/reference/API_Definition.md",
     "lint": "prettier --check -u -w .",
     "complex": "cr --newmi --config .complexrc .",
     "js-docs": "jsdoc2md -c ./jsdoc.conf.js ./src/*.js ./src/handlers/*.js ./docs/resources/jsdoc_typedef.js > ./docs/reference/Source_Documentation.md",
     "contributors:add": "all-contributors add",
-    "test_search": "node ./scripts/tools/search.js",
     "migrations": "pg-migrations apply --directory ./scripts/migrations",
     "ignore": "compactignore",
     "tool:delete": "node ./scripts/tools/manual-delete-package.js",

scripts/deprecated-migrations/0008-migrated-initial.sql

@@ -0,0 +1,120 @@
+-- Enter our Test data into the Database.
+
+INSERT INTO packages (pointer, package_type, name, creation_method, downloads, stargazers_count, data, original_stargazers)
+VALUES (
+  'd28c7ce5-c9c4-4fb6-a499-a7c6dcec355b', 'package', 'language-css', 'user made', 400004, 1,
+  '{"name": "language-css", "readme": "Cool readme", "metadata": {"bugs": {"url": "https://github.com/atom/language-css/issues"},
+  "name": "language-css", "engines": {"atom": "*","node":"*"},"license":"MIT","version":"0.45.7","homepage":"http://atom.github.io/language-css",
+  "keywords":["tree-sitter"],"repository":{"url":"https://github.com/atom/language-css.git","type":"git"},"description":"CSS Support in Atom",
+  "dependencies":{"tree-sitter-css":"^0.19.0"},"devDependencies":{"coffeelint":"^1.10.1"}},"repository":{"url":"https://github.com/atom/langauge-css",
+  "type":"git"}}', 76
+), (
+  'd27dbd37-e58e-4e02-b804-9e3e6ae02fb1', 'package', 'language-cpp', 'user made', 849156, 1,
+  '{"name": "language-cpp", "description": "C++ Support in Atom", "keywords": ["tree-sitter"]}', 91
+), (
+  'ee87223f-65ab-4a1d-8f45-09fcf8e64423', 'package', 'hydrogen', 'Migrated from Atom.io', 2562844, 1,
+  '{"name": "hydrogen", "readme": "Hydrogen Readme", "metadata": { "main": "./dist/main", "name": "Hydrogen",
+  "author": "nteract contributors", "engines": {"atom": ">=1.28.0 <2.0.0"}, "license": "MIT", "version": "2.16.3"}}', 821
+), (
+  'aea26882-8459-4725-82ad-41bf7aa608c3', 'package', 'atom-clock', 'Migrated from Atom.io', 1090899, 1,
+  '{"name": "atom-clock", "readme": "Atom-clok!", "metadata": { "main": "./lib/atom-clock", "name": "atom-clock",
+  "author": { "url": "https://github.com/b3by", "name": "Antonio Bevilacqua", "email": "[email protected]"}}}', 528
+), (
+  '1e19da12-322a-4b37-99ff-64f866cc0cfa', 'package', 'hey-pane', 'Migrated from Atom.io', 206804, 1,
+  '{"name": "hey-pane", "readme": "hey-pane!", "metadata": { "main": "./lib/hey-pane", "license": "MIT"}}', 176
+), (
+  'a0ef01cb-720e-4c0d-80c5-f0ed441f31fc', 'theme', 'atom-material-ui', 'Migrated from Atom.io', 2509605, 1,
+  '{"name": "atom-material-ui", "readme": "ATOM!"}', 1772
+), (
+  '28952de5-ddbf-41a8-8d87-5d7e9d7ad7ac', 'theme', 'atom-material-syntax', 'Migrated from Atom.io', 1743927, 1,
+  '{"name": "atom-material-syntax"}', 1309
+), (
+  '504cd079-a6a4-4435-aa06-daab631b1243', 'theme', 'atom-dark-material-ui', 'Migrated from Atom.io', 300, 1,
+  '{"name": "atom-dark-material-ui"}', 2
+);
+
+INSERT INTO names (name, pointer)
+VALUES (
+  'language-css', 'd28c7ce5-c9c4-4fb6-a499-a7c6dcec355b'
+), (
+  'language-cpp', 'd27dbd37-e58e-4e02-b804-9e3e6ae02fb1'
+), (
+  'hydrogen', 'ee87223f-65ab-4a1d-8f45-09fcf8e64423'
+), (
+  'atom-clock', 'aea26882-8459-4725-82ad-41bf7aa608c3'
+), (
+  'hey-pane', '1e19da12-322a-4b37-99ff-64f866cc0cfa'
+), (
+  'atom-material-ui', 'a0ef01cb-720e-4c0d-80c5-f0ed441f31fc'
+), (
+  'atom-material-syntax', '28952de5-ddbf-41a8-8d87-5d7e9d7ad7ac'
+), (
+  'atom-dark-material-ui', '504cd079-a6a4-4435-aa06-daab631b1243'
+);
+
+INSERT INTO versions (package, status, semver, license, engine, meta)
+VALUES (
+  'd28c7ce5-c9c4-4fb6-a499-a7c6dcec355b', 'published', '0.45.7', 'MIT', '{"atom": "*", "node": "*"}',
+  '{"name": "language-css", "description": "CSS Support in Atom", "keywords": ["tree-sitter"],
+     "tarball_url": "https://github.com/pulsar-edit/language-css"}'
+), (
+  'd28c7ce5-c9c4-4fb6-a499-a7c6dcec355b', 'latest', '0.46.0', 'MIT', '{"atom": "*", "node": "*"}',
+  '{"name": "language-css", "description": "CSS Support in Atom", "keywords": ["tree-sitter"]}'
+), (
+  'd28c7ce5-c9c4-4fb6-a499-a7c6dcec355b', 'published', '0.45.0', 'MIT', '{"atom": "*", "node": "*"}',
+  '{"name":"language-css", "description": "CSS Support in Atom", "keywords": ["tree-sitter"]}'
+), (
+  'd27dbd37-e58e-4e02-b804-9e3e6ae02fb1', 'published', '0.11.8', 'MIT', '{"atom": "*", "node": "*"}',
+  '{"name": "language-cpp", "description": "C++ Support in Atom", "keywords": ["tree-sitter"]}'
+), (
+  'd27dbd37-e58e-4e02-b804-9e3e6ae02fb1', 'latest', '0.11.9', 'MIT', '{"atom": "*", "node": "*"}',
+  '{"name": "language-cpp", "description": "C++ Support in Atom", "keywords": ["tree-sitter"]}'
+), (
+  'ee87223f-65ab-4a1d-8f45-09fcf8e64423', 'latest', '2.16.3', 'MIT', '{"atom": "*"}',
+  '{"name": "Hydrogen", "dist": {"tarball": "https://www.atom.io/api/packages/hydrogen/version/2.16.3/tarball"}}'
+), (
+  'aea26882-8459-4725-82ad-41bf7aa608c3', 'latest', '0.1.18', 'MIT', '{"atom": "*"}',
+  '{"name": "atom-clock", "dist": {"tarball": "https://www.atom.io/api/packages/atom-clock/version/1.18.0/tarball"}}'
+), (
+  '1e19da12-322a-4b37-99ff-64f866cc0cfa', 'latest', '1.2.0', 'MIT', '{"atom": "*"}',
+  '{"name":"hey-pane", "dist": {"tarball": "https://www.atom.io/api/packages/hydrogen/version/1.2.0/tarball"}}'
+), (
+  'a0ef01cb-720e-4c0d-80c5-f0ed441f31fc', 'latest', '2.1.3', 'MIT', '{"atom": "*"}',
+  '{"name": "atom-material-ui", "dist": {"tarball": "https://www.atom.io/api/packages/atom-material-ui/version/2.1.3/tarball"}}'
+), (
+  '28952de5-ddbf-41a8-8d87-5d7e9d7ad7ac', 'latest', '1.0.8', 'MIT', '{"atom":"*"}',
+  '{"name": "atom-material-syntax", "dist": {"tarball":"https://www.atom/io/api/packages/atom-material-syntax/version/1.0.8/tarball"}}'
+), (
+  '504cd079-a6a4-4435-aa06-daab631b1243', 'latest', '1.0.0', 'MIT', '{"atom": "*"}',
+  '{"name":"atom-dark-material-ui", "dist": {"tarball": "https://www.atom.io/api/packages/atom-dark-material-ui/versions/1.0.0/tarball"}}'
+);
+
+INSERT INTO users (username, node_id, avatar)
+VALUES (
+  'dever', 'dever-nodeid', 'https://roadtonowhere.com'
+), (
+  'no_perm_user', 'no-perm-user-nodeid', 'https://roadtonowhere.com'
+), (
+  'admin_user', 'admin-user-nodeid', 'https://roadtonowhere.com'
+), (
+  'has-no-stars', 'has-no-stars-nodeid', 'https://roadtonowhere.com'
+), (
+  'has-all-stars', 'has-all-stars-nodeid', 'https://roadtonowhere.com'
+);
+
+INSERT INTO stars (package, userid)
+VALUES (
+  'd28c7ce5-c9c4-4fb6-a499-a7c6dcec355b', 5
+), (
+  'd27dbd37-e58e-4e02-b804-9e3e6ae02fb1', 5
+), (
+  'ee87223f-65ab-4a1d-8f45-09fcf8e64423', 5
+), (
+  'aea26882-8459-4725-82ad-41bf7aa608c3', 5
+), (
+  '1e19da12-322a-4b37-99ff-64f866cc0cfa', 5
+), (
+  'a0ef01cb-720e-4c0d-80c5-f0ed441f31fc', 5
+), (
+  '28952de5-ddbf-41a8-8d87-5d7e9d7ad7ac', 5
+);