Mongodb DAO Update #112

PatrickFuhrmann-HTWBerlin · 2024-06-20T12:21:21Z

Main Objective:

The goal of this pull request is to prepare for adding new formats to the OAI PMH provider in addition to Panosc, dc and openaire. As a first step we would like to get the mongodb DAO and the entire package updated to the most recent version.
The pull request consists of various commits with the corresponding explanation.

Some additional remarks:

We are running:

npm --version : 10.5.1
node --version: v22.0.0
A) on 'npm install':
When doing a npm install, you will find WARNINGS on 'inflight' and 'glob'. They are hard to fix because they are loaded from gulp. But they are not so important as they are only used for the installation while using gulp. The actual services is not using them.
B) I didn't touch the CI/ESS part. I don't know why they have the private directory.
C) We only updated 'production/.env' to add the key 'COLLECTION_ID'. This is only important when not using the COLLECTION_ID='doi'. As a general remark: When running docker, docker-compose or kubernetes, you will overwrite those values anyway, so the content is not that important.
D) We are working on the workflow part now, as Carlo wants us to push a docker image to ghcr.io from within a .githup/workflow.
E) Works for us
- 'npm ci'
- 'npm run compile'
- docker build -t oai-pmh-provider' .
F) I checked the functionality by building the docker container by the Dockerfile and running it within scicat-live. The following REST calls produce the correct output:
http://localhost:3000/
http://localhost:3000/scicat/oai?verb=Identify
http://localhost:3000/scicat/oai?verb=ListMetadataFormats
http://localhost:3000/scicat/oai?verb=GetRecord&identifier=1&metadataPrefix=oai_dc
http://localhost:3000/scicat/oai?verb=ListIdentifiers&metadataPrefix=oai_dc
http://localhost:3000/scicat/oai?verb=ListRecords&metadataPrefix=oai_dc

The same is true for the '/panosc/oai' and '/openaire/oai' URL. I must admit, I didn't try to 'PUT' a record, because records are added through the backend-next/frontend system and PUT is not mentioned in the README.md.

We are happy to improve the pull request. We might not be 100% compliant with your coding rules.

-p.

The goal is to have the mongodb driver to something > 6.0.

deprication warnings.

Most important for us is adding the 'COLLECTION_ID=' because sometimes the test data doesn't have the 'doi' as a key and we change it to the '_id' value, which is always available in mongodb. In general we don't understand where the .env is really used in production. When using docker, docker-compose or kubernetes, we would specify all environment variable specifically anyway. Like in '-e' or the environment tag in docker compose.

- Using then/catch with mongdb.connect. - Using the Promise returned by library calls instead of creating our own Promise. - It seems that MongoClient.filter is no longer used in the 6.0 mongodb library. So we change to 'any'. Is of course less type save.

for the 'pino' package.

if necessary. Adding an example on how to start the docker container.

minottic

a part from minor changes, it looks good, thanks!

I see there are some npm upgrades, but the node version is still fairly old. I guess, once this is merged, we could open another PR doing the node upgrade in the dockerfile

But most important: the tests fail, so they need to be fixed. In principle, it would be good to have tests for the mongo functions, but it's quite some work I guess

package.json

gulpfile.js

src/providers/scicat-provider/dao/mongo-dao.ts

ENVIRONMENT_README.md

Dockerfile

…ctions.

a) Using tsx instead of tsc. Didn't get the mocha to work with tsc. b) Fixing the the expected return from the oai-pmh API calles. - Lower case to the proper answer - Removing date structures before the comparison. c) Reading the dotenv part first before loading the test. Mostly to get the proper LOG_LEVEL value. d) The PUT of the 'test' record into the DB is not defined by the V2 oai-pmh protocol and the return of the insertOne mongodb call changed. That is reflected in the fixtures ... record-doi.ts file.

PatrickFuhrmann-HTWBerlin · 2024-07-21T19:41:14Z

