Feature/csr retrieve entity #1246

thomasBousselin · 2024-10-08T15:56:48Z

Context Source registration working for retrieve entity.

some minimal testing ok with postman and the test suite

missing :

local parameter
unit test
more complete testing with the test suite

github-actions · 2024-10-08T16:04:26Z

Test Results

25 files - 40 25 suites - 40 37s ⏱️ -44s
355 tests - 677 355 ✅ - 676 0 💤 ±0 0 ❌ - 1
355 runs - 716 355 ✅ - 715 0 💤 ±0 0 ❌ - 1

Results for commit deca20c. ± Comparison against base commit 32c5133.

♻️ This comment has been updated with latest results.

bobeal

A first quick review

search-service/src/main/kotlin/com/egm/stellio/search/entity/service/EntityQueryService.kt

bobeal · 2024-10-09T09:35:11Z

search-service/src/main/kotlin/com/egm/stellio/search/csr/model/CSRFilters.kt

+    fun buildWHEREStatement(): String {
+        val idsMatcher = if (ids.isNotEmpty()) "(" +
+            "entity_info.id is null" +
+            " OR entity_info.id in ('${ids.joinToString("', '")}')" +
+            ") AND (" +
+            "entity_info.\"idPattern\" is null OR " +
+            ids.map { "'$it' ~ entity_info.\"idPattern\"" }.joinToString(" OR ") +
+            ")"
+        else "true"
+        return idsMatcher


a bit ugly TBH. at least use multiline strings.

why not put it in CSRService like is done for other services?

at least use multiline strings.

Yes

why not put it in CSRService like is done for other services?

I think it make more sense to put it there especially since the CSRService already do lot of stuff.
Do you see a reason why not?

consistency with other services. And CSRService is only 230 lines...

search-service/src/main/kotlin/com/egm/stellio/search/csr/model/CSRFilters.kt

...rvice/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceRegistrationService.kt

search-service/src/main/kotlin/com/egm/stellio/search/entity/web/EntityHandler.kt

search-service/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceUtils.kt

bobeal

PS: I still have to look at the merge algorithm

...rvice/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceRegistrationService.kt

bobeal · 2024-10-13T09:13:59Z

search-service/src/main/kotlin/com/egm/stellio/search/entity/web/EntityHandler.kt


-        val filteredExpandedEntity = ExpandedEntity(
-            expandedEntity.filterAttributes(queryParams.attrs, queryParams.datasetId)
+        val matchingCSR = contextSourceRegistrationService.getContextSourceRegistrations(


here you also have to filter CSRs that match the current operation (so retrieveEntity in the operations or federationOps in the operation groups)

filtering the results from the call to the service is not a clean way to do it. it should be part of the call to the service.

search-service/src/test/kotlin/com/egm/stellio/search/entity/web/EntityHandlerTests.kt

shared/src/main/kotlin/com/egm/stellio/shared/util/ApiResponses.kt

shared/src/main/kotlin/com/egm/stellio/shared/model/ErrorResponse.kt

search-service/src/main/kotlin/com/egm/stellio/search/entity/web/EntityHandler.kt

…some typos and naming

search-service/src/test/kotlin/com/egm/stellio/search/csr/service/ContextSourceUtilsTests.kt

...rvice/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceRegistrationService.kt

context-source-docker-compose.yml

...rvice/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceRegistrationService.kt

search-service/src/main/kotlin/com/egm/stellio/search/entity/web/EntityHandler.kt

bobeal · 2024-10-19T16:30:16Z

search-service/src/main/kotlin/com/egm/stellio/search/entity/web/EntityHandler.kt


-        val filteredExpandedEntity = ExpandedEntity(
-            expandedEntity.filterAttributes(queryParams.attrs, queryParams.datasetId)
+        val matchingCSR = contextSourceRegistrationService.getContextSourceRegistrations(


filtering the results from the call to the service is not a clean way to do it. it should be part of the call to the service.

shared/src/main/kotlin/com/egm/stellio/shared/model/ErrorResponse.kt

search-service/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceUtils.kt

shared/src/main/kotlin/com/egm/stellio/shared/model/ApiExceptions.kt

… merging return warnings

search-service/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceCaller.kt

bobeal · 2024-10-26T14:43:05Z

search-service/src/main/kotlin/com/egm/stellio/search/csr/model/ContextSourceRegistration.kt

@@ -23,16 +24,25 @@ import java.util.regex.Pattern
 data class ContextSourceRegistration(
    val id: URI = "urn:ngsi-ld:ContextSourceRegistration:${UUID.randomUUID()}".toUri(),
    val endpoint: URI,
+    val registrationName: String? = null,


don't forget to "bind" in CSRService (create and get operations)

search-service/src/test/kotlin/com/egm/stellio/search/csr/service/ContextSourceCallerTests.kt

search-service/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceUtils.kt

bobeal · 2024-10-27T17:46:32Z

search-service/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceUtils.kt

+        remoteEntitiesWithMode.sortedBy { (_, csr) -> csr.isAuxiliary() }
+            .forEach { (entity, csr) ->
+                mergedEntity.putAll(
+                    getMergeNewValues(mergedEntity, entity, csr).toIor().toIorNel().bind()


this deserves a test to be sure we get the expected result if one or two merges fail

bobeal · 2024-10-28T06:50:47Z

search-service/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceUtils.kt

+        localEntity: CompactedEntity,
+        remoteEntity: CompactedEntity,


when fixing merge conflicts, the terminology is current and incoming

…omments

sonarcloud · 2024-10-30T14:08:12Z

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
74.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

bobeal · 2024-11-01T08:54:52Z

search-service/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceCaller.kt

-
+        params.remove(QUERY_PARAM_GEOMETRY_PROPERTY)
+        params.remove(QUERY_PARAM_OPTIONS) // only request normalized
+        params.remove(QUERY_PARAM_LANG) // todo not sure its needed


it is handled when doing the final representation, so yes, it should be removed.

bobeal · 2024-11-01T08:55:37Z

search-service/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceUtils.kt

-typealias DataSetId = String?
-typealias AttributeByDataSetId = Map<DataSetId, CompactedAttributeInstance>
-
+typealias AttributeByDataSetId = Map<String?, CompactedAttributeInstance>


still DataSetId -> DatasetId

bobeal · 2024-11-01T08:59:28Z

search-service/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceUtils.kt

@@ -110,7 +111,7 @@ object ContextSourceUtils {
        if (values.size == 1) values[0] else values
    }

-    // do not work with CORE MEMBER since they are nor list nor map


I would instead say: only meant to work with attributes under the normalized representation

bobeal · 2024-11-01T09:01:00Z

search-service/src/test/kotlin/com/egm/stellio/search/csr/service/ContextSourceCallerTests.kt

+        )
+        verify(
+            getRequestedFor(urlPathEqualTo(path))
+                .withHeader(ACCEPT, notContaining("GEO_JSON_MEDIA_TYPE"))


the value of the header is application/geo+json (and not GEO_JSON_MEDIA_TYPE)

bobeal · 2024-11-01T09:01:57Z

search-service/src/test/kotlin/com/egm/stellio/search/csr/service/ContextSourceCallerTests.kt

+    }
+
+    @Test
+    fun `getDistributedInformation should not ask context source for a GEO_JSON`() = runTest {


getDistributedInformation should not ask context source for a GeoJson representation

bobeal · 2024-11-01T09:04:48Z

search-service/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceUtils.kt

    }

-    private fun getMergeNewValues(
+    fun getMergeNewValues(


if it is just to be able to call it in the test, you can instead make it internal

bobeal · 2024-11-01T09:06:59Z

search-service/src/test/kotlin/com/egm/stellio/search/entity/web/EntityHandlerTests.kt

@@ -804,6 +808,43 @@ class EntityHandlerTests {
            )
    }

+    @Test
+    fun `get entity by id should return the warnings send by the csr and update the csr status`() {


... the warning sent by ...

thomasBousselin added 3 commits October 3, 2024 10:38

wip: not tested merge entities function

41e4af9

wip: not tested merge entities function

c7bff1e

feat: retrieve Entity Working

cc6de35

github-actions bot added the feature New feature or request label Oct 8, 2024

bobeal reviewed Oct 9, 2024

View reviewed changes

thomasBousselin force-pushed the feature/csr-retrieve-entity branch from 21561cc to 9d2ee23 Compare October 10, 2024 14:31

feat: first error handling and fix pr comments

e89f92f

thomasBousselin force-pushed the feature/csr-retrieve-entity branch from 9d2ee23 to e89f92f Compare October 10, 2024 15:10

feat: test for mergeEntity

3de35ba

bobeal reviewed Oct 13, 2024

View reviewed changes

fix: working Where statement for CSR / add some tests on Query CSR / …

53f0b87

…some typos and naming

bobeal reviewed Oct 13, 2024

View reviewed changes

search-service/src/test/kotlin/com/egm/stellio/search/csr/service/ContextSourceUtilsTests.kt Outdated Show resolved Hide resolved

thomasBousselin added 3 commits October 14, 2024 15:49

feat: fix merging returning lists and PR comments

c8bf170

feat: fix merging returning lists and PR comments

e8ccf0c

feat: updating

8387130

bobeal reviewed Oct 17, 2024

View reviewed changes

...rvice/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceRegistrationService.kt Outdated Show resolved Hide resolved

...rvice/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceRegistrationService.kt Outdated Show resolved Hide resolved

bobeal reviewed Oct 20, 2024

View reviewed changes

thomasBousselin and others added 8 commits October 21, 2024 17:52

feat: MR comment - clearer name - no warning impact in ApiException -…

dd31576

… merging return warnings

feat: get warnings from merge

3bcf7b3

feat: separate context source utils + work with sysAttrs

690fd59

feat: warning following rfc7234 + docker-compose for csr + fix tests

f38c04b

feat: doc csr launch

129228a

feat: doc csr launch

b07e032

chore: slight refactoring of the docker compose context source instance

6d4831a

chore: misc typo, wording and naming

8740239

bobeal reviewed Oct 27, 2024

View reviewed changes

bobeal reviewed Oct 28, 2024

View reviewed changes

feat: always call context source for normalized representation + PR c…

f74ceec

…omments

feat: registrationName in CSR + Warning fixes

deca20c

bobeal reviewed Nov 1, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/csr retrieve entity #1246

Feature/csr retrieve entity #1246

thomasBousselin commented Oct 8, 2024

github-actions bot commented Oct 8, 2024 •

edited

Loading

bobeal left a comment

bobeal Oct 9, 2024

thomasBousselin Oct 10, 2024

bobeal Oct 13, 2024

bobeal left a comment

bobeal Oct 13, 2024

bobeal Oct 19, 2024

bobeal Oct 19, 2024

bobeal Oct 26, 2024

bobeal Oct 27, 2024

bobeal Oct 28, 2024

sonarcloud bot commented Oct 30, 2024

bobeal Nov 1, 2024

bobeal Nov 1, 2024

bobeal Nov 1, 2024

bobeal Nov 1, 2024

bobeal Nov 1, 2024

bobeal Nov 1, 2024

bobeal Nov 1, 2024

Feature/csr retrieve entity #1246

Are you sure you want to change the base?

Feature/csr retrieve entity #1246

Conversation

thomasBousselin commented Oct 8, 2024

github-actions bot commented Oct 8, 2024 • edited Loading

Test Results

bobeal 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

bobeal 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 30, 2024

Quality Gate passed

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

github-actions bot commented Oct 8, 2024 •

edited

Loading