New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding a context source layer #1308

Closed

thomasBousselin wants to merge 3 commits into develop from refacto/context-source-layer

Contributor

thomasBousselin commented Jan 8, 2025

No description provided.

thomasBousselin commented

View reviewed changes

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

+              import java.net.URI
+              @Service
+              class EntityCompactionService(

Contributor Author

thomasBousselin Jan 8, 2025 •

edited

Loading

I choose "compaction" name for the layer and for the class name, because i can't just take the logic from the context sources i also have to do the task associated with the compaction.

I am open to other name suggestion.

Member

bobeal Jan 9, 2025

Compaction, definitely not. Compaction is JSON-LD compaction, nothing else.

That being said, I think you have extracted too much and the responsibilities become unclear (and too wide).

Instead, I would say that the web handler:

call a service that will match the CSR, call ContextSourceCaller to get the remote entities and return the result with the warning, the entities, ...
call entityQueryService to get the local entities
call ContextSourceUtils to merge the entities

...ice/src/test/kotlin/com/egm/stellio/search/entity/compaction/EntityCompactionServiceTests.kt

Comment on lines +17 to +21

+              // // DO NOT REVIEW WIP
+              // // DO NOT REVIEW WIP
+              // // DO NOT REVIEW WIP
+              // // DO NOT REVIEW WIP
+              // // DO NOT REVIEW WIP

Contributor Author

thomasBousselin Jan 8, 2025

Comming soon 😃 ...

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

                               }
                               """.trimIndent()
                           )
                   }
-                  @Test
-                  fun `get entity by id should correctly filter the asked attributes`() = runTest {

Contributor Author

thomasBousselin Jan 8, 2025 •

edited

Loading

This test will be migrated in entityCompactionServiceTest.

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

@@ @@ -480,342 +422,47 @@ class EntityHandlerTests { @@
                           )
                   }
-                  @Test
-                  fun `get entity by id should return 404 if the entity has none of the requested attributes`() {

Contributor Author

thomasBousselin Jan 8, 2025 •

edited

Loading

This test will be migrated in entityCompactionServiceTest.

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

                           .jsonPath("$.createdAt").doesNotExist()
                           .jsonPath("$.modifiedAt").doesNotExist()
                   }
-                  @Test
-                  fun `get entity by id should correctly serialize properties of type DateTime and display sysAttrs asked`() {

Contributor Author

thomasBousselin Jan 8, 2025

I am still looking if this test make sense in entityCompactionServiceTest.

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

-                  }
-                  @Test
-                  fun `get entity by id should correctly serialize multi-attribute property having more than one instance`() {

Contributor Author

thomasBousselin Jan 8, 2025

I am still looking if this test make sense in entityCompactionServiceTest.

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

-                  }
-                  @Test
-                  fun `get entity by id should correctly serialize multi-attribute relationship having one instance`() {

Contributor Author

thomasBousselin Jan 8, 2025

I am still looking if this test make sense in entityCompactionServiceTest.

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

-                  }
-                  @Test
-                  fun `get entity by id should correctly serialize multi-attribute relationship having more than one instance`() {

Contributor Author

thomasBousselin Jan 8, 2025

I am still looking if this test make sense in entityCompactionServiceTest.

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


		@Test
		fun `get entity by id should return the warnings sent by the CSRs and update the CSRs statuses`() {

Contributor Author

thomasBousselin Jan 8, 2025 •

edited

Loading

This test will be migrated in entityCompactionServiceTest.

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

@@ @@ -1258,42 +820,31 @@ class EntityHandlerTests { @@
                   }
                   @Test
-                  fun `get entities should return the warnings sent by the CSRs and update the CSRs statuses`() {

Contributor Author

thomasBousselin Jan 8, 2025

This test will be migrated in entityCompactionServiceTest.

thomasBousselin added 2 commits

January 8, 2025 16:32


          feat: first refacto no test

8b5fbf0


          feat: entity handler test

e6fdb2c

thomasBousselin force-pushed the refacto/context-source-layer branch from 8b6d74b to e6fdb2c Compare

January 8, 2025 15:36

Contributor

github-actions bot commented Jan 8, 2025 •

edited

Loading

Test Results

1 files - 69 1 suites - 69 0s ⏱️ - 1m 24s
1 tests - 1 137 1 ✅ - 1 137 0 💤 ±0 0 ❌ ±0
1 runs - 1 176 1 ✅ - 1 176 0 💤 ±0 0 ❌ ±0

Results for commit 92c2d73. ± Comparison against base commit e88ba43.

This pull request removes 1291 tests.

                           …, withTemporalValues=true, withAudit=false, expectation={
                      "@id": "https://uri…
                      "@type": "@json",
                      …
                    "@value": "/A/B"
                    "@value": "/C/D"
                    "@value": 20
                    "…
                    {
                  "@type": "https://uri.etsi.org/ngsi-ld/DateTime",
…

♻️ This comment has been updated with latest results.


          fix: detekt baseline

92c2d73

thomasBousselin closed this

github-actions bot locked and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet