Critical code review PR - Last sprint handover checklist of 0.10.0 release #84

vishwa-vyom · 2024-12-13T10:54:51Z

This PR is created just to add the review comments as part of the critical code review task of last sprint handover checklist for release of 0.10.0 version.

** THIS PR SHOULD NOT BE MERGED **

) * [INJICERT-189] add mosip id plugin implementation module Signed-off-by: Challarao <[email protected]> * Updated push-trigger.yml Signed-off-by: Praful Rakhade <[email protected]> * Update push-trigger.yml Signed-off-by: Praful Rakhade <[email protected]> * Update push-trigger.yml Signed-off-by: Praful Rakhade <[email protected]> * Update push-trigger.yml Signed-off-by: Praful Rakhade <[email protected]> * [INJICERT-189] add esignet dependency for redis cache share Signed-off-by: Challarao <[email protected]> * [INJICERT-189] change package name and exclude esignet core transitive dependency Signed-off-by: Challarao <[email protected]> * [INJICERT-189] add test dependencies and fix workflow issue Signed-off-by: Challarao <[email protected]> --------- Signed-off-by: Praful Rakhade <[email protected]> Co-authored-by: Vishwa <[email protected]> Co-authored-by: Praful Rakhade <[email protected]> Co-authored-by: Chandra Keshav Mishra <[email protected]>

Signed-off-by: Challarao <[email protected]>

[INJICERT-189] make service classes as conditional beans

Signed-off-by: Challarao <[email protected]>

[INJICERT-189] add esignet integration api dependency

Signed-off-by: Challa Rao <[email protected]>

[MOSIP-33939] change esignet dependency versions for mosip ida plugin

* [INJICERT-186] add mock identity plugin Signed-off-by: Challarao <[email protected]> * [INJICERT-186] mock identity system call for getting details Signed-off-by: Challarao <[email protected]> * [INJICERT-186] add license Signed-off-by: Challarao <[email protected]> * [INJICERT-186] add test cases Signed-off-by: Challarao <[email protected]>

Signed-off-by: abhishek8shankar <[email protected]>

[DSD-5591] Updated push-trigger.yml

Signed-off-by: abhishek8shankar <[email protected]>

[DSD-5591] Updated push-trigger.yml

* [DSD-5591] add plugin Signed-off-by: Challarao <[email protected]> * [DSD-5591] change the artifact id and folder name Signed-off-by: Challarao <[email protected]>

Signed-off-by: Harsh Vardhan <[email protected]>

[INJICERT-444] merge release 0.2.x changes to develop

* [INJIMOB-1588] add mock mdoc Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] Get issuer certificate + keypair from local p12 file Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] Populate mock mDoc from DB Build mdoc from mock DB using the cached transaction and in case of any issue give hardcoded set of data Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1788] generate mdoc Library used - open-wallet-foundation-labs/identity-credetial Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] produce mso_mdoc VC from individualId provided Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] read crypto details for signing mso_mdoc vc Other changes: removing unused classes/ debug logs Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] remove debug logs , optimize imports Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] remove unused declared dependencies Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] add respository for identity-credential dependency Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] remove debug logs Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] refactor unused fields, renaming Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] git ignore .DS_Store Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] perform base64 url safe encoding Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] refactor variable name Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] Get issuer and ca cryto details from config property Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] revert IDE format changes Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] fix merge conflict issue Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] remove unused repo declaration Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] revert remove unused plugin Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1588] extract data generation for mdoc to a separate function Signed-off-by: KiruthikaJeyashankar <[email protected]> --------- Signed-off-by: KiruthikaJeyashankar <[email protected]> Signed-off-by: KiruthikaJeyashankar <[email protected]>

Signed-off-by: Harsh Vardhan <[email protected]>

* [INJICERT-434] Added mock data provider plugin for data model 2.0 Signed-off-by: piyush-shukla03_infosys <[email protected]> * Added test cases for mock data provider plugin Signed-off-by: Piyush7034 <[email protected]> * Changed name of plugin to mock-ida-dataprovider-plugin Signed-off-by: Piyush7034 <[email protected]> * Removed issuer from mock data response json Signed-off-by: Piyush7034 <[email protected]> --------- Signed-off-by: piyush-shukla03_infosys <[email protected]> Signed-off-by: Piyush7034 <[email protected]> Co-authored-by: piyush-shukla03_infosys <[email protected]>

…#74) Signed-off-by: Piyush7034 <[email protected]>

