Create and Update the validation service #97

asmataamallah25 · 2025-01-16T08:18:49Z

No description provided.

flopezag · 2025-01-21T08:17:06Z

Validation_service/README.md

+The validation service implements a three-step verification protocol.
+
+### Initial Validation Request
+The client initiates the verification process by submitting a POST request to the Canis Major Validator service. 


Canis Major Validator service or Canis Major Validation service?

It is Canis Major Validation service. I will change it.

flopezag · 2025-01-21T08:18:48Z

Validation_service/pom.xml

+            </plugin>
+        </plugins>
+    </build>
+</project>


add a new line at the end of the file.

flopezag · 2025-01-21T08:20:42Z

Validation_service/src/main/java/validator/client/ValidationHttpClient.java

+            // Adds Link header for JSON-LD context
+            .header("Link", "<https://raw.githubusercontent.com/smart-data-models/dataModel.DistributedLedgerTech/master/context.jsonld>; rel=\"http://www.w3.org/ns/json-ld#context\"; type=\"application/ld+json\"")
+            // Sets the NGSILD-Tenant header to "orion"
+            .header("NGSILD-Tenant", "orion")


Just a question, Is this NGSILD-Tenant fixed in Canis Major?

Upon reviewing the code in the StepDefinition.java file, I can confirm that in the Canis Major blockchain adaptor, the NGSI-LD tenant is indeed configured as a static final constant:
public static final String NGSILD_TENANT = "orion";

The StepDefinition.java is just defining the steps and calls for Integration-Testing. The tenant is not fixed and needs to be configurable.

flopezag · 2025-01-21T08:22:49Z

Validation_service/src/main/java/validator/client/ValidationHttpClient.java

+        // Sends the request and returns the response body as a String
+        return client.send(request, HttpResponse.BodyHandlers.ofString()).body();
+    }
+}


Add a new line at the end of the file.

flopezag · 2025-01-21T08:27:06Z

Validation_service/src/main/java/validator/model/ValidationResult.java

+    public List<String> getMessages() {
+        return messages;
+    }
+}


add newline at the end of the file

flopezag · 2025-01-21T08:29:01Z

Validation_service/src/main/java/validator/service/ValidationService.java

+         }
+     }
+ }
+


add newline to the end of the file

flopezag · 2025-01-21T08:32:08Z

Validation_service/validation_service.png

it is missing in this diagram the message 4) with the communication of the results.

This is the diagram you sent me in the chat. I can make a new diagram and add this step

flopezag · 2025-01-21T08:37:44Z

Validation_service/src/main/java/validator/controller/ValidationController.java

