fix: Response body for post #484

VivekHub97 · 2024-10-08T20:42:42Z

fix: Response body for posting in submodel repository and submodel service

Closes #398
Closes #457

mdanish98

Thanks a lot for the implementation.

Actually, this should be fixed in core [1] and [2] first and then at the Controller side, the return type here in [1] and [2] is void but it should be SubmodelElement, and then at the HttpController [3] and [4] respectively for Repo and Service, it should be like below:

@Override
public ResponseEntity<SubmodelElement> postSubmodelElementSubmodelRepo(Base64UrlEncodedIdentifier submodelIdentifier, @Valid SubmodelElement body) {
    SubmodelElement createdSME = repository.createSubmodelElement(submodelIdentifier.getIdentifier(), body);
    return new ResponseEntity<SubmodelElement>(createdSME, HttpStatus.CREATED);
}

Inside SubmodelRepoHttpController [3] the method postSubmodelElementByPathSubmodelRepo is not fixed.

Following is the summary:

Update interfaces of SubmodelRepository [1] and SubmodelService [2] for createSubmodelElement methods (there are multiple overloaded methods) such that the return type is SME instead of void.
Then handle this appropriately wherever these interfaces are implemented possibly in all implementations (Crud*Repo) and all features.
Update HTTP controllers for SMRepo [3] and SMService [4] like I defined above
Update the test cases to additionally check the creation response wherever required. (Maybe do this first to achieve Test Driven Development).

…rvice

mdanish98

There are no tests in the SubmodelServiceSuite. Please add them as well.

mdanish98 · 2024-11-04T08:11:30Z

...in/java/org/eclipse/digitaltwin/basyx/submodelrepository/backend/CrudSubmodelRepository.java

+	public SubmodelElement createSubmodelElement(String submodelId, SubmodelElement smElement) {
 		SubmodelService submodelService = getSubmodelServiceOrThrow(submodelId);

 		submodelService.createSubmodelElement(smElement);

 		updateSubmodel(submodelId, submodelService.getSubmodel());
+		return smElement;
 	}


Suggested change

public SubmodelElement createSubmodelElement(String submodelId, SubmodelElement smElement) {

SubmodelService submodelService = getSubmodelServiceOrThrow(submodelId);

submodelService.createSubmodelElement(smElement);

updateSubmodel(submodelId, submodelService.getSubmodel());

return smElement;

}

public SubmodelElement createSubmodelElement(String submodelId, SubmodelElement smElement) {

SubmodelService submodelService = getSubmodelServiceOrThrow(submodelId);

SubmodelElement createdSME = submodelService.createSubmodelElement(smElement);

updateSubmodel(submodelId, submodelService.getSubmodel());

return createdSME;

}

mdanish98 · 2024-11-04T08:12:16Z

...in/java/org/eclipse/digitaltwin/basyx/submodelrepository/backend/CrudSubmodelRepository.java

+	public SubmodelElement createSubmodelElement(String submodelId, String idShortPath, SubmodelElement smElement) throws ElementDoesNotExistException {
 		SubmodelService submodelService = getSubmodelServiceOrThrow(submodelId);

 		submodelService.createSubmodelElement(idShortPath, smElement);

 		updateSubmodel(submodelId, submodelService.getSubmodel());
+		return smElement;
 	}


Suggested change

public SubmodelElement createSubmodelElement(String submodelId, String idShortPath, SubmodelElement smElement) throws ElementDoesNotExistException {

SubmodelService submodelService = getSubmodelServiceOrThrow(submodelId);

submodelService.createSubmodelElement(idShortPath, smElement);

updateSubmodel(submodelId, submodelService.getSubmodel());

return smElement;

}

public SubmodelElement createSubmodelElement(String submodelId, String idShortPath, SubmodelElement smElement) throws ElementDoesNotExistException {

SubmodelService submodelService = getSubmodelServiceOrThrow(submodelId);

SubmodelElement createdSME = submodelService.createSubmodelElement(idShortPath, smElement);

updateSubmodel(submodelId, submodelService.getSubmodel());

return createdSME;

}

mdanish98 · 2024-11-04T08:16:08Z

...va/org/eclipse/digitaltwin/basyx/submodelrepository/feature/mqtt/MqttSubmodelRepository.java

@@ -99,17 +99,19 @@ public void setSubmodelElementValue(String submodelId, String idShortPath, Submo
 	}

 	@Override