* [INJIMOB-1862] replace hardcoded dates with realtime date in mdoc generation other changes: modify issuing country for vc verification check Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1862] fix to have same doctype in document & MSO Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1862] modify issue and expiry date to have full date format Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-2160] add UUID as id to mdoc data Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-2160] add test for mdoc generator Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJICERT-533] use random uuid for mock VCs Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1862] remove printing UUID Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJIMOB-1862] extract constant for fullDate formatter Signed-off-by: KiruthikaJeyashankar <[email protected]> --------- Signed-off-by: KiruthikaJeyashankar <[email protected]>

* [INJICERT-587] add CSV plugin implementation * add unit tests for csv data plugin * add invalid id unit test Signed-off-by: Piyush7034 <[email protected]> * [INJICERT-587] remove unsued dependencies Signed-off-by: Harsh Vardhan <[email protected]> * [INJICERT-587] load CSV to memory at start Other minor changes: * add support for loading CSV from classpath, filepath & a URL(spring config-server) * add CI Signed-off-by: Harsh Vardhan <[email protected]> * [INJICERT-587] set the id field for the credential via config Signed-off-by: Harsh Vardhan <[email protected]> --------- Signed-off-by: Piyush7034 <[email protected]> Signed-off-by: Harsh Vardhan <[email protected]> Signed-off-by: Harsh Vardhan <[email protected]> Co-authored-by: Piyush7034 <[email protected]>

* [INJICERT-587] Fixed file access through classpath Upgraded version for mockito-core from 3.6.3 to 5.7.0 Signed-off-by: Piyush7034 <[email protected]> * Upgraded mockito-core version from 5.7.0 to 5.11.0 due to security vulnerability Signed-off-by: Piyush7034 <[email protected]> * Update mock-csv-dataprovider-plugin/src/main/java/io/mosip/certify/mockcsvdataprovider/integration/service/DataProviderService.java Co-authored-by: Harsh Vardhan <[email protected]> Signed-off-by: Hitesh Jain <[email protected]> --------- Signed-off-by: Piyush7034 <[email protected]> Signed-off-by: Hitesh Jain <[email protected]> Co-authored-by: Hitesh Jain <[email protected]> Co-authored-by: Harsh Vardhan <[email protected]>

* [INJICERT-587] Fixed file access through classpath Upgraded version for mockito-core from 3.6.3 to 5.7.0 Signed-off-by: Piyush7034 <[email protected]> * Upgraded mockito-core version from 5.7.0 to 5.11.0 due to security vulnerability Signed-off-by: Piyush7034 <[email protected]> * [INJICERT-587] Merged mock-csv-dataprovider into mock-certify-plugin Signed-off-by: Piyush7034 <[email protected]> * Removed build flows for deleted mock csv plugin project Signed-off-by: Piyush7034 <[email protected]> --------- Signed-off-by: Piyush7034 <[email protected]>

* Added postgres data provider plugin Signed-off-by: Piyush7034 <[email protected]> * Simplified plugin implementation with property config Signed-off-by: Piyush7034 <[email protected]> * Added postgres plugin in github workflows Signed-off-by: Piyush7034 <[email protected]> * Removed id field and @value split for table fields Signed-off-by: Piyush7034 <[email protected]> * Added scope query mapping for generating different vcs with different scopes Signed-off-by: Piyush7034 <[email protected]> * Changed mock-postgres-plugin name to postgres-plugin Signed-off-by: Piyush7034 <[email protected]> * Removed unused repo from dependencies Signed-off-by: Piyush7034 <[email protected]> * Replaced object[] with map for query resullt Signed-off-by: Piyush7034 <[email protected]> --------- Signed-off-by: Piyush7034 <[email protected]> Signed-off-by: Vishwa <[email protected]>

.github/workflows/push-trigger.yml

...tify-plugin/src/main/java/io.mosip.certify.mock.integration/service/DataProviderService.java

...lugin/src/main/java/io.mosip.certify.mock.integration/service/MockCSVDataProviderPlugin.java

mock-ida-dataprovider-plugin/pom.xml

mock-certify-plugin/src/main/java/io/mosip/certify/util/CSVReader.java

vishwa-vyom · 2024-12-13T13:25:18Z

...ify-plugin/src/main/java/io.mosip.certify.mock.integration/service/MockVCIssuancePlugin.java

+		data.put("document_number", documentNumber);
+		data.put("driving_privileges",new HashMap<>(){{
+			put("vehicle_category_code","A");
+		}});


