Skip to content
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

fix(Attachment): Make get attachment endpoints consistent, allow to update project/component/release with attachment data #2917

Merged
merged 1 commit into from
Feb 14, 2025
Merged

fix(Attachment): Make get attachment endpoints consistent, allow to update project/component/release with attachment data #2917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions backend/attachments/src/main/java/org/eclipse/sw360/attachments/AttachmentHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,4 +252,11 @@ public List<AttachmentUsage> getAttachmentUsagesByReleaseId(String releaseId) th
public RequestStatus deleteOldAttachmentFromFileSystem() throws TException {
return handler.deleteOldAttachmentFromFileSystem();
}

@Override
public AttachmentContent getAttachmentContentById(String attachmentContentId) throws TException {
assertNotEmpty(attachmentContentId);
assertNotNull(attachmentContentId);
return handler.getAttachmentContentById(attachmentContentId);
}
}
4 changes: 4 additions & 0 deletions backend/common/src/main/java/org/eclipse/sw360/datahandler/db/AttachmentDatabaseHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,4 +276,8 @@ public List<AttachmentUsage> getAttachmentUsagesByReleaseId(String releaseId) {
public RequestStatus deleteOldAttachmentFromFileSystem() throws TException {
return DatabaseHandlerUtil.deleteOldAttachmentFromFileSystem();
}

public AttachmentContent getAttachmentContentById(String attachmentContentId) {
return attachmentContentRepository.get(attachmentContentId);
}
}
30 changes: 4 additions & 26 deletions libraries/datahandler/src/main/thrift/attachments.thrift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,35 +71,11 @@ struct Attachment {
20: optional set<string> uploadHistory, // just for importing data by now
21: optional CheckStatus checkStatus; // simple status of checks
22: optional string superAttachmentId,
23: optional string superAttachmentFilename
}

struct AttachmentDTO {
// WILL NOT BE SAVED IN DB, only for api
// General information
1: required string attachmentContentId,
5: required string filename,
6: optional string sha1,

10: optional AttachmentType attachmentType,

11: optional string createdBy,
12: optional string createdTeam,
13: optional string createdComment,
14: optional string createdOn,
15: optional string checkedBy,
16: optional string checkedTeam,
17: optional string checkedComment,
18: optional string checkedOn,

20: optional set<string> uploadHistory,
21: optional CheckStatus checkStatus;
22: optional string superAttachmentId,
23: optional string superAttachmentFilename,
24: optional UsageAttachment usageAttachment;
24: optional ProjectAttachmentUsage projectAttachmentUsage, // for view data only, will not be saved in DB
}

