feature/1309: API version separation in NodeJS backend #1318
Conversation
This file contains 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
- v1 and v2 have their own dirs
- I appended a .v2 to all logger definitions for API V2 controllers/services
- I removed the obsolete endpoints from V2
- Added the new endpoints: PUT and DELETE /mapconfig/{name} instead of /mapconfig/create/{name} and /mapconfig/delete/{name}. This was already changed in the API spec so we will now conform to that
- This is by no means complete (at least I think there's room for improvement). See this as a proposal for future structure inside the Backend dir and please let me know what you think in #1309.
- Separation of API requires either duplicating a lot of functionality (common to the API versions) or drawing a line somewhere and saying 'hey, this part of code should be shared between versions'. - That's what I'm attemptign to do here and opinions are welcome. - The 'line' is currently drawn at 'stuff that don't expose /api/vN endpoint should be shared'. So we share the error handler, logger and detailed logger middlewares. The proxy middlewares and perhaps even the static files exposer middleware will be placed inside each API's version. That will come in a future commit.
- Added a key in .env, API_VERSIONS. It can contain a comma-separated list of integers that corespond to API versions. By default any empty value or no value at all means that all API versions are enabled. - Added a apiVersions constant both in server.js and as an Express app variable, so we can do app.get(apiVersions) anywhere and get the array. Nice. - Moved two product proxies (FME Server and Sokigo FB) to helper methods. Inside of them, we iterate and import required proxies dynamically. - Another iteration takes place to set up the OpenAPI Validator middleware and expose the specification, depending on which versions are required. Also nice.
- There were issues with it: seemingly the last middleware to be setup got all logging messages to it, which is unintended. - The logging we setup ourselves in the middlewares is fine though. Since we take care of both success and error and log then correctly, there was no use for HPM's internal logging.
…n check. - The generic proxy is not really versioned as it merely exposes some endpoints using HTTP Proxy Middleware. But we do expose proxies for each of the enabled versions of API to keep it consistent with previous Hajk versions. - The check added at startup ensure that we only attempt to initialize valid API versions. Those are, for now, kept in const ALLOWED_API_VERSIONS but could be moved further on.
- The static routes are still exposed outside the /api/vN, but the code used is dynamically imported. - Admin can specify a certain version of active API versions to use. Default value is the highest (latest) active API version.
…e bug discovered in 59b7915.
jacobwod
deleted the
feature/1309-api-v1-v2-separation-in-nodejs-backend
branch
jacobwod
added a commit
that referenced
this pull request
Apr 13, 2023
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.
Pretty massive but should work.
Some new keys in .env can be used to customise the functionality, but my primary intention is not to require any new keys, so please test with your current
.env.