EIP1-11113: Import Register Checker API Codebase and Project Setup #113
Conversation
… API project that has merged db migrations already in it; resolve issues arising from address entity field differences between APIs
SimPaySW
force-pushed
the
EIP1-11113-review-1-import-reg-check-codebase
branch
from
ec70132 to
72cfda2
Compare
SimPaySW
changed the base branch from
EIP1-11113-review-target-proxy-main
to
main
SimPaySW
changed the base branch from
main
to
EIP1-11113-review-target-proxy-main-wo-11198
SimPaySW
changed the title
SimPaySW
changed the title
SimPaySW
changed the base branch from
EIP1-11113-review-target-proxy-main-wo-11198
to
main
| @@ -44,4 +44,14 @@ databaseChangeLog: | |||
| - include: | |||
| file: /db/changelog/create/0022_EROPSPT-350_add_index_on_status.xml | |||
| - include: | |||
| file: db/changelog/create/0023_EIP1-11198_add-is-from-applications-api-flag.xml | |||
There was a problem hiding this comment.
is there a reason for no longer including this Db migration?
There was a problem hiding this comment.
The aim of the isFromApplicationsApi flag is to give a marker on the portal that applications are from the service apis or applications api. It isn't needed by the EMS api.
There was a problem hiding this comment.
Will we definately not need docker on this service anymore?
There was a problem hiding this comment.
Docker comes back in in review 3.
| @@ -1,243 +1,3 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <suppressions xmlns="https://jeremylong.github.io/DependencyCheck/dependency-suppression.1.3.xsd"> | |||
| <suppress until="2024-05-03Z"> | |||
There was a problem hiding this comment.
In the register checker api there are two suppersions "CVE-2024-12801" and "CVE-2024-12798" do we need to include them here?
There was a problem hiding this comment.
It's either include them here or bump the springboot version to where these are no longer issues
There was a problem hiding this comment.
Yes we are using SB 3.3.5 which brings in QOS 1.5.11 so is vulnerable. I will re-add these back, and look into upgrading SB to 3.3.8 or later.
|
|
||
| fun main(args: Array<String>) { | ||
| runApplication<EmsIntegrationApiApplication>(*args) | ||
| runApplication<RegisterCheckerApiApplication>(*args) |
There was a problem hiding this comment.
Should this not stay as EmsIntegrationApiApplication?
There was a problem hiding this comment.
I've made it like that in the final increment of this dev effort e.g. review 5.
| @@ -1,56 +0,0 @@ | |||
| package uk.gov.dluhc.emsintegrationapi.client | |||
There was a problem hiding this comment.
Why don't we need these api clients any more?
There was a problem hiding this comment.
We definitely do! It's the way I have put these PRs together. The main code is added back in as part of review 3.
There was a problem hiding this comment.
the "database" subfolder that is present in the eip-ero-register-checker-api that has "ReplicaAwareTransactionManager" and "TransactonRoutingDataSource" files is missing from this config file. Is there a reason they were left out or are they needed here to?
There was a problem hiding this comment.
Those files are in this branch. See here.
|
|
||
| class EmsIntegrationHeaderAuthenticationFilter( | ||
| class RegisterCheckerHeaderAuthenticationFilter( |
There was a problem hiding this comment.
Should this not stay as EmsIntegrationHeaderAuthenticationFilter?
There was a problem hiding this comment.
Change covered by review 3.
| PersonalDetailMapper::class | ||
| ] | ||
| ) | ||
| abstract class RegisterCheckResultMapper { |
There was a problem hiding this comment.
I think we should only be using mapstruct.Mappings for Enums mappings done to from Api, Dto, and etity objects should be manual
There was a problem hiding this comment.
if you see the Mappers section of docs/guidelines/general.md in the applications API, we should be using those guidelines, and updating prexisitng mappers where we can
There was a problem hiding this comment.
Thanks, it's a valid point, although this work was done before that guidance appeared. I could to raise a ticket to upgrade the merged service to the latest coding standards, and schedule it to be done after this work has been merged. I'm just keen to keep the review and merge process simple if possible
There was a problem hiding this comment.
Yup if you could raise a new ticket for doing this as part of this that would be great
There was a problem hiding this comment.
Raised JIRA https://mhclgdigital.atlassian.net/browse/EIP1-11807 for this
There was a problem hiding this comment.
As per the ticket:
RegisterCheckerAPI.yaml should be copied across alongside the EMSIntegrationAPI.yaml and not renamed/altered
Ref JIRA EIP1-11113
The feature commits on this branch cover import of the Register Checker API project setup, gradle, software versions (SB3 etc). as well as the commits under EIP1-11111 as the db migrations are a pre-requisite to this code working.
Most of the changes are files deleted in the branch under review as this is an interim step in the code migration.
This PR targets a proxy main branch to enable merging of 11113 and 11114 changes into one version of the code without changing the code deployed to the dev & test environments.