+    public HttpResponse<ValidationResult> compareResponses(@Body CompareRequest request) {
+        // Call the validation service to perform the validation
+        ValidationResult result = validationService.validateEntity(request.getEntityId());
+        return HttpResponse.ok(result);


You are returning always a HttpResponse.ok, but what's about an error returning in the validationService.validateEntity function?

I will add error handling in this case

flopezag · 2025-01-21T08:40:27Z

it/docker-compose/clean.sh

@@ -0,0 +1,12 @@
+#Stop and remove all Docker containers:


at this point, I prefer to have the two options that let the user select which option he wants to use, let's say, request to the user which option to use 1) Clean delete of the docker compose resources, or 2) Delete all docker resources.

pulledtim · 2025-01-22T09:38:47Z

Validation Service/README.md

@@ -0,0 +1,22 @@
+# Validation service


Why didn't you put this in a separate repo? Currently the service is placed in the repo but not included in the build system ( neither as submodule nor in the github flows).

pulledtim · 2025-01-22T09:43:33Z

Validation Service/pom.xml

+
+    <build>
+        <plugins>
+            <plugin>


What is the further plan? At the moment no container will be build. Will there be another PR adding functionality?

pulledtim · 2025-01-22T09:54:22Z

Validation Service/src/main/java/validator/client/ValidationHttpClient.java

+@Singleton
+public class ValidationHttpClient {
+    // Creates a single HttpClient instance to be reused for all requests
+    private final HttpClient client = HttpClient.newHttpClient();


Might be to much for the current level but its possible to use a generated client for the ngsi-ld standard. Either you generate it yourself in this project -> https://github.com/wistefan/ngsi-ld-java-mapping/blob/main/pom.xml#L400-L442 or you could use an even more abstract approach using the lib to access ngsi-ld context broker via a convenient repository: https://github.com/wistefan/ngsi-ld-java-mapping/tree/main

pulledtim · 2025-01-22T09:55:00Z

Validation Service/src/main/java/validator/client/ValidationHttpClient.java

+    private final HttpClient client = HttpClient.newHttpClient();
+
+    // Method to fetch data from Orion Context Broker
+    public String fetchOrionData(String entityId) throws Exception {


Canis Major does not request Orion-ld but a NGSI_LD compatible broker right? Then the name would be misleading

pulledtim · 2025-01-22T09:55:33Z

Validation Service/src/main/java/validator/client/ValidationHttpClient.java

+        // Builds an HTTP request:
+        HttpRequest request = HttpRequest.newBuilder()
+            // Sets the URI for the request, querying for DLTtxReceipt entities with matching refEntity
+            .uri(new URI("http://localhost:1026/ngsi-ld/v1/entities/?type=DLTtxReceipt&q=refEntity==" + entityId))


The broker location should be configurable

pulledtim · 2025-01-22T09:56:12Z

Validation Service/src/main/java/validator/client/ValidationHttpClient.java

+            // Adds Link header for JSON-LD context
+            .header("Link", "<https://raw.githubusercontent.com/smart-data-models/dataModel.DistributedLedgerTech/master/context.jsonld>; rel=\"http://www.w3.org/ns/json-ld#context\"; type=\"application/ld+json\"")
+            // Sets the NGSILD-Tenant header to "orion"
+            .header("NGSILD-Tenant", "orion")


Tenant should be configurable too

pulledtim · 2025-01-22T10:00:51Z

Validation Service/src/main/java/validator/client/ValidationHttpClient.java

+            .build();
+
+        // Sends the request and returns the response body as a String
+        return client.send(request, HttpResponse.BodyHandlers.ofString()).body();


For future maintainability the use of async mechanisms would be nice, or else we can run into annoying errors when running it

pulledtim · 2025-01-22T10:01:43Z

Validation Service/src/main/java/validator/client/ValidationHttpClient.java

+    }
+
+    // Method to fetch data from a blockchain service
+    public String fetchBlockchainData(String entityId) throws Exception {


Is it really a blockchain service (somehow generic) or is it CanisMajor?

pulledtim · 2025-01-22T10:02:26Z

Validation Service/src/main/java/validator/client/ValidationHttpClient.java

+        // Builds an HTTP request:
+        HttpRequest request = HttpRequest.newBuilder()
+            // Sets the URI for the blockchain service endpoint
+            .uri(new URI("http://localhost:4000/entity/" + entityId))


Configurable please 😄

pulledtim · 2025-01-22T10:04:27Z

Validation Service/src/main/java/validator/controller/ValidationController.java

+import validator.service.ValidationService;
+import validator.model.ValidationResult;
+
+@Controller("/service/v1/validation")


Would be nice to have an openapi file and generate the api from it (like here https://github.com/FIWARE/CanisMajor/blob/master/src/main/java/org/fiware/canismajor/rest/EntitiesController.java#L21)

pulledtim · 2025-01-22T10:06:08Z

Validation Service/src/main/java/validator/controller/ValidationController.java

+    }
+
+    // Inner class to represent the request body
+    public static class CompareRequest {


Would be generated too

pulledtim · 2025-01-22T10:08:12Z

Validation Service/src/main/java/validator/service/ValidationService.java

+
+ @Singleton
+ public class ValidationService {
+     private static final Logger logger = LoggerFactory.getLogger(ValidationService.class);


Just for fun you should check out Lombok. Its a very convenient tool to reduce boilerplate

pulledtim · 2025-01-22T10:09:14Z

Validation Service/src/main/java/validator/service/ValidationService.java

+ public class ValidationService {
+     private static final Logger logger = LoggerFactory.getLogger(ValidationService.class);
+     private final ValidationHttpClient validationHttpClient;
+     private final ObjectMapper mapper = new ObjectMapper();


Micronaut in its basic/default config lets you inject an ObjectMapper afaik

pulledtim · 2025-01-22T10:10:47Z

Validation Service/src/main/java/validator/service/ValidationService.java

+
+             if (orionData == null || orionData.isEmpty()) {
+                 logger.error("No data found from Orion for entityId: {}", entityId);
+                 return new ValidationResult(false, List.of("No data found from Orion"));


Suggested change

return new ValidationResult(false, List.of("No data found from Orion"));

return new ValidationResult(false, List.of("No data found from Context Broker"));

pulledtim · 2025-01-22T10:17:28Z

Validation Service/src/main/java/validator/client/ValidationHttpClient.java

+
+// Marks this class as a singleton - only one instance will exist in the application
+@Singleton
+public class ValidationHttpClient {


It would be nicer if the methods of this class would return meaningful objects rather than unchecked strings

pulledtim · 2025-01-22T10:18:28Z

Validation Service/src/main/java/validator/service/ValidationService.java

+             JsonNode orionNode = mapper.readTree(orionData);
+             JsonNode blockchainNode = mapper.readTree(blockchainData);
+
+             JsonNode txReceipts = orionNode.get("TxReceipts").get("value");


Possible NPE, better have the clients return checked objects

wistefan · 2025-01-22T15:49:40Z

Hi @asmataamallah25 ,
I think there is quite some misconception of validation in the context of CanisMajor. The proposed service does not validate anything. It just retrieves the same entity through two different endpoints(directly from the broker and additional from canis major, which also requests it from the same broker). As far as I understand the readme, the service should "verify transaction integrity".
In order to do so, you would have to take a different approach:

The api should receive the entity to be checked and its transaction receipt.
The service need to access the blockchain(not CanisMajor, since it only writes, but not reads) and retrieve the linked transaction.
Retrieve the entity hash from the transaction.
Build the hash of the entity to be checked(e.g. the one that was received), the same hash method that was used for writing needs to be used
Compare the hash in the transaction and the hash that was build.

I hope it does not sound rude, but the code in the PR does not add any validation functionality. Since you compare the exact same entities, from the exact same source, it does not add any value.

asmataamallah25 · 2025-01-23T09:52:46Z

Hello Stefan, Thank you for taking the time to read my code and give me feedback. I appreciate it. I will make sure to follow each of the steps you provided and adjust the code. I appreciate every opportunity to learn. Thank you once again.

…

On Wed, Jan 22, 2025, 4:50 PM Stefan Wiedemann ***@***.***> wrote: Hi @asmataamallah25 <https://github.com/asmataamallah25> , I think there is quite some misconception of validation in the context of CanisMajor. The proposed service does not validate anything. It just retrieves the same entity through two different endpoints(directly from the broker and additional from canis major, which also requests it from the same broker). As far as I understand the readme, the service should "verify transaction integrity". In order to do so, you would have to take a different approach: 1. The api should receive the entity to be checked and its transaction receipt <https://github.com/FIWARE/CanisMajor/blob/master/api/api.yaml#L855>. 2. The service need to access the blockchain(not CanisMajor, since it only writes, but not reads) and retrieve the linked transaction. 3. Retrieve the entity hash from the transaction. 4. Build the hash of the entity to be checked(e.g. the one that was received), the same hash method that was used for writing <https://github.com/FIWARE/CanisMajor/blob/master/src/main/java/org/fiware/canismajor/dlt/EthereumService.java#L119> needs to be used 5. Compare the hash in the transaction and the hash that was build. I hope it does not sound rude, but the code in the PR does not add any validation functionality. Since you compare the exact same entities, from the exact same source, it does not add any value. — Reply to this email directly, view it on GitHub <#97 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BIHMIN3IAK3MWKGDNC7NN332L643XAVCNFSM6AAAAABVJALUUOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMBXGYYDONBXGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

asmataamallah25 and others added 16 commits December 6, 2024 14:02

docs: Add Canis Major workflow test commands

5fc76ea

Updates Canis Major workflow test commands

481b382

Validation service: first iteration

4e5dfc3

Updated test commands documentation and changed code location

95b565b

Delete validator/Tests.md -

9cc270d

Update Tests.md

e0551ca

Delete validator/Tests.md

8942860

Upload the most recent version of Tests.md

fec88d0

Delete validator/Tests.md

ee5749e

Rename the file as README.md

4ba7cb3

Merge branch 'master' into validator

ce3dde2

Merge branch 'master' into validator

be32c2f

Update README.md

87ec990

Update validation service

f3e0665

Resolve conflicts

a6115c2

Delete validator directory

42d298b

asmataamallah25 requested review from flopezag and jason-fox January 16, 2025 08:19

flopezag added the patch Should be applied for dependency updates and small bugfixes. label Jan 21, 2025

flopezag requested changes Jan 21, 2025

View reviewed changes

asmataamallah25 added 3 commits January 21, 2025 14:21

Update validation service

f41e172

Update Validation Service

aef7f85

Update Validation service

90fc1bd

asmataamallah25 marked this pull request as draft January 22, 2025 08:56

asmataamallah25 requested a review from pulledtim January 22, 2025 08:56

pulledtim reviewed Jan 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create and Update the validation service #97

Create and Update the validation service #97

asmataamallah25 commented Jan 16, 2025

flopezag Jan 21, 2025

asmataamallah25 Jan 21, 2025

flopezag Jan 21, 2025

flopezag Jan 21, 2025

asmataamallah25 Jan 21, 2025

wistefan Jan 23, 2025

flopezag Jan 21, 2025

flopezag Jan 21, 2025

flopezag Jan 21, 2025

flopezag Jan 21, 2025

asmataamallah25 Jan 21, 2025

flopezag Jan 21, 2025

asmataamallah25 Jan 21, 2025

flopezag Jan 21, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

pulledtim Jan 22, 2025

wistefan commented Jan 22, 2025

asmataamallah25 commented Jan 23, 2025 via email

	return new ValidationResult(false, List.of("No data found from Orion"));
	return new ValidationResult(false, List.of("No data found from Context Broker"));

Create and Update the validation service #97

Are you sure you want to change the base?

Create and Update the validation service #97

Conversation

asmataamallah25 commented Jan 16, 2025

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

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

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

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

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

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

wistefan commented Jan 22, 2025

asmataamallah25 commented Jan 23, 2025 via email