fix: add a check for / suffix for basyx external url in registry integration #333

ShehriyarShariq-Fraunhofer · 2024-07-09T07:10:17Z

Added a trailing slash in case one was missing for the basyx external url in registry integration.

Closes #414

…egration

ShehriyarShariq-Fraunhofer · 2024-07-09T07:10:55Z

@mateusmolina-iese

….com/ShehriyarShariq-Fraunhofer/basyx-java-server-sdk into fix-slash-check-registry-integration

…n tests

….com/ShehriyarShariq-Fraunhofer/basyx-java-server-sdk into fix-slash-check-registry-integration

mateusmolina-iese · 2024-07-25T09:26:00Z

...e/digitaltwin/basyx/aasrepository/feature/registry/integration/TestAasDescriptorFactory.java

+    public void testUrlWithTrailingSlashAndPathWithLeadingSlash() {
+        String baseURLWithSlash = AAS_REPO_URL + "/";
+
+        assertEquals(baseURLWithSlash + AAS_REPOSITORY_PATH_WITHOUT_SLASH, AasDescriptorFactory.createAasRepositoryUrl(baseURLWithSlash));


The assertion is wrongly defined

mateusmolina-iese · 2024-07-25T09:26:28Z

...e/digitaltwin/basyx/aasrepository/feature/registry/integration/TestAasDescriptorFactory.java

+    public void testUrlWithoutTrailingSlashAndPathWithoutLeadingSlash() {
+        String baseURLWithoutSlash = AAS_REPO_URL;
+
+        assertEquals(baseURLWithoutSlash + AAS_REPOSITORY_PATH_WITH_SLASH , AasDescriptorFactory.createAasRepositoryUrl(baseURLWithoutSlash));


The assertion is wrongly defined

mateusmolina-iese · 2024-07-25T09:27:51Z

...e/digitaltwin/basyx/aasrepository/feature/registry/integration/TestAasDescriptorFactory.java

+	private static final String AAS_REPO_URL = "http://localhost:8081";
+
+	private static final String AAS_REPOSITORY_PATH_WITH_SLASH = "/shells";
+	private static final String AAS_REPOSITORY_PATH_WITHOUT_SLASH = "shells";


Either stick to building the withSlash/withoutSlash for both path and repo_url in the test or define them consistenly as final vars; but don't mix both approaches.

mateusmolina-iese · 2024-07-25T09:29:57Z

...win/basyx/submodelrepository/feature/registry/integration/TestSubmodelDescriptorFactory.java

+    public void testUrlWithTrailingSlashAndPathWithLeadingSlash() {
+        String baseURLWithSlash = SUBMODEL_REPO_URL + "/";
+
+        assertEquals(baseURLWithSlash + SUBMODEL_REPOSITORY_PATH_WITHOUT_SLASH, SubmodelDescriptorFactory.createSubmodelRepositoryUrl(baseURLWithSlash));


The assertion is wrongly defined

mateusmolina-iese · 2024-07-25T09:30:17Z

...win/basyx/submodelrepository/feature/registry/integration/TestSubmodelDescriptorFactory.java

+	private static final String SUBMODEL_REPO_URL = "http://localhost:8081";
+
+
+	private static final String SUBMODEL_REPOSITORY_PATH_WITH_SLASH = "/submodels";


Same issue as in the TestAasDescriptorFactory

mateusmolina-iese · 2024-07-25T09:30:29Z

...win/basyx/submodelrepository/feature/registry/integration/TestSubmodelDescriptorFactory.java

+    public void testUrlWithoutTrailingSlashAndPathWithoutLeadingSlash() {
+        String baseURLWithoutSlash = SUBMODEL_REPO_URL;
+
+        assertEquals(baseURLWithoutSlash + SUBMODEL_REPOSITORY_PATH_WITH_SLASH , SubmodelDescriptorFactory.createSubmodelRepositoryUrl(baseURLWithoutSlash));


The assertion is wrongly defined

Latest update from main

mateusmolina-iese · 2024-07-31T07:12:28Z

basyx.common/basyx.core/src/main/java/org/eclipse/digitaltwin/basyx/core/Helper.java

+
+	public static String createRepositoryUrl(String baseUrl, String additionalPath) {
+        try {
+            // Create a URI from the base URL


Please the remove the comments

mateusmolina-iese · 2024-07-31T07:13:04Z

basyx.common/basyx.core/src/main/java/org/eclipse/digitaltwin/basyx/core/Helper.java

+import org.springframework.web.util.UriComponentsBuilder;
+
+
+public class Helper {


change the name of the helper class. Please also change the test's name

mateusmolina-iese · 2024-07-31T07:13:29Z

basyx.common/basyx.core/src/test/java/org/eclipse/digitaltwin/basyx/core/TestHelper.java

+
+	@Test
+    public void testUrlWithTrailingSlashAndPathNameWithNoLeadingSlash() {
+		assertEquals(BASE_URL + "/" + PATH, Helper.createRepositoryUrl(BASE_URL + "/", PATH));


mateusmolina-iese · 2024-07-31T07:13:33Z

basyx.common/basyx.core/src/test/java/org/eclipse/digitaltwin/basyx/core/TestHelper.java

+
+    @Test
+    public void testUrlWithTrailingSlashAndPathNameWithLeadingSlash() {
+		assertEquals(BASE_URL + "/" + PATH, Helper.createRepositoryUrl(BASE_URL + "/", "/" + PATH));


mateusmolina-iese · 2024-07-31T07:14:58Z

basyx.common/basyx.core/src/test/java/org/eclipse/digitaltwin/basyx/core/TestHelper.java

+
+    @Test
+    public void testUrlWithoutTrailingSlashAndPathNameWithNoLeadingSlash() {
+        assertEquals(BASE_URL + "/" + PATH , Helper.createRepositoryUrl(BASE_URL, PATH));


make BASE_URL + "/" + PATH a private static final String EXPECTED_URL

mateusmolina-iese · 2024-07-31T07:17:05Z

basyx.common/basyx.core/src/test/java/org/eclipse/digitaltwin/basyx/core/TestHelper.java

+    public void testUrlWithoutTrailingSlashAndPathNameWithLeadingSlash() {
+        assertEquals(BASE_URL + "/" + PATH , Helper.createRepositoryUrl(BASE_URL, "/" + PATH));
+    }
+


Add tests for urls with a contextPath already set

….com/ShehriyarShariq-Fraunhofer/basyx-java-server-sdk into fix-slash-check-registry-integration

mateusmolina-iese

The main issue is that there is no explicit test for the externalUrl URL in the registry-integration features. I was expecting tests for externalUrl in the aasrepository-feature-registry-integration and submodelrepository-feature-registry-integration features. Please find below the entry point to where the externalUrl is mapped (similarly for the submodel feature):

basyx-java-server-sdk/basyx.aasrepository/basyx.aasrepository-feature-registry-integration/src/main/java/org/eclipse/digitaltwin/basyx/aasrepository/feature/registry/integration/RegistryIntegrationAasRepositoryFeature.java

Line 53 in 4fc702e

@Value("${basyx.externalurl:}")

What happens when the / suffix is added or not added to the externalUrl there?

mateusmolina-iese · 2024-09-24T06:30:16Z

....common/basyx.core/src/main/java/org/eclipse/digitaltwin/basyx/core/RepositoryUrlHelper.java

@@ -0,0 +1,36 @@
+package org.eclipse.digitaltwin.basyx.core;


Add license header

mateusmolina-iese · 2024-09-24T06:30:23Z

...mon/basyx.core/src/test/java/org/eclipse/digitaltwin/basyx/core/TestRepositoryUrlHelper.java

@@ -0,0 +1,59 @@
+package org.eclipse.digitaltwin.basyx.core;


Add license header

mateusmolina-iese · 2024-09-24T06:34:18Z

...lipse/digitaltwin/basyx/aasrepository/feature/authorization/TestAuthorizedAasRepository.java

@@ -529,10 +530,10 @@ public void getThumbnailWithCorrectRoleAndPermission() throws IOException {

 		String accessToken = getAccessToken(DummyCredentialStore.BASYX_READER_CREDENTIAL);

-		CloseableHttpResponse retrievalResponse = getElementWithAuthorization(BaSyxHttpTestUtils.getThumbnailAccessURL(createAasRepositoryUrl(aasRepositoryBaseUrl), SPECIFIC_SHELL_ID), accessToken);
+		CloseableHttpResponse retrievalResponse = getElementWithAuthorization(BaSyxHttpTestUtils.getThumbnailAccessURL(RepositoryUrlHelper.createRepositoryUrl(aasRepositoryBaseUrl, null), SPECIFIC_SHELL_ID), accessToken);


Are the calls to createRepositoryUrl(...) necessary? The second par is always null. Either remove them OR remove the shells context in aasRepositoryBaseUrl and use AAS_REPOSITORY_PATH and createRepositoryUrl(...) to generate the url.

….com/ShehriyarShariq-Fraunhofer/basyx-java-server-sdk into fix-slash-check-registry-integration

fix: �add a check for / suffix for basyx external url in registry int…

82a79d2

…egration

ShehriyarShariq-Fraunhofer and others added 14 commits July 9, 2024 09:25

fix: submodal integration test

4a7a8aa

Empty commit for the CI to run

62d44d1

fix: registry integration create url logic

13868a9

Merge branch 'fix-slash-check-registry-integration' of https://github…

94923af

….com/ShehriyarShariq-Fraunhofer/basyx-java-server-sdk into fix-slash-check-registry-integration

Empty commit for the CI to run

bb3e3e9

fix: latest merge

437ac16

Empty commit for the CI to run

9bdb684

fix: feature-registry-integration createURL remove static reference i…

c410c6a

…n tests

Merge branch 'fix-slash-check-registry-integration' of https://github…

245bae2

….com/ShehriyarShariq-Fraunhofer/basyx-java-server-sdk into fix-slash-check-registry-integration

Triggering the CI

dae87d7

fix: createUrl method issue with path without leading slash resolved

ba08cbd

Merge branch 'fix-slash-check-registry-integration' of https://github…

99a47de

….com/ShehriyarShariq-Fraunhofer/basyx-java-server-sdk into fix-slash-check-registry-integration

fix: use main static createUrl method in DescriptorFactory test cases

9d86f9e

refactor: remove create*Url method from Dummy*DescriptorFactory

c75b166

mateusmolina-iese reviewed Jul 25, 2024

View reviewed changes

ShehriyarShariq-Fraunhofer added 3 commits July 25, 2024 11:51

fix: Test*DescriptorFactory refactor tests

9272437

Merge branch 'main' into fix-slash-check-registry-integration

ad5b8a1

Latest update from main

refactor: make createRepositoryUrl common + refactor implementation

27e19d4

mateusmolina-iese reviewed Jul 31, 2024

View reviewed changes

ShehriyarShariq-Fraunhofer and others added 4 commits August 1, 2024 12:18

fix: RepositoryUrlHelper - add tests with context path in url

da4b87b

refactor: reformat RepositoryUrlHelper + Test

3306f65

fix: resolve merge conflicts with latest updates from main

2be64c0

Empty commit to trigger the CI

7b303c4

ShehriyarShariq-Fraunhofer and others added 5 commits September 3, 2024 10:34

fix: aas repository feature auth url function fix

37bc55e

Merge branch 'fix-slash-check-registry-integration' of https://github…

a29ab5a

….com/ShehriyarShariq-Fraunhofer/basyx-java-server-sdk into fix-slash-check-registry-integration

Triggering the CI

66b6354

fix: revert pom + update createRepositoryUrl functionality

581e641

Triggering the CI

dedc173

mateusmolina-iese reviewed Sep 24, 2024

View reviewed changes

fix: add license headers

3ccbec3

mateusmolina-iese assigned ShehriyarShariq-Fraunhofer Sep 24, 2024

ShehriyarShariq-Fraunhofer and others added 19 commits October 1, 2024 00:40

fix: update test for external url

9c118a4

stash

27bdb45

Merge branch 'main' into fix-slash-check-registry-integration

ec7e126

feat: AasRepositoryRegistryLinkDescriptorGenerationTest

c7f3d55

stash

5bd2f77

Merge branch 'main' into fix-slash-check-registry-integration

09f00d0

stash

493f177

stash

d0ba025

stash

2b5163f

fix: working registry url slash test

0de2d33

feat: working test

b1babf6

fix: merge from main

ec123aa

feat: add Submodel Repo Registry Test

ae3c928

fix: merge with main

d15ef4d

Merge branch 'fix-slash-check-registry-integration' of https://github…

f03d036

….com/ShehriyarShariq-Fraunhofer/basyx-java-server-sdk into fix-slash-check-registry-integration

fix: revert pom.xml

a9f4ad2

style: code clean-up

ed11d33

test: refactor *RepositoryRegistryLink tests to be parameterized

4913629

style: minor fixes

1119feb

ShehriyarShariq-Fraunhofer marked this pull request as ready for review November 5, 2024 06:41

mateusmolina-iese approved these changes Nov 5, 2024

View reviewed changes

aaronzi merged commit c9f8845 into eclipse-basyx:main Nov 5, 2024
27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add a check for / suffix for basyx external url in registry integration #333

fix: add a check for / suffix for basyx external url in registry integration #333

ShehriyarShariq-Fraunhofer commented Jul 9, 2024 •

edited by aaronzi

Loading

ShehriyarShariq-Fraunhofer commented Jul 9, 2024

mateusmolina-iese Jul 25, 2024

mateusmolina-iese Jul 25, 2024

mateusmolina-iese Jul 25, 2024

mateusmolina-iese Jul 25, 2024

mateusmolina-iese Jul 25, 2024

mateusmolina-iese Jul 25, 2024

mateusmolina-iese Jul 31, 2024

mateusmolina-iese Jul 31, 2024

mateusmolina-iese Jul 31, 2024

mateusmolina-iese Jul 31, 2024

mateusmolina-iese Jul 31, 2024

mateusmolina-iese Jul 31, 2024

mateusmolina-iese left a comment •

edited

Loading

mateusmolina-iese Sep 24, 2024

mateusmolina-iese Sep 24, 2024

mateusmolina-iese Sep 24, 2024

		private static final String SUBMODEL_REPO_URL = "http://localhost:8081";


		private static final String SUBMODEL_REPOSITORY_PATH_WITH_SLASH = "/submodels";

		import org.springframework.web.util.UriComponentsBuilder;


		public class Helper {

fix: add a check for / suffix for basyx external url in registry integration #333

fix: add a check for / suffix for basyx external url in registry integration #333

Conversation

ShehriyarShariq-Fraunhofer commented Jul 9, 2024 • edited by aaronzi Loading

ShehriyarShariq-Fraunhofer commented Jul 9, 2024

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

mateusmolina-iese left a comment • edited Loading

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

ShehriyarShariq-Fraunhofer commented Jul 9, 2024 •

edited by aaronzi

Loading

mateusmolina-iese left a comment •

edited

Loading