-	public void createSubmodelElement(String submodelId, SubmodelElement smElement) {
+	public SubmodelElement createSubmodelElement(String submodelId, SubmodelElement smElement) {
 		decorated.createSubmodelElement(submodelId, smElement);
 		SubmodelElement submodelElement = decorated.getSubmodelElement(submodelId, smElement.getIdShort());


Ideally, we can get rid of this getSME operation and use the createSME return.

Suggested change

SubmodelElement submodelElement = decorated.getSubmodelElement(submodelId, smElement.getIdShort());

SubmodelElement submodelElement = decorated.createSubmodelElement(submodelId, smElement);

mdanish98 · 2024-11-04T08:16:19Z

...va/org/eclipse/digitaltwin/basyx/submodelrepository/feature/mqtt/MqttSubmodelRepository.java

 	}

 	@Override
-	public void createSubmodelElement(String submodelId, String idShortPath, SubmodelElement smElement) throws ElementDoesNotExistException {
+	public SubmodelElement createSubmodelElement(String submodelId, String idShortPath, SubmodelElement smElement) throws ElementDoesNotExistException {
 		decorated.createSubmodelElement(submodelId, idShortPath, smElement);
 		SubmodelElement submodelElement = decorated.getSubmodelElement(submodelId, idShortPath);


Same as above

mdanish98 · 2024-11-04T08:18:14Z

.../org/eclipse/digitaltwin/basyx/submodelrepository/feature/mqtt/TestMqttSubmodelObserver.java


+		assertEquals(responseSubmodelElement, submodelElement);


Expected should be first parameter.

Suggested change

assertEquals(responseSubmodelElement, submodelElement);

assertEquals(submodelElement, responseSubmodelElement);

mdanish98 · 2024-11-04T08:25:58Z

...godb/src/main/java/org/eclipse/digitaltwin/basyx/submodelservice/MongoDBSubmodelService.java

 		InMemorySubmodelService inMemorySubmodelService = getInMemorySubmodelService();
 		inMemorySubmodelService.createSubmodelElement(submodelElement);
 		Submodel submodel = inMemorySubmodelService.getSubmodel();
 		crudRepository.save(submodel);
+		return submodelElement;


Use the returned SME from line 108 and not return the SME received as a parameter.

mdanish98 · 2024-11-04T08:26:08Z

...godb/src/main/java/org/eclipse/digitaltwin/basyx/submodelservice/MongoDBSubmodelService.java

 		InMemorySubmodelService inMemorySubmodelService = getInMemorySubmodelService();
 		inMemorySubmodelService.createSubmodelElement(idShortPath, submodelElement);
 		Submodel submodel = inMemorySubmodelService.getSubmodel();
 		crudRepository.save(submodel);
+		return submodelElement;


Same as above.

mdanish98 · 2024-11-04T08:28:21Z

...e-core/src/test/java/org/eclipse/digitaltwin/basyx/submodelservice/SubmodelServiceSuite.java

@@ -718,6 +718,10 @@ private DefaultEntity createDummyEntityWithStatement(SubmodelElement submodelEle
 	private DefaultProperty createDummyProperty(String idShort) {
 		return new DefaultProperty.Builder().idShort(idShort).category("cat1").value("123").valueType(DataTypeDefXsd.INTEGER).build();
 	}
+
+	private SubmodelElement createDummySME(String idShort) {


Could you please explain the need of this? Because I can't find any reference to this private method in this class.

mdanish98 · 2024-11-04T09:23:17Z

...ain/java/org/eclipse/digitaltwin/basyx/submodelservice/feature/mqtt/MqttSubmodelService.java

@@ -93,19 +93,21 @@ public void setSubmodelElementValue(String idShortPath, SubmodelElementValue val
 	}

 	@Override
-	public void createSubmodelElement(SubmodelElement submodelElement) {
+	public SubmodelElement createSubmodelElement(SubmodelElement submodelElement) {
 		decorated.createSubmodelElement(submodelElement);
 		SubmodelElement smElement = decorated.getSubmodelElement(submodelElement.getIdShort());


We can get rid of this method and use the returned SME from line 97

mdanish98 · 2024-11-04T09:23:33Z

...ain/java/org/eclipse/digitaltwin/basyx/submodelservice/feature/mqtt/MqttSubmodelService.java

 	}

 	@Override
-	public void createSubmodelElement(String idShortPath, SubmodelElement submodelElement) throws ElementDoesNotExistException {
+	public SubmodelElement createSubmodelElement(String idShortPath, SubmodelElement submodelElement) throws ElementDoesNotExistException {

 		decorated.createSubmodelElement(idShortPath, submodelElement);

 		SubmodelElement smElement = decorated.getSubmodelElement(submodelElement.getIdShort());


Same comment as above.

mdanish98 suggested changes Oct 9, 2024

View reviewed changes

VivekHub97 added 2 commits October 29, 2024 14:12

fix: Response body for posting in submodel repository and submodel se…

7a0b4d2

…rvice

Changes return type for createSubmodelElement

52e3ab5

VivekHub97 force-pushed the bugFix_body branch from e7d1b97 to 52e3ab5 Compare October 29, 2024 16:27

VivekHub97 added 3 commits October 29, 2024 18:26

testcases for response of createSubmodelElement in submodelrepository

28160ca

testcases for response of createSubmodelElement in submodelservice

33cf443

some fixes and testcase updates

b160d85

mdanish98 suggested changes Nov 4, 2024

View reviewed changes

VivekHub97 added 3 commits November 15, 2024 09:37

Merge branch 'eclipse-basyx:main' into bugFix_body

947ed6c

resolved review remarks

3ba2774

fix issues

b08c21b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Response body for post #484

fix: Response body for post #484

VivekHub97 commented Oct 8, 2024 •

edited by aaronzi

Loading

mdanish98 left a comment •

edited by aaronzi

Loading

mdanish98 left a comment

mdanish98 Nov 4, 2024

mdanish98 Nov 4, 2024

mdanish98 Nov 4, 2024

mdanish98 Nov 4, 2024

mdanish98 Nov 4, 2024

mdanish98 Nov 4, 2024

mdanish98 Nov 4, 2024

mdanish98 Nov 4, 2024

mdanish98 Nov 4, 2024

mdanish98 Nov 4, 2024

	SubmodelElement submodelElement = decorated.getSubmodelElement(submodelId, smElement.getIdShort());
	SubmodelElement submodelElement = decorated.createSubmodelElement(submodelId, smElement);

	assertEquals(responseSubmodelElement, submodelElement);
	assertEquals(submodelElement, responseSubmodelElement);

fix: Response body for post #484

Are you sure you want to change the base?

fix: Response body for post #484

Conversation

VivekHub97 commented Oct 8, 2024 • edited by aaronzi Loading

mdanish98 left a comment • edited by aaronzi Loading

Choose a reason for hiding this comment

mdanish98 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

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

VivekHub97 commented Oct 8, 2024 •

edited by aaronzi

Loading

mdanish98 left a comment •

edited by aaronzi

Loading