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

6.0.0 alpha.1 #1088

Open
wants to merge 570 commits into
base: master
Choose a base branch
from
Open

6.0.0 alpha.1 #1088

wants to merge 570 commits into from

Conversation

BMartinos
Copy link
Collaborator

@BMartinos BMartinos commented Aug 27, 2020

This Pull Request is for the Alpha release for the OpenHIM core routing refactoring changes that has been implemented.

The routing refactoring changes made various changes to how the routing engine works within the OpenHIM. Previously, all payload messages were kept in memory while routing the transaction through to its destinations. While for small transactions this is fine, when bigger transactions are processed the OpenHIM can run out of memory and cause performance issues.

The OpenHIM also had to wait for the entire message to be read into memory before continuing the routing process, and the same applies for the response. They would need to be processed completely before responding to the client.

To combat these issues, we are not reading the entire incoming/outgoing payload into memory, but instead streaming it directly into MongoDB GridFS. This also allows us to remove the body truncate limit for storing payloads as the MongoDB limit of 16mb no longer applies.

Note: Some features have been temporarily dropped while developing this refactoring changes and will be coming back in in future, proved there is still a need for them.

Zooloo2014 and others added 30 commits July 23, 2019 14:12
Content matching isn't part of matchRequest anymore; so it cannot be
integration-tested with other matching functions.
matchContent it is called directly from the streamingReceiver.
matchContent has individual unit tests.

OHM-831
OHM-831 Add back content matching for request bodies
When a channel's responseBody property (which determines whether the response body is stored) is set to false or undefined, the client should still get a response. This issue has been resolved in this commit and the authentication tests are now passing. A function that is no longer used has also been removed

OHM-896
The order of the middlewares had to be changed as the auto retry attempt number for a rerun transaction was being set after the logic that updates the transaction in the database. This was resulting in continuous rerunning for channels that have a max number of attempts

OHM-824
The method that completes the response and updates the transaction (in the db) was not updating the error property of the transaction (if there is an error). This was resulting in failure for some integration tests.

OHM-824
The response from an openhim mediator comes in a special format, and it has to parsed so that the response body, status, etc can be extracted. This means that it has to be stored in memory. The method that collects the chunks of the mediator's response is asynchronous and it has to be waited on. This is so we can set the right status and send back the body only (without the other fields on the mediator's response) to the client

OHM-824
Calling the method was not necessay as it is called in some other places once the response has been returned

OHM-824
whenever there is an error, the method updateWithError is invoked. Calling initiateResponse is not necessary as updateWithError does the updates that initiateResponse does. Calling the two methods at once was causing a race condition. At times the error was not being updated, resulting in some tests which have assertions on the error to be inconsistent

OHM-824
The property 'responseBody' of the channel has been set to true. This is because, at the moment when the property is false or undefined, the app does not send a response to the client. This sending of the response issue (when channel's responseBody property is set to false) has been addressed on another branch. This is a temporary fix to get the tests to pass.

OHM-824
One test has an assertion that expects the error message, when the secondary route fails, to be 'ERRCONNECT' and this was failing as the app has been modified such that when ever there is an error on the primary route the socket for the secondary route request is destroyed

OHM-824
fix responding to the client (responseBody -false)
The app was storing request bodies even when the channel had been configured not to store the bodies

OHM-898
The logic had to be modified as all transactions that failed were being queued up for auto Retry, even when the channel is set not to auto retry a failed transction. The queueing of the transactions to be retried and the auto retries should only occur when the channel's property "autoRetryEnabled" is true. This way only transactions that fail after the autoRetry is enabled are rerun

OHM-824
This was causing the tests to fail as transactions are not marked for autoRetries when this property is false

OHM-824
The response body Id was being saved on transaction, and would never show up on the console

OHM-824
The response body id was not being set in the response headers and this resulted in bodies not showing on the openhim console as the transctions' body Id would be undefined. This was happening for responses from openhim mediators, which we handle differently

OHM-824
The orchestration's response body id was not being set, resulting in the response body for the orchestrations not showing up on the console

OHM-824
The body id should be added to the response headers. Logic has been added to check whether the response headers property exists before adding the body id. One test in which the openhim mediator response did not have headers was failing as a result.

OHM-824
The response from an openhim mediator is different and has to be handled differently. The mediator response contains a response and orchestrations, which have bodies. These bodies have to be stored seperately in gridfs and the transaction updated with the body ids. The api consumed by the console expects a body id for each body (orchestration and response). Because the body ids were not being set, the orchestr<ations displayed on the console were bodiless

OHM-824
One function for storing the transaction was removed, so the tests were failing

OHM-820
Orcherstartions were not being stored in the event of a failure on the primary route

OHM-820
The method that stores the transaction metrics was changed, it now calculaates the responseTime using the timestamp created at the end of the response streaming and not the one created at the start of the streaming

OHM-820
When a transaction does not exist in the database any method that tries to get or update the transaction should return a callback with an error or reject with an error. If this does not happen references to the transaction will cause the app failures

OHM-820
the function setFinalStatus returns an error and a null value for the transaction when there is an error. The error message logged out had to be changed as you cant get an id of a null value. this can cause a failure in the app

OHM-820
MattyJ007 and others added 30 commits May 11, 2021 09:13
…t-info-2.8.9

Bump hosted-git-info from 2.8.8 to 2.8.9
Issue #1100: Request matching should include HTTP methods
Revert "Issue #1100: Request matching should include HTTP methods"
We don't register polling channels when import metadata from a file. This is because we go directly to the database and don't use our programmatic API that was designed to cover these sorts to nuances. This is why builting interfaces to DB access (or any external system) is important. It allows you to put all the handling code in one place.

Eventually we should re-work the metadata import to rather use the API for all objects that are imported. This, however, is a much larger task.
Fix importing of polling channels - for alpha branch
When a request does not match any channel a response status of 404 should be sent back

OHM-1078
THis adds the unit tests for the method that is used for matching the request method to a channel's method

OHM-1078
The tests had to be modified as the request matching logic has been changed. The channels in the tests did not have the method, which is used in the matching of the request to a channel

OHM-1078
These are the config files for applying eslint styling to the codebase

OHM-1082
To prevent special characters being used in the patientID field (fix a bug)

OHM-1084 OHM-927
Improve testing code coverage and correct mistake in regex check
OHM-1084 OHM-927
The previous changes reduced the auditAPITests.js code coverage
OHM-1084 OHM-927
Keep the codebase consistent

OHM-927
Hyphens are safe characters without other meaning in the regex syntax

OHM-927
Fix any security vulnerabilities
Bug fix and dep update
Moving the dockerfile from a separate repo to keep everything in one
place

OHM-1052
Prevent clutter in docker image

OHM-1052
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.

8 participants