There is a consistency issue, the LD VC generator from this class will create a VC with data from mock identity system, but this has hardcoded data. I accept this is a mock, but if we create a new vciplugin class with mdoc impl, we might be little better.

hello @vishwa-vyom
Are we suggesting like MdocVCIPlugin class which will encapsulate both the data and generation logic?

Lets create a new Plugin as MDocMockVCIPlugin.java and move all the code to this class

Is MDocMockVCIPlugin also expected to implement VCIssuancePlugin and when mdoc is required config will be updated to point to the MdocVCIPlugin?
If this change is coming we will not be able to issue both ldp_vc as well as mdoc from same plugin, is this expected here? Asking since previously we were having mock-certify-plugin issuing both mdocs and ldp_vc format VCs

vishwa-vyom · 2024-12-13T13:27:07Z

mock-certify-plugin/src/main/java/io/mosip/certify/mock/integration/mocks/MdocGenerator.kt

+import java.util.*
+
+
+class MdocGenerator {


Do we really need this in kotlin code, not possible to keep this also in java ?

@KiruthikaJeyashankar please check this suggestion

Now tried modifying to Java code and unit tests are passing. This can be modified to Java to maintain consistency

this is great, so then we can remove current file changes and do all the restructuring as part of Inji mobile release or we are good to include in the current Inji certify release ?

…se and last sprint handover review comments (#85) * [INJICERT-657] Removed mock-ida-dataprovider-plugin project as it is not a part of 0.10.0 certify release Signed-off-by: Piyush7034 <[email protected]> * Added code review changes for release Signed-off-by: Piyush7034 <[email protected]> --------- Signed-off-by: Piyush7034 <[email protected]> Signed-off-by: Vishwa <[email protected]>

...lugin/src/main/java/io.mosip.certify.mock.integration/service/MockCSVDataProviderPlugin.java

+        if (csvRegistryURI.startsWith("http")) {
+            // download the file to a path: usecase(docker, spring cloud config)
+            filePath = restTemplate.execute(csvRegistryURI, HttpMethod.GET, null, resp -> {
+                File ret = File.createTempFile("download", "tmp");


Signed-off-by: Piyush7034 <[email protected]>

[INJICERT-657] Removed mdoc and mdl related code

Signed-off-by: Piyush7034 <[email protected]> Signed-off-by: Vishwa <[email protected]>

* [INJICERT-695] add Mock mdoc VCI plugin Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJICERT-695] extract constant for VCFormat Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJICERT-695] modify property name of mdoc issuer key certificate mosip.certify.mock.vciplugin.issuer.key-cert changed to mosip.certify.mock.vciplugin.mdoc.issuer-key-cert Signed-off-by: KiruthikaJeyashankar <[email protected]> * [INJICERT-695] refactor - rename class Signed-off-by: KiruthikaJeyashankar <[email protected]> --------- Signed-off-by: KiruthikaJeyashankar <[email protected]> Signed-off-by: Vishwa <[email protected]>

In case of IndividualId not stored, documentNumber value becomes null and mdoc generated will have the documentNumber field as empty Signed-off-by: KiruthikaJeyashankar <[email protected]> Signed-off-by: Vishwa <[email protected]>

challabeehyv and others added 30 commits June 18, 2024 14:33

[INJICERT-189] make service classes as conditional beans

a8d48d3

Signed-off-by: Challarao <[email protected]>

Merge pull request #34 from challabeehyv/mosipid-conditional-beans

c54607e

[INJICERT-189] make service classes as conditional beans

[INJICERT-189] add esignet integration api dependency

b42ad84

Signed-off-by: Challarao <[email protected]>

Merge pull request #36 from challabeehyv/esignet-dependency-mosipid

d00c2f6

[INJICERT-189] add esignet integration api dependency

[MOSIP-33939] change esignet dependency versions for mosip ida plugin

462be90

Signed-off-by: Challa Rao <[email protected]>

Merge pull request #37 from challabeehyv/develop

9457ba9

[MOSIP-33939] change esignet dependency versions for mosip ida plugin

[DSD-5591] Updated push-trigger.yml

e3f7ab1

Signed-off-by: abhishek8shankar <[email protected]>

Merge pull request #40 from abhishek8shankar/develop

6fa688f

[DSD-5591] Updated push-trigger.yml

[DSD-5591] Updated push-trigger.yml

6ed7586

Signed-off-by: abhishek8shankar <[email protected]>

[DSD-5591] Updated push-trigger.yml

e4693ee

Signed-off-by: abhishek8shankar <[email protected]>

[DSD-5591] Updated push-trigger.yml

1da9d75

Signed-off-by: abhishek8shankar <[email protected]>

Merge pull request #41 from abhishek8shankar/develop

14eaef3

[DSD-5591] Updated push-trigger.yml

[DSD-5591] add plugin (#42)

a96fada

* [DSD-5591] add plugin Signed-off-by: Challarao <[email protected]> * [DSD-5591] change the artifact id and folder name Signed-off-by: Challarao <[email protected]>

[INJICERT-317] rename credential.type to MockVerifiableCredential (#43)

fa0aee7

Signed-off-by: Harsh Vardhan <[email protected]>

[INJICERT-444] update to release URL

6720dd8

Signed-off-by: Harsh Vardhan <[email protected]>

Merge branch 'develop' into rel

60d4697

Signed-off-by: Harsh Vardhan <[email protected]>

[INJICERT-444] use java-11 for sunbird esignet plugin

7bcd0d9

Signed-off-by: Harsh Vardhan <[email protected]>

[INJICERT-444] bump up the version to 0.3.0

79d0a6a

Signed-off-by: Harsh Vardhan <[email protected]>

[INJICERT-444] bump down version due to artifact unavailability

c38d8da

Signed-off-by: Harsh Vardhan <[email protected]>

Merge pull request #64 from vharsh/rel

7f116ef

[INJICERT-444] merge release 0.2.x changes to develop

[INJICER-444] prepare for 0.10.0 release (#71)

563a383

[INJICERT-316] remove null field in the Mock VC (#69)

1e20150

Signed-off-by: Harsh Vardhan <[email protected]>

[INJICERT-499] Added JsonObject in place of HashMap for identity data (…

b329ca7

…#74) Signed-off-by: Piyush7034 <[email protected]>

Piyush7034 added 2 commits November 14, 2024 14:44

vishwa-vyom commented Dec 13, 2024

View reviewed changes

vishwa-vyom mentioned this pull request Dec 14, 2024

[INJICERT-657] Remove mock-ida-dataprovider-plugin project from release and last sprint handover review comments #85

Merged

github-advanced-security bot found potential problems Dec 14, 2024

View reviewed changes

KiruthikaJeyashankar mentioned this pull request Dec 16, 2024

[INJIMOB-2471] remove id field from mock VCs #86

Merged

Piyush7034 and others added 5 commits December 17, 2024 12:15

Removed mdoc and mdl related code

f352d7d

Signed-off-by: Piyush7034 <[email protected]>

Merge pull request #88 from Infosys/remove-mdoc

9c29bb9

[INJICERT-657] Removed mdoc and mdl related code

[INJICERT-657] Added jakarta dependency for build fix (#92)

e50fa83

Signed-off-by: Piyush7034 <[email protected]> Signed-off-by: Vishwa <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Critical code review PR - Last sprint handover checklist of 0.10.0 release #84

Critical code review PR - Last sprint handover checklist of 0.10.0 release #84

vishwa-vyom commented Dec 13, 2024

vishwa-vyom Dec 13, 2024 •

edited

Loading

KiruthikaJeyashankar Dec 16, 2024

vishwa-vyom Dec 16, 2024

KiruthikaJeyashankar Dec 16, 2024 •

edited

Loading

vishwa-vyom Dec 13, 2024

Piyush7034 Dec 13, 2024

KiruthikaJeyashankar Dec 16, 2024

vishwa-vyom Dec 16, 2024

Critical code review PR - Last sprint handover checklist of 0.10.0 release #84

Are you sure you want to change the base?

Critical code review PR - Last sprint handover checklist of 0.10.0 release #84

Conversation

vishwa-vyom commented Dec 13, 2024

vishwa-vyom Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

KiruthikaJeyashankar Dec 16, 2024

Choose a reason for hiding this comment

vishwa-vyom Dec 16, 2024

Choose a reason for hiding this comment

KiruthikaJeyashankar Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

vishwa-vyom Dec 13, 2024

Choose a reason for hiding this comment

Piyush7034 Dec 13, 2024

Choose a reason for hiding this comment

KiruthikaJeyashankar Dec 16, 2024

Choose a reason for hiding this comment

vishwa-vyom Dec 16, 2024

Choose a reason for hiding this comment

vishwa-vyom Dec 13, 2024 •

edited

Loading

KiruthikaJeyashankar Dec 16, 2024 •

edited

Loading