If fixed the Tests. The way the tests are done are a bit fragile. The response to the API calls are not checked against XML spec of XMl namespace but are a char to char comparison.

I got the test to succeed in my repository but don't know if it works in the scicat one.
I took the deploy.yml from BE as suggested but removed the ESS pipeline for now.

bpedersen2

LGTM

.github/workflows/deploy.yml

README.md

bpedersen2 · 2024-07-22T06:41:37Z

README.md

+DB_USER | Database Username | none
+DB_PASS | Database Password | none
+DB_URL |  [&lt;user&gt;:&lt;password&gt;@]&lt;host&gt;:&lt;port&gt;/&lt;dbName&gt;| none
+DATABASE | Publication Database | dacat-next


maybe change to new scicat namings in a follow-up

Ok will do later.

package.json

bpedersen2 · 2024-07-22T06:43:45Z

test/mongo-dao.ts

+    it('should wait awhile before continuing', function(done) {
+        this.timeout(61000); // Optional: set timeout for this test to 61 seconds
+        setTimeout(function() {
+            // Your test logic here


currently a no-op test?

True. But it creates a 10 second delay which might not be very elegant. But I had to make sure that the oai-pmh-service part had a chance to connect to the mongodb server. Otherwise all tests fail.

minottic

LGTM, thanks. 2 minor comments, in addition to Bjorn's.

Just a side note. In principle this PR should include tests for the modified functions (as they aren't there already). The existing tests don't really test the mongo functionality as they mock their behaviour. I am fine mergining without, anyway these changes are mostly using mongo native functionalities that we assume have been tested by mongo

src/providers/scicat-provider/dao/mongo-dao.ts

… errors during the githup workflow

…ding error message during compilation.

PatrickFuhrmann-HTWBerlin · 2024-07-23T06:13:04Z

I had to fix two more problems:

c16411c: On githup (not on my mac M1 and not on my ARM reference) the node:18-alpine produced errors. This doesn't happen with the node:18-slim.
0c81791: Using the stricter and more typesafe Filter<> over Filter syntax to satisfy the gulp ts compiler.

minottic

some more comments, but it's almost ready I think.

I am unsure I understand why the tests changes were needed

minottic · 2024-07-23T07:10:47Z

.github/workflows/ci.yml

@@ -1,6 +1,10 @@
 name: CI

-on: [pull_request]
+on:
+  push:


maybe this ca be on PR only?

Sure. I wanted to make sure that the test/deploy works on your side.

Dockerfile

test/routes.ts

test/fixtures/returns/panosc.returns.ts

minottic · 2024-07-23T07:19:13Z

I think the push to ghcr is failing because it's happening from a fork

minottic · 2024-07-24T08:21:51Z

.github/workflows/deploy.yml

+        with:
+          context: .
+          platforms: linux/amd64,linux/arm64/v8
+          push: true


can we set this to true only on push to master? in this way, we avoid the permission error when coming from a fork

Yes, I think we can:

eac0a72 : made the push dependent on push to master.

372bfc7: forgot to change the trigger back to master.

I think it would be enough to evaluate the true false condition in the push field.

push: ${{ github.event_name. == 'push' }}

Sorry I misunderstood. I fixed with 077150a. Seems to work for 'push' on my branch and 'pr' on the scicat master.

np! thanks for the changes! I would be fine merging now, do you plan any other change or is it good to go?

I'm not planning any other changes in this PR so we are good to go from my side. Thanks a lot for your help.

minottic

thx!

patrickfuhrmann added 8 commits June 20, 2024 11:10

Updating all versions to the highest possible w/o conflicts.

6b8b89b

The goal is to have the mongodb driver to something > 6.0.

Replacing the 'del' package by the 'rimraf' package to avoid

c0d6c09

deprication warnings.

Using the 'date' substructure of releaseDate. Error otherwise.

ee96447

TS now requires to use 'const pino ..' instead of 'import pino ..'

924ccdd

for the 'pino' package.

Adding an upgrade of npm to [email protected].

c8e8e19

Adding a description of the required variables. Can to to README.md

f1835ee

if necessary. Adding an example on how to start the docker container.

minottic reviewed Jul 12, 2024

View reviewed changes

patrickfuhrmann added 10 commits July 21, 2024 17:39

Formatting fixed.

3ba061a

a) mongo 'Filter' type replacing 'any' b) Formatting improved

4485e2e

Mongodb already returns Promises, which we forward to the calling fun…

9819f17

…ctions.

Run the TEST

4e088c6

Deployment taken from BE

2aac59c

Describing necessary ENV variables

658b721

Was moved to README.md

8e50d62

Adding the real update time

6ddedbf

Fixing format with XML namespace url

ee664d0

bpedersen2 reviewed Jul 22, 2024

View reviewed changes

minottic reviewed Jul 22, 2024

View reviewed changes

src/providers/scicat-provider/dao/mongo-dao.ts Outdated Show resolved Hide resolved

src/providers/scicat-provider/dao/mongo-dao.ts Outdated Show resolved Hide resolved

patrickfuhrmann added 6 commits July 22, 2024 12:38

Applying workflow to master instead of the development branch.

0dc0853

Formal Fix: Merging MongoClient, Filter import from mongodb.

daab992

Changed default LOG_LEVEL to 'error' in .env and README.md

95b512b

Putting { id:1 } projection back to identifiersQuery.

738df60

Switching back to the node:18-slim version as apline seems to produce…

c16411c

… errors during the githup workflow

Making the mongoFilter more typesafe with Filter<StatusFilter>), avoi…

0c81791

…ding error message during compilation.

minottic reviewed Jul 23, 2024

View reviewed changes

minottic reviewed Jul 24, 2024

View reviewed changes

patrickfuhrmann added 3 commits July 24, 2024 20:40

Only pushing output image to ghcr if we push code to master

eac0a72

Changing trigger back to master

372bfc7

Pushing to the GitHub Container Registry only on 'push' not on 'pr'

077150a

minottic approved these changes Jul 25, 2024

View reviewed changes

minottic merged commit a42f7a0 into SciCatProject:master Jul 25, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mongodb DAO Update #112

Mongodb DAO Update #112

PatrickFuhrmann-HTWBerlin commented Jun 20, 2024

minottic left a comment

PatrickFuhrmann-HTWBerlin commented Jul 21, 2024

bpedersen2 left a comment

bpedersen2 Jul 22, 2024

PatrickFuhrmann-HTWBerlin Jul 22, 2024

bpedersen2 Jul 22, 2024

PatrickFuhrmann-HTWBerlin Jul 22, 2024

minottic left a comment

PatrickFuhrmann-HTWBerlin commented Jul 23, 2024

minottic left a comment

minottic Jul 23, 2024

PatrickFuhrmann-HTWBerlin Jul 23, 2024

minottic commented Jul 23, 2024

minottic Jul 24, 2024

PatrickFuhrmann-HTWBerlin Jul 24, 2024

minottic Jul 25, 2024 •

edited

Loading

PatrickFuhrmann-HTWBerlin Jul 25, 2024

minottic Jul 25, 2024

PatrickFuhrmann-HTWBerlin Jul 25, 2024

minottic left a comment

Mongodb DAO Update #112

Mongodb DAO Update #112

Conversation

PatrickFuhrmann-HTWBerlin commented Jun 20, 2024

Main Objective:

Some additional remarks:

We are running:

minottic left a comment

Choose a reason for hiding this comment

PatrickFuhrmann-HTWBerlin commented Jul 21, 2024

bpedersen2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minottic left a comment

Choose a reason for hiding this comment

PatrickFuhrmann-HTWBerlin commented Jul 23, 2024

minottic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minottic commented Jul 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minottic Jul 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minottic left a comment

Choose a reason for hiding this comment

minottic Jul 25, 2024 •

edited

Loading