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.

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

Extending Concept Set Items with Annotations #2403

Merged
merged 19 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e6c3857
ATL-17: Updating branch with latest release
hernaldourbina Mar 26, 2024
633eb78
ATL-17: Add feature about the metadata in concept set
May 3, 2024
757929a
Fixing migration scripts and Concept Set Service to save and update m…
alex-odysseus Jun 4, 2024
83579dd
[ATL-17] Refactored the annotations feature. Renamed metadata to anno…
oleg-odysseus Jun 28, 2024
7e9359c
[ATL-17] Fixed issue with clashing endpoints for conceptset delete op…
oleg-odysseus Jul 16, 2024
25f44ae
Correcting newly adjusted DELETE endpoint notation
alex-odysseus Jul 17, 2024
948fde3
Fixing the previous commit as the endpoint is DELETE /conceptset/anno…
alex-odysseus Jul 17, 2024
0fff858
Fixing the notation as the endpoint is DELETE /conceptset/annotation/…
alex-odysseus Jul 17, 2024
7adb69b
[ATL-58] Added concept set version to annotations
oleg-odysseus Jul 26, 2024
f40a666
[ATL-58] Implemented copying of annotations along with ConceptSet
oleg-odysseus Jul 30, 2024
2b9c35a
[ATL-58] Fixed permissions issues for annotations feature, transforme…
oleg-odysseus Aug 6, 2024
485b41a
Fix for annotation search data shown as JSON instead of human readabl…
oleg-odysseus Aug 22, 2024
937c72e
Added 'Copied From' list of concept set ids for concept set annotations
oleg-odysseus Sep 12, 2024
6a88e5d
Added i18n records for Origin Concept Sets ('Copied From' feature in …
oleg-odysseus Sep 18, 2024
deed559
A combined migration script has been assembled
alex-odysseus Nov 6, 2024
1f6476c
Changed owner/permissions check for annotations delete to consider ow…
oleg-odysseus Nov 9, 2024
4ee6b48
Added migration script to re-create annotation-related permissions. A…
oleg-odysseus Nov 9, 2024
57491c3
Changed SearchDataTransformer in accordance with the community discus…
oleg-odysseus Nov 9, 2024
4c3a304
Permissions update, concept_set_version column type change to integer
alex-odysseus Dec 3, 2024
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
2 changes: 1 addition & 1 deletion src/main/java/org/ohdsi/webapi/conceptset/ConceptSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
*
* @author fdefalco
*/
@Entity(name="ConceptSet")
@Entity(name = "ConceptSet")
@Table(name="concept_set")
public class ConceptSet extends CommonEntityExt<Integer> implements Serializable {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

@Entity(name = "ConceptSetAnnotation")
@Table(name = "concept_set_annotation")
public class ConceptSetAnnotation extends CommonEntity<Integer> implements Serializable {
public class ConceptSetAnnotation implements Serializable {
/**
*
*/
Expand Down Expand Up @@ -45,7 +45,7 @@ public class ConceptSetAnnotation extends CommonEntity<Integer> implements Seria
private String vocabularyVersion;

@Column(name = "concept_set_version")
private String conceptSetVersion;
private Integer conceptSetVersion;

@Column(name = "copied_from_concept_set_ids")
private String copiedFromConceptSetIds;
Expand Down Expand Up @@ -90,11 +90,11 @@ public void setVocabularyVersion(String vocabularyVersion) {
this.vocabularyVersion = vocabularyVersion;
}

public String getConceptSetVersion() {
public Integer getConceptSetVersion() {
return conceptSetVersion;
}

public void setConceptSetVersion(String conceptSetVersion) {
public void setConceptSetVersion(Integer conceptSetVersion) {
this.conceptSetVersion = conceptSetVersion;
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,21 @@ public class ConceptSetPermissionSchema extends EntityPermissionSchema {
private static Map<String, String> writePermissions = new HashMap<String, String>() {{
put("conceptset:%s:put", "Update Concept Set with ID = %s");
put("conceptset:%s:items:put", "Update Items of Concept Set with ID = %s");
put("conceptset:*:annotation:put", "Create Concept Set Annotation");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should not be * permissions. What happens is that when you delete the entity, it finds the write permission schema and deletes it, thus deleting the wildcard permission' from all users, and then preventing access to the endpiont for all users.

However, this is what happens when you extend the 'CommonEntity' which you didn't do in this case, however, ou are using a permissionSchema as if it was a common entity, so there may be a little confusion here.

But, in any case, we shouldn't see * permissions in any writePermission schema because these permission schemas are targeting a specific entity ID.

put("conceptset:%s:annotation:*:delete", "Delete Annotations of Concept Set with ID = %s");
put("conceptset:*:annotation:*:delete", "Delete Annotations of any Concept Set");
put("conceptset:%s:delete", "Delete Concept Set with ID = %s");
}};

private static Map<String, String> readPermissions = new HashMap<String, String>() {{
put("conceptset:%s:get", "view conceptset definition with id %s");
put("conceptset:%s:expression:get", "Resolve concept set %s expression");
put("conceptset:%s:annotation:get", "Resolve concept set annotations");
put("conceptset:*:annotation:get", "Resolve concept set annotations");
put("conceptset:%s:version:*:expression:get", "Get expression for concept set %s items for default source");
}};

public ConceptSetPermissionSchema() {

super(EntityType.CONCEPT_SET, readPermissions, writePermissions);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import org.ohdsi.webapi.cohortdefinition.CohortDefinition;
import org.ohdsi.webapi.cohortsample.CohortSample;
import org.ohdsi.webapi.conceptset.ConceptSet;
import org.ohdsi.webapi.conceptset.annotation.ConceptSetAnnotation;
import org.ohdsi.webapi.estimation.Estimation;
import org.ohdsi.webapi.feanalysis.domain.FeAnalysisEntity;
import org.ohdsi.webapi.ircalc.IncidenceRateAnalysis;
Expand All @@ -27,9 +26,7 @@ public enum EntityType {
PREDICTION(PredictionAnalysis.class),
COHORT_SAMPLE(CohortSample.class),
TAG(Tag.class),
REUSABLE(Reusable.class),
CONCEPT_SET_ANNOTATION(ConceptSetAnnotation.class);

REUSABLE(Reusable.class);
private final Class<? extends CommonEntity> entityClass;

EntityType(Class<? extends CommonEntity> entityClass) {
Expand Down
66 changes: 20 additions & 46 deletions src/main/java/org/ohdsi/webapi/service/ConceptSetService.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package org.ohdsi.webapi.service;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
Expand All @@ -29,8 +28,6 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.shiro.authz.UnauthorizedException;
import org.ohdsi.circe.vocabulary.ConceptSetExpression;
import org.ohdsi.vocabulary.Concept;
Expand Down Expand Up @@ -895,7 +892,7 @@ private ConceptSetVersion saveVersion(int id) {
@Transactional
public boolean saveConceptSetAnnotation(@PathParam("id") final int conceptSetId, SaveConceptSetAnnotationsRequest request) {
removeAnnotations(conceptSetId, request);
if (CollectionUtils.isNotEmpty(request.getNewAnnotation())) {
if (request.getNewAnnotation() != null && !request.getNewAnnotation().isEmpty()) {
List<ConceptSetAnnotation> annotationList = request.getNewAnnotation()
.stream()
.map(newAnnotationData -> {
Expand All @@ -908,13 +905,12 @@ public boolean saveConceptSetAnnotation(@PathParam("id") final int conceptSetId,
annotationDetailsDTO.setSearchData(newAnnotationData.getSearchData());
conceptSetAnnotation.setAnnotationDetails(mapper.writeValueAsString(annotationDetailsDTO));
} catch (JsonProcessingException e) {
log.error("Could not serialize Concept Set AnnotationDetailsDTO", e);
throw new RuntimeException(e);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a log entry

}
conceptSetAnnotation.setVocabularyVersion(newAnnotationData.getVocabularyVersion());
conceptSetAnnotation.setConceptSetVersion(newAnnotationData.getConceptSetVersion());
conceptSetAnnotation.setConceptId(newAnnotationData.getConceptId());
conceptSetAnnotation.setCreatedBy(getCurrentUser());
conceptSetAnnotation.setCreatedDate(new Date());
return conceptSetAnnotation;
}).collect(Collectors.toList());

Expand All @@ -924,46 +920,40 @@ public boolean saveConceptSetAnnotation(@PathParam("id") final int conceptSetId,
return true;
}
private void removeAnnotations(int id, SaveConceptSetAnnotationsRequest request){
if (CollectionUtils.isNotEmpty(request.getRemoveAnnotation())) {
if (request.getRemoveAnnotation() != null && !request.getRemoveAnnotation().isEmpty()) {
for (AnnotationDTO annotationDTO : request.getRemoveAnnotation()) {
this.getConceptSetAnnotationRepository().deleteAnnotationByConceptSetIdAndConceptId(id, annotationDTO.getConceptId());
}
}
}
@POST
@Path("/copy-annotations")
@Produces(MediaType.APPLICATION_JSON)
@Transactional
public void copyAnnotations(CopyAnnotationsRequest copyAnnotationsRequest ) {
List<ConceptSetAnnotation> sourceAnnotations = getConceptSetAnnotationRepository().findByConceptSetId(copyAnnotationsRequest.getSourceConceptSetId());
List<ConceptSetAnnotation> copiedAnnotations= sourceAnnotations.stream()
.map(sourceAnnotation -> copyAnnotation(sourceAnnotation, copyAnnotationsRequest.getSourceConceptSetId(), copyAnnotationsRequest.getTargetConceptSetId()))
.collect(Collectors.toList());
getConceptSetAnnotationRepository().save(copiedAnnotations);
}
private ConceptSetAnnotation copyAnnotation(ConceptSetAnnotation sourceConceptSetAnnotation, int sourceConceptSetId, int targetConceptSetId){
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move it down below the public method where it is used

ConceptSetAnnotation targetConceptSetAnnotation = new ConceptSetAnnotation();
targetConceptSetAnnotation.setConceptSetId(targetConceptSetId);
targetConceptSetAnnotation.setConceptSetVersion(sourceConceptSetAnnotation.getConceptSetVersion());
targetConceptSetAnnotation.setAnnotationDetails(sourceConceptSetAnnotation.getAnnotationDetails());
targetConceptSetAnnotation.setConceptId(sourceConceptSetAnnotation.getConceptId());
targetConceptSetAnnotation.setVocabularyVersion(sourceConceptSetAnnotation.getVocabularyVersion());
targetConceptSetAnnotation.setCreatedBy(sourceConceptSetAnnotation.getCreatedBy());
targetConceptSetAnnotation.setCreatedDate(sourceConceptSetAnnotation.getCreatedDate());
targetConceptSetAnnotation.setModifiedBy(sourceConceptSetAnnotation.getModifiedBy());
targetConceptSetAnnotation.setModifiedDate(sourceConceptSetAnnotation.getModifiedDate());
targetConceptSetAnnotation.setCopiedFromConceptSetIds(appendCopiedFromConceptSetId(sourceConceptSetAnnotation.getCopiedFromConceptSetIds(), sourceConceptSetId));
return targetConceptSetAnnotation;
}

private String appendCopiedFromConceptSetId(String copiedFromConceptSetIds, int sourceConceptSetId) {
if(StringUtils.isEmpty(copiedFromConceptSetIds)){
if(copiedFromConceptSetIds == null || copiedFromConceptSetIds.isEmpty()){
return Integer.toString(sourceConceptSetId);
}
return copiedFromConceptSetIds.concat(",").concat(Integer.toString(sourceConceptSetId));
}

@POST
@Path("/copy-annotations")
@Produces(MediaType.APPLICATION_JSON)
@Transactional
public void copyAnnotations(CopyAnnotationsRequest copyAnnotationsRequest ) {
List<ConceptSetAnnotation> sourceAnnotations = getConceptSetAnnotationRepository().findByConceptSetId(copyAnnotationsRequest.getSourceConceptSetId());
List<ConceptSetAnnotation> copiedAnnotations= sourceAnnotations.stream()
.map(sourceAnnotation -> copyAnnotation(sourceAnnotation, copyAnnotationsRequest.getSourceConceptSetId(), copyAnnotationsRequest.getTargetConceptSetId()))
.collect(Collectors.toList());
getConceptSetAnnotationRepository().save(copiedAnnotations);
}

@GET
@Path("/{id}/annotation")
@Produces(MediaType.APPLICATION_JSON)
Expand All @@ -980,6 +970,7 @@ private AnnotationDTO convertAnnotationEntityToDTO(ConceptSetAnnotation conceptS
try {
annotationDetails = mapper.readValue(conceptSetAnnotation.getAnnotationDetails(), AnnotationDetailsDTO.class);
} catch (JsonProcessingException e) {
log.error("Could not deserialize Concept Set AnnotationDetailsDTO", e);
throw new RuntimeException(e);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A log entry should be added additionally

}

Expand All @@ -995,34 +986,17 @@ private AnnotationDTO convertAnnotationEntityToDTO(ConceptSetAnnotation conceptS
annotationDTO.setVocabularyVersion(conceptSetAnnotation.getVocabularyVersion());
annotationDTO.setConceptSetVersion(conceptSetAnnotation.getConceptSetVersion());
annotationDTO.setCopiedFromConceptSetIds(conceptSetAnnotation.getCopiedFromConceptSetIds());
annotationDTO.setCreatedBy(conceptSetAnnotation.getCreatedBy() != null ? conceptSetAnnotation.getCreatedBy().getName() : null);
annotationDTO.setCreatedDate(conceptSetAnnotation.getCreatedDate() != null ? conceptSetAnnotation.getCreatedDate().toString() : null);

return annotationDTO;
}

@DELETE
@Path("/annotation/{id}")
@Path("/{conceptSetId}/annotation/{annotationId}")
@Produces(MediaType.APPLICATION_JSON)
public Response deleteConceptSetAnnotation(@PathParam("id") final int id) {
ConceptSetAnnotation conceptSetAnnotation = getConceptSetAnnotationRepository().findById(id);
public Response deleteConceptSetAnnotation(@PathParam("conceptSetId") final int conceptSetId, @PathParam("annotationId") final int annotationId) {
ConceptSetAnnotation conceptSetAnnotation = getConceptSetAnnotationRepository().findById(annotationId);
if (conceptSetAnnotation != null) {
getConceptSetAnnotationRepository().deleteById(id);
getConceptSetAnnotationRepository().deleteById(annotationId);
return Response.ok().build();
} else throw new NotFoundException("Concept set annotation not found");
}

@PUT
@Path("/update/{id}/annotation")
@Produces(MediaType.APPLICATION_JSON)
public AnnotationDTO updateConceptSetAnnotation(@PathParam("id") final int id, AnnotationDTO annotationDTO) throws IOException {
ConceptSetAnnotation conceptSetAnnotation = getConceptSetAnnotationRepository()
.findConceptSetAnnotationByConceptIdAndConceptId(id, annotationDTO.getConceptId())
.orElseThrow(() -> new RuntimeException("Concept set annotation not found"));
conceptSetAnnotation.setAnnotationDetails(mapper.writeValueAsString(annotationDTO));
conceptSetAnnotation.setModifiedBy(getCurrentUser());
conceptSetAnnotation.setModifiedDate(new Date());
getConceptSetAnnotationRepository().save(conceptSetAnnotation);
return annotationDTO;
}
}
Loading
Loading