struct UsageAttachment {
struct ProjectAttachmentUsage {
// WILL NOT BE SAVED IN DB, only for api
// General information
1: optional string id,
Expand Down Expand Up @@ -361,4 +337,6 @@ service AttachmentService {
* calls deleteAttachmentAndDirectory for identifying the file and delete
*/
RequestStatus deleteOldAttachmentFromFileSystem();

AttachmentContent getAttachmentContentById(1: string attachmentContentId);
}
2 changes: 1 addition & 1 deletion libraries/datahandler/src/main/thrift/components.thrift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ struct ComponentDTO {
6: optional string description,

// Additional informations
10: optional set<AttachmentDTO> attachmentDTOs,
10: optional set<Attachment> attachments,
11: optional string createdOn, // Creation date YYYY-MM-dd
12: optional ComponentType componentType,

Expand Down
12 changes: 6 additions & 6 deletions .../src/main/java/org/eclipse/sw360/rest/resourceserver/attachment/AttachmentController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.apache.thrift.TException;
import org.eclipse.sw360.datahandler.thrift.Source;
import org.eclipse.sw360.datahandler.thrift.attachments.Attachment;
import org.eclipse.sw360.datahandler.thrift.attachments.AttachmentDTO;
import org.eclipse.sw360.datahandler.thrift.components.Component;
import org.eclipse.sw360.datahandler.thrift.components.Release;
import org.eclipse.sw360.datahandler.thrift.projects.Project;
Expand Down Expand Up @@ -127,7 +126,7 @@ public ResponseEntity<CollectionModel<EntityModel<Attachment>>> getAttachments(
tags = {"Attachments"}
)
@RequestMapping(value = ATTACHMENTS_URL , method = RequestMethod.POST, consumes = {MediaType.MULTIPART_MIXED_VALUE, MediaType.MULTIPART_FORM_DATA_VALUE})
public ResponseEntity<List<AttachmentDTO>> createAttachment(
public ResponseEntity<CollectionModel<EntityModel<Attachment>>> createAttachment(
@Parameter(description = "List of files to attach",
schema = @Schema(
type = "string",
Expand All @@ -141,12 +140,13 @@ public ResponseEntity<List<AttachmentDTO>> createAttachment(
throw new RuntimeException("You must select at least one file for uploading");
}
final User sw360User = restControllerHelper.getSw360UserFromAuthentication();
List<AttachmentDTO> attachmentDTOs = new ArrayList<>();
List<EntityModel<Attachment>> attachments = new ArrayList<>();
for (MultipartFile file: files) {
AttachmentDTO attachmentDTO = restControllerHelper.convertAttachmentToAttachmentDTO(attachmentService.addAttachment(file, sw360User),null);
attachmentDTOs.add(attachmentDTO);
Attachment attachment = attachmentService.addAttachment(file, sw360User);
attachments.add(EntityModel.of(attachment));
}
return new ResponseEntity<>(attachmentDTOs, HttpStatus.OK);
CollectionModel<EntityModel<Attachment>> attachmentsResponse = CollectionModel.of(attachments);
return new ResponseEntity<>(attachmentsResponse, HttpStatus.OK);
}

private HalResource<Attachment> createHalAttachment(AttachmentInfo attachmentInfo, User sw360User) throws TException {
Expand Down
140 changes: 69 additions & 71 deletions ...rc/main/java/org/eclipse/sw360/rest/resourceserver/attachment/Sw360AttachmentService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

package org.eclipse.sw360.rest.resourceserver.attachment;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import lombok.NonNull;
Expand All @@ -31,6 +32,7 @@
import org.eclipse.sw360.datahandler.common.Duration;
import org.eclipse.sw360.datahandler.common.SW360Utils;
import org.eclipse.sw360.datahandler.couchdb.AttachmentConnector;
import org.eclipse.sw360.datahandler.thrift.SW360Exception;
import org.eclipse.sw360.datahandler.thrift.Source;
import org.eclipse.sw360.datahandler.thrift.ThriftUtils;
import org.eclipse.sw360.datahandler.thrift.attachments.*;
Expand Down Expand Up @@ -60,6 +62,8 @@

import static org.eclipse.sw360.datahandler.common.CommonUtils.isNullEmptyOrWhitespace;
import static org.eclipse.sw360.datahandler.common.CommonUtils.nullToEmptyList;
import static org.eclipse.sw360.datahandler.common.SW360Assert.assertNotNull;
import static org.eclipse.sw360.datahandler.common.WrappedException.wrapSW360Exception;

@Service
@RequiredArgsConstructor(onConstructor = @__(@Autowired))
Expand Down Expand Up @@ -98,11 +102,7 @@ public AttachmentInfo getAttachmentById(String id) throws TException {

public List<AttachmentUsage> getAttachmentUseById(String id) throws TException {
AttachmentService.Iface attachmentClient = getThriftAttachmentClient();
List<AttachmentUsage> attachments = attachmentClient.getUsedAttachmentsById(id);
if (attachments.isEmpty()) {
throw new ResourceNotFoundException("Attachment not found.");
}
return attachments;
return attachmentClient.getUsedAttachmentsById(id);
}

public List<AttachmentInfo> getAttachmentsBySha1(String sha1) throws TException {
Expand Down Expand Up @@ -297,10 +297,12 @@ public AttachmentContent getAttachmentContent(String id) {
}
}

public CollectionModel<EntityModel<Attachment>> getAttachmentResourcesFromList(Set<Attachment> attachmentList) {
public CollectionModel<EntityModel<Attachment>> getAttachmentResourcesFromList(User user, Set<Attachment> attachments, Source owner) throws TTransportException {
Set<Attachment> attachmentsWithUsages = getAttachmentsWithUsages(user, attachments, owner);

final List<EntityModel<Attachment>> attachmentResources = new ArrayList<>();
if (CommonUtils.isNotEmpty(attachmentList)) {
for (final Attachment attachment : attachmentList) {
if (CommonUtils.isNotEmpty(attachmentsWithUsages)) {
for (final Attachment attachment : attachmentsWithUsages) {
final EntityModel<Attachment> attachmentResource = EntityModel.of(attachment);
attachmentResources.add(attachmentResource);
}
Expand All @@ -320,19 +322,6 @@ public CollectionModel<EntityModel<Attachment>> getResourcesFromList(Set<Attachm
return CollectionModel.of(attachmentResources);
}

public CollectionModel<EntityModel<AttachmentDTO>> getAttachmentDTOResourcesFromList(User user, Set<Attachment> attachments, Source owner) throws TTransportException {
Map<Attachment,UsageAttachment> attachmentUsageAttachmentMap = getAttachmentUsages(user, attachments, owner);
Set<AttachmentDTO> attachmentDTOs = getAttachmentDTOs(attachments, attachmentUsageAttachmentMap);
final List<EntityModel<AttachmentDTO>> attachmentResources = new ArrayList<>();
if (CommonUtils.isNotEmpty(attachmentDTOs)) {
for (final AttachmentDTO attachment : attachmentDTOs) {
final EntityModel<AttachmentDTO> attachmentResource = EntityModel.of(attachment);
attachmentResources.add(attachmentResource);
}
}
return CollectionModel.of(attachmentResources);
}

public List<AttachmentUsage> getAllAttachmentUsage(String projectId) throws TException {
AttachmentService.Iface attachmentClient = getThriftAttachmentClient();
return attachmentClient.getUsedAttachments(Source.projectId(projectId), null);
Expand Down Expand Up @@ -428,13 +417,27 @@ private File renameFile(File sourceFile, String filename) throws IOException {
return file;
}

public Map<Attachment, UsageAttachment> getAttachmentUsages(User user, Set<Attachment> attachments, Source owner) throws TTransportException {
private Set<Attachment> getAttachmentsWithUsages(User user, Set<Attachment> attachments, Source owner) throws TTransportException {
Set<Attachment> atts = CommonUtils.nullToEmptySet(attachments);
Set<String> attachmentContentIds = atts.stream().map(Attachment::getAttachmentContentId).collect(Collectors.toSet());
Map<String, Long> restrictedProjectsCountsByContentId = getRestrictedProjectsCountsByContentId(attachmentContentIds, user, owner);
Map<Attachment, UsageAttachment> attachmentUsageMap = getAttachmentUsageMap(restrictedProjectsCountsByContentId, user);

return attachmentUsageMap;
for (Attachment attachment : atts) {
try {
List<AttachmentUsage> attachmentUsages = getAttachmentUseById(attachment.getAttachmentContentId());
long numberProjectByAttachmentUsages = distinctProjectIdsFromAttachmentUsages(attachmentUsages).count();
Set<ProjectUsage> projectUsages = getProjectAttachmentUsages(attachmentUsages, user);

ProjectAttachmentUsage usage = new ProjectAttachmentUsage();
usage.setVisible(numberProjectByAttachmentUsages);
usage.setRestricted(restrictedProjectsCountsByContentId.getOrDefault(attachment.getAttachmentContentId(), 0L));
usage.setProjectUsages(projectUsages);
attachment.setProjectAttachmentUsage(usage);
} catch (TException e) {
log.error(e.getMessage());
}
}
return atts;
}

private Map<String, Long> getRestrictedProjectsCountsByContentId(Set<String> attachmentContentIds, User user, Source owner) throws TTransportException {
Expand All @@ -454,30 +457,6 @@ private Map<String, Long> getRestrictedProjectsCountsByContentId(Set<String> att
return restrictedProjectsCountsByContentId;
}

private Map<Attachment, UsageAttachment> getAttachmentUsageMap(Map<String, Long> restrictedProjectsCountsByContentId, User user) {
Map<Attachment, UsageAttachment> attachmentUsageMap = new HashMap<>();

restrictedProjectsCountsByContentId.entrySet().stream().forEach(stringLongEntry -> {
try {
List<AttachmentUsage> attachmentUsages = getAttachmentUseById(stringLongEntry.getKey());

Attachment attachment = getAttachmentForId(stringLongEntry.getKey());
Set<ProjectUsage> projectUsages = getProjectAttachmentUsages(attachmentUsages, user);
long numberProjectByAttachmentUsages = distinctProjectIdsFromAttachmentUsages(attachmentUsages).count();

UsageAttachment usage = new UsageAttachment();
usage.setVisible(numberProjectByAttachmentUsages);
usage.setRestricted(stringLongEntry.getValue());
usage.setProjectUsages(projectUsages);

attachmentUsageMap.put(attachment,usage);
} catch (TException e) {
log.error("Cannot load map attachment usages", e);
}
});
return attachmentUsageMap;
}

private Set<ProjectUsage> getProjectAttachmentUsages(List<AttachmentUsage> attachmentUsages, User user) {
Set<ProjectUsage> projectUsages = new HashSet<>();
attachmentUsages.stream().forEach(attachmentUsage -> {
Expand Down Expand Up @@ -540,35 +519,54 @@ private Map<String, Long> countRestrictedProjectsByContentId(Map<String, List<At
));
}

public Attachment getAttachmentForId(String id) throws TException {
private AttachmentContent getAttachmentContentForId(String id) throws TException {
AttachmentService.Iface attachmentClient = getThriftAttachmentClient();
List<Attachment> attachments = attachmentClient.getAttachmentsByIds(Collections.singleton(id));
if (attachments.isEmpty()) {
throw new ResourceNotFoundException("Attachment not found.");
}
return attachments.get(0);
return attachmentClient.getAttachmentContentById(id);
}

public Set<AttachmentDTO> getAttachmentDTOs(Set<Attachment> attachments, Map<Attachment,UsageAttachment> attachmentUsages ) {
Set<AttachmentDTO> attachmentDTOS = new HashSet<>();
attachmentUsages.entrySet().stream().forEach(attachmentUsageEntry -> {
attachments.remove(attachmentUsageEntry.getKey());
AttachmentDTO attachmentDTO = restControllerHelper.convertAttachmentToAttachmentDTO(attachmentUsageEntry.getKey(),attachmentUsageEntry.getValue());
attachmentDTOS.add(attachmentDTO);
});
attachments.forEach(attachment -> {
AttachmentDTO attachmentDTO = restControllerHelper.convertAttachmentToAttachmentDTO(attachment,new UsageAttachment());
attachmentDTOS.add(attachmentDTO);
});
return attachmentDTOS;
}

public boolean isAttachmentExist(String id) {
public boolean isAttachmentContentExist(String id) {
try {
Attachment attachment = getAttachmentForId(id);
return attachment != null;
AttachmentContent attachmentContent = getAttachmentContentForId(id);
return attachmentContent != null;
} catch (ResourceNotFoundException | TException notFoundException) {
log.error(notFoundException.getMessage());
return false;
}
}

public Set<Attachment> getAttachmentsFromRequest(Object attachmentData, ObjectMapper mapper) {
User sw360User = restControllerHelper.getSw360UserFromAuthentication();
if (null == attachmentData) {
return null;
}
Set<Attachment> attachments = mapper.convertValue(attachmentData,
mapper.getTypeFactory().constructCollectionType(LinkedHashSet.class, Attachment.class));
return attachments.stream()
.map(attachment -> wrapSW360Exception(() -> {
boolean isAttachmentExist = isAttachmentContentExist(attachment.getAttachmentContentId());
if (!isAttachmentExist) {
throw new ResourceNotFoundException("Attachment " + attachment.getAttachmentContentId() + " not found.");
}
fillCheckedAttachmentData(attachment, sw360User);
return attachment;
}))
.collect(Collectors.toSet());
}

public void fillCheckedAttachmentData(Attachment attachment, User user) throws SW360Exception {
assertNotNull(attachment);
if (attachment.getCheckStatus() == null) {
attachment.setCheckStatus(CheckStatus.NOTCHECKED);
}
if (!CheckStatus.NOTCHECKED.equals(attachment.getCheckStatus())) {
attachment.setCheckedBy(attachment.getCheckedBy() != null ? attachment.getCheckedBy() : user.getEmail());
attachment.setCheckedTeam(attachment.getCheckedTeam() != null ? attachment.getCheckedTeam() : user.getDepartment());
attachment.setCheckedOn(attachment.getCheckedOn() != null ? attachment.getCheckedOn() : SW360Utils.getCreatedOn());
} else {
attachment.unsetCheckedBy();
attachment.unsetCheckedTeam();
attachment.setCheckedComment("");
attachment.unsetCheckedOn();
}
}
}
17 changes: 6 additions & 11 deletions ...er/src/main/java/org/eclipse/sw360/rest/resourceserver/component/ComponentController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.eclipse.sw360.datahandler.thrift.Source;
import org.eclipse.sw360.datahandler.thrift.VerificationStateInfo;
import org.eclipse.sw360.datahandler.thrift.attachments.Attachment;
import org.eclipse.sw360.datahandler.thrift.attachments.AttachmentDTO;
import org.eclipse.sw360.datahandler.thrift.components.Component;
import org.eclipse.sw360.datahandler.thrift.components.ComponentDTO;
import org.eclipse.sw360.datahandler.thrift.components.ComponentType;
Expand Down Expand Up @@ -95,6 +94,7 @@
import java.util.function.Predicate;
import java.util.stream.Collectors;

import static org.eclipse.sw360.datahandler.common.WrappedException.wrapSW360Exception;
import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.linkTo;

@BasePathAwareController
Expand Down Expand Up @@ -387,15 +387,10 @@ public ResponseEntity<EntityModel<Component>> patchComponent(
) throws TException {
User user = restControllerHelper.getSw360UserFromAuthentication();
Component sw360Component = componentService.getComponentForUserById(id, user);
sw360Component = this.restControllerHelper.updateComponent(sw360Component, updateComponentDto);
Set<AttachmentDTO> attachmentDTOS = updateComponentDto.getAttachmentDTOs();
if (!CommonUtils.isNullOrEmptyCollection(attachmentDTOS)) {
Set<Attachment> attachments = new HashSet<>();
for (AttachmentDTO attachmentDTO: attachmentDTOS) {
attachments.add(restControllerHelper.convertToAttachment(attachmentDTO, user));
}
sw360Component.setAttachments(attachments);
if (updateComponentDto.getAttachments() != null) {
updateComponentDto.getAttachments().forEach(attachment -> wrapSW360Exception(() -> this.attachmentService.fillCheckedAttachmentData(attachment, user)));
}
sw360Component = this.restControllerHelper.updateComponent(sw360Component, updateComponentDto);
RequestStatus updateComponentStatus = componentService.updateComponent(sw360Component, user);
HalResource<Component> userHalResource = createHalComponent(sw360Component, user);
if (updateComponentStatus == RequestStatus.SENT_TO_MODERATOR) {
Expand Down Expand Up @@ -482,13 +477,13 @@ public ResponseEntity<EntityModel<Component>> createComponent(
tags = {"Components"}
)
@RequestMapping(value = COMPONENTS_URL + "/{id}/attachments", method = RequestMethod.GET)
public ResponseEntity<CollectionModel<EntityModel<AttachmentDTO>>> getComponentAttachments(
public ResponseEntity<CollectionModel<EntityModel<Attachment>>> getComponentAttachments(
@Parameter(description = "The id of the component.")
@PathVariable("id") String id
) throws TException {
final User sw360User = restControllerHelper.getSw360UserFromAuthentication();
final Component sw360Component = componentService.getComponentForUserById(id, sw360User);
final CollectionModel<EntityModel<AttachmentDTO>> resources = attachmentService.getAttachmentDTOResourcesFromList(sw360User, sw360Component.getAttachments(), Source.releaseId(sw360Component.getId()));
final CollectionModel<EntityModel<Attachment>> resources = attachmentService.getAttachmentResourcesFromList(sw360User, sw360Component.getAttachments(), Source.componentId(id));
return new ResponseEntity<>(resources, HttpStatus.OK);
}

Expand Down
Loading