Skip to content

Commit 331dcb1

Browse files
authored
fix(file-sme): preserve file attachments when updating metadata (Issue 405) (#764)
* fix(file-sme): preserve file attachments when updating metadata Previously, updating a File submodel element via PUT would unconditionally delete the associated file, even when the file path (value) did not change. This commit introduces logic to detect whether the file path has actually changed before deleting the file. Changes: - Added path comparison logic using java.nio.file.Path to handle OS-specific path formats. - Only delete attachments if the path has changed. - Introduced helper methods for clarity and testability: isFileElement, extractFilePath, getExistingFilePath, pathsDiffer. - Added robust exception handling and conservative fallback (delete if uncertain). - Added structured SLF4J logging for traceability. This resolves unintended data loss during metadata-only updates to File submodel elements. * fix(file-sme): delete file attachment when replacing File with non-File element Ensures that when a File submodel element is replaced by a non-File element (e.g., Property), the previously attached file is properly deleted.
1 parent 4a7868a commit 331dcb1

1 file changed

Lines changed: 75 additions & 3 deletions

File tree

  • basyx.submodelservice/basyx.submodelservice-backend/src/main/java/org/eclipse/digitaltwin/basyx/submodelservice/backend

basyx.submodelservice/basyx.submodelservice-backend/src/main/java/org/eclipse/digitaltwin/basyx/submodelservice/backend/CrudSubmodelService.java

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,14 @@
3838
import org.eclipse.digitaltwin.basyx.operation.InvokableOperation;
3939
import org.eclipse.digitaltwin.basyx.submodelservice.SubmodelService;
4040
import org.eclipse.digitaltwin.basyx.submodelservice.value.SubmodelElementValue;
41+
import org.slf4j.Logger;
42+
import org.slf4j.LoggerFactory;
4143
import org.springframework.lang.NonNull;
4244

4345
import java.io.File;
4446
import java.io.InputStream;
47+
import java.nio.file.Path;
48+
import java.nio.file.Paths;
4549
import java.util.List;
4650

4751
/**
@@ -54,6 +58,7 @@ public class CrudSubmodelService implements SubmodelService {
5458
private final SubmodelBackend backend;
5559
private final String submodelId;
5660
private final SubmodelFileOperations submodelFileOperations;
61+
private Logger logger = LoggerFactory.getLogger(CrudSubmodelService.class);
5762

5863
public CrudSubmodelService(SubmodelBackend submodelRepositoryBackend, FileRepository fileRepository, @NonNull Submodel submodel) {
5964
this(submodelRepositoryBackend, fileRepository, submodel.getId());
@@ -102,10 +107,77 @@ public void createSubmodelElement(String idShortPath, SubmodelElement submodelEl
102107
}
103108

104109
@Override
105-
public void updateSubmodelElement(String idShortPath, SubmodelElement submodelElement) throws ElementDoesNotExistException {
106-
deleteAssociatedFileIfAny(idShortPath);
110+
public void updateSubmodelElement(String idShortPath, SubmodelElement newElement) throws ElementDoesNotExistException {
111+
SubmodelElement existingSubmodelElement = backend.getSubmodelElement(submodelId, idShortPath);
112+
boolean existingSubmodelElementIsFile = existingSubmodelElement instanceof org.eclipse.digitaltwin.aas4j.v3.model.File;
113+
boolean newSubmodelElementIsFile = newElement instanceof org.eclipse.digitaltwin.aas4j.v3.model.File;
114+
115+
if (existingSubmodelElementIsFile && newSubmodelElementIsFile) {
116+
handleFileAttachmentUpdate(submodelId, idShortPath, newElement);
117+
} else if (existingSubmodelElementIsFile) {
118+
deleteAssociatedFileIfAny(idShortPath);
119+
}
120+
121+
backend.updateSubmodelElement(submodelId, idShortPath, newElement);
122+
}
123+
124+
private void handleFileAttachmentUpdate(String submodelId, String idShortPath, SubmodelElement updatedElement) {
125+
try {
126+
if (hasFilePathChanged(submodelId, idShortPath, updatedElement)) {
127+
deleteAssociatedFileIfAny(idShortPath);
128+
}
129+
} catch (Exception e) {
130+
logger.warn("Could not verify file path change for '{}'. Assuming changed. Reason: {}", idShortPath, e.getMessage());
131+
deleteAssociatedFileIfAny(idShortPath);
132+
}
133+
}
134+
135+
private boolean hasFilePathChanged(String submodelId, String idShortPath, SubmodelElement updatedElement) {
136+
if (!(updatedElement instanceof org.eclipse.digitaltwin.aas4j.v3.model.File file)) {
137+
logger.warn("Expected File element but got: {}", updatedElement.getClass().getSimpleName());
138+
return true;
139+
}
140+
141+
String newPathString = extractFilePath(file);
142+
if (newPathString == null) {
143+
return true;
144+
}
145+
146+
String oldPathString = getExistingFilePath(submodelId, idShortPath);
147+
if (oldPathString == null) {
148+
return true;
149+
}
107150

108-
backend.updateSubmodelElement(submodelId, idShortPath, submodelElement);
151+
return pathsDiffer(oldPathString, newPathString);
152+
}
153+
154+
private boolean isFileElement(SubmodelElement element) {
155+
return element instanceof org.eclipse.digitaltwin.aas4j.v3.model.File;
156+
}
157+
158+
// Extracts the new file path (value) from the updated File element
159+
private String extractFilePath(org.eclipse.digitaltwin.aas4j.v3.model.File fileElement) {
160+
return fileElement.getValue();
161+
}
162+
163+
private String getExistingFilePath(String submodelId, String idShortPath) {
164+
try {
165+
return submodelFileOperations.getFile(submodelId, idShortPath).getPath();
166+
} catch (Exception e) {
167+
logger.warn("Failed to retrieve existing file path for '{}': {}", idShortPath, e.getMessage(), e);
168+
return null;
169+
}
170+
}
171+
172+
private boolean pathsDiffer(String path1, String path2) {
173+
try {
174+
Path normalized1 = Paths.get(path1).normalize().toAbsolutePath();
175+
Path normalized2 = Paths.get(path2).normalize().toAbsolutePath();
176+
return !normalized1.equals(normalized2);
177+
} catch (Exception e) {
178+
logger.error("Error comparing file paths: {}", e.getMessage(), e);
179+
return true; // Consider paths different in case of error
180+
}
109181
}
110182

111183
@Override

0 commit comments

Comments
 (0)