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

Tools module functionality #2389

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 0 deletions src/main/java/org/ohdsi/webapi/security/model/EntityType.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.ohdsi.webapi.reusable.domain.Reusable;
import org.ohdsi.webapi.source.Source;
import org.ohdsi.webapi.tag.domain.Tag;
import org.ohdsi.webapi.tool.Tool;

public enum EntityType {
COHORT_DEFINITION(CohortDefinition.class),
Expand All @@ -26,6 +27,7 @@ public enum EntityType {
PREDICTION(PredictionAnalysis.class),
COHORT_SAMPLE(CohortSample.class),
TAG(Tag.class),
TOOL(Tool.class),
REUSABLE(Reusable.class);

private final Class<? extends CommonEntity> entityClass;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package org.ohdsi.webapi.security.model;

import org.springframework.stereotype.Component;

import java.util.HashMap;
import java.util.Map;

@Component
public class ToolPermissionSchema extends EntityPermissionSchema {
private static Map<String, String> writePermissions = new HashMap<String, String>() {{
put("tool:%s:delete", "Delete Tool with id = %s");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am afraid that the notation doesn't coincide with what is inserted by the migration scripts

put("tool:%s:put", "Update Tool with id = %s");
put("tool:%s:post", "Create Tool with id = %s");
}};
private static Map<String, String> readPermissions = new HashMap<String, String>() {
{
put("tool:%s:get", "View Tool with id = %s");
}
};

public ToolPermissionSchema() {
super(EntityType.TOOL, readPermissions, writePermissions);
}

}
93 changes: 93 additions & 0 deletions src/main/java/org/ohdsi/webapi/tool/Tool.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package org.ohdsi.webapi.tool;

import org.hibernate.annotations.GenericGenerator;
import org.hibernate.annotations.Parameter;
import org.ohdsi.webapi.model.CommonEntity;

import javax.persistence.Entity;
import javax.persistence.Table;
import javax.persistence.Id;
import javax.persistence.GeneratedValue;
import javax.persistence.Column;
import java.util.Objects;

@Entity
@Table(name = "tool")
public class Tool extends CommonEntity<Integer> {
@Id
@GenericGenerator(
name = "tool_generator",
strategy = "org.hibernate.id.enhanced.SequenceStyleGenerator",
parameters = {
@Parameter(name = "sequence_name", value = "tool_seq"),
@Parameter(name = "increment_size", value = "1")
}
)
@GeneratedValue(generator = "tool_generator")
private Integer id;

@Column(name = "name")
private String name;

@Column(name = "url")
private String url;

@Column(name = "description")
private String description;
@Column(name = "is_enabled")
private Boolean isEnabled;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The property name should be renamed to 'enabled'


@Override
public Integer getId() {
return id;
}

public void setId(Integer id) {
this.id = id;
}

public String getName() {
return name;
}

public void setName(String name) {
this.name = name;
}

public String getUrl() {
return url;
}

public void setUrl(String url) {
this.url = url;
}

public String getDescription() {
return description;
}

public void setDescription(String description) {
this.description = description;
}

public Boolean getEnabled() {
return isEnabled;
}

public void setEnabled(Boolean enabled) {
isEnabled = enabled;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Tool tool = (Tool) o;
return Objects.equals(name, tool.name);
}

@Override
public int hashCode() {
return Objects.hash(name);
}
}
55 changes: 55 additions & 0 deletions src/main/java/org/ohdsi/webapi/tool/ToolController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package org.ohdsi.webapi.tool;

import org.ohdsi.webapi.tool.dto.ToolDTO;
import org.springframework.stereotype.Controller;

import javax.ws.rs.*;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Imports with * should be eliminated

import javax.ws.rs.core.MediaType;
import java.util.List;

@Controller
@Path("/tool")
public class ToolController {
private final ToolServiceImpl service;

public ToolController(ToolServiceImpl service) {
this.service = service;
}

@GET
@Path("")
@Produces(MediaType.APPLICATION_JSON)
public List<ToolDTO> getTools() {
return service.getTools();
}

@GET
@Path("/{id}")
@Produces(MediaType.APPLICATION_JSON)
public ToolDTO getToolById(@PathParam("id") Integer id) {
return service.getById(id);
}

@POST
@Path("")
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
public ToolDTO createTool(ToolDTO dto) {
return service.saveTool(dto);
}

@DELETE
@Path("/{id}")
@Produces(MediaType.APPLICATION_JSON)
public void delete(@PathParam("id") Integer id) {
service.delete(id);
}

@PUT
@Path("")
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
public ToolDTO updateTool(ToolDTO toolDTO) {
return service.saveTool(toolDTO);
}
}
11 changes: 11 additions & 0 deletions src/main/java/org/ohdsi/webapi/tool/ToolRepository.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package org.ohdsi.webapi.tool;

import java.util.List;

import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;

@Repository
public interface ToolRepository extends JpaRepository<Tool, Integer> {
List<Tool> findAllByIsEnabled(boolean isEnabled);
}
13 changes: 13 additions & 0 deletions src/main/java/org/ohdsi/webapi/tool/ToolService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.ohdsi.webapi.tool;

import org.ohdsi.webapi.tool.dto.ToolDTO;

import java.util.List;

public interface ToolService {
List<ToolDTO> getTools();
ToolDTO saveTool(ToolDTO toolDTO);
ToolDTO getById(Integer id);

void delete(Integer id);
}
64 changes: 64 additions & 0 deletions src/main/java/org/ohdsi/webapi/tool/ToolServiceImpl.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package org.ohdsi.webapi.tool;

import java.util.List;
import java.util.stream.Collectors;

import javax.ws.rs.ForbiddenException;

import org.ohdsi.webapi.service.AbstractDaoService;
import org.ohdsi.webapi.shiro.Entities.UserEntity;
import org.ohdsi.webapi.tool.converter.ToolConvertor;
import org.ohdsi.webapi.tool.dto.ToolDTO;
import org.springframework.stereotype.Service;

@Service
public class ToolServiceImpl extends AbstractDaoService implements ToolService {
private final ToolRepository toolRepository;
private final ToolConvertor toolConvertor;

public ToolServiceImpl(ToolRepository toolRepository, ToolConvertor toolConvertor) {
this.toolRepository = toolRepository;
this.toolConvertor = toolConvertor;
}

@Override
public List<ToolDTO> getTools() {
List<Tool> tools = isAdmin() ? toolRepository.findAll() : toolRepository.findAllByIsEnabled(true);
return tools.stream()
.map(tool -> {
ToolDTO dto = toolConvertor.toDTO(tool);
return dto;
}).collect(Collectors.toList());
}

@Override
public ToolDTO saveTool(ToolDTO toolDTO) {
if (!isAdmin()) {
throw new ForbiddenException();
}
Tool tool = saveToolFromDTO(toolDTO, getCurrentUser());
return toolConvertor.toDTO(toolRepository.saveAndFlush(tool));
}

private Tool saveToolFromDTO(ToolDTO toolDTO, UserEntity currentUser) {
Tool tool = toolConvertor.toEntity(toolDTO);
if (toolDTO.getId() == null) {
tool.setCreatedBy(currentUser);
}
tool.setModifiedBy(currentUser);
return tool;
}

@Override
public ToolDTO getById(Integer id) {
return toolConvertor.toDTO(toolRepository.findOne(id));
}

@Override
public void delete(Integer id) {
if (!isAdmin()) {
throw new ForbiddenException();
}
toolRepository.delete(id);
}
}
82 changes: 82 additions & 0 deletions src/main/java/org/ohdsi/webapi/tool/converter/ToolConvertor.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package org.ohdsi.webapi.tool.converter;

import org.ohdsi.webapi.service.AbstractDaoService;
import org.ohdsi.webapi.shiro.Entities.UserEntity;
import org.ohdsi.webapi.shiro.Entities.UserRepository;
import org.ohdsi.webapi.shiro.PermissionManager;
import org.ohdsi.webapi.tool.Tool;
import org.ohdsi.webapi.tool.ToolRepository;
import org.ohdsi.webapi.tool.dto.ToolDTO;
import org.ohdsi.webapi.util.DateUtils;
import org.springframework.stereotype.Component;

import java.time.Instant;
import java.util.Date;
import java.util.Optional;

@Component
public class ToolConvertor extends AbstractDaoService {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we need a separate class, to be moved to ToolServiceImpl

private final ToolRepository toolRepository;
private final UserRepository userRepository;

public ToolConvertor(ToolRepository toolRepository, UserRepository userRepository) {
this.toolRepository = toolRepository;
this.userRepository = userRepository;
}

public Tool toEntity(ToolDTO toolDTO) {
boolean isNewTool = toolDTO.getId() == null;
Tool tool = isNewTool ? new Tool() : toolRepository.findOne(toolDTO.getId());
Instant currentInstant = Instant.now();
if (isNewTool) {
setCreationDetails(tool, currentInstant);
} else {
setModificationDetails(tool, currentInstant);
}
updateToolFromDTO(tool, toolDTO);
return tool;
}

private void setCreationDetails(Tool tool, Instant currentInstant) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we need two separate methods

tool.setCreatedDate(Date.from(currentInstant));
tool.setCreatedBy(getCurrentUser());
}

private void setModificationDetails(Tool tool, Instant currentInstant) {
tool.setModifiedDate(Date.from(currentInstant));
tool.setModifiedBy(getCurrentUser());
}

private void updateToolFromDTO(Tool tool, ToolDTO toolDTO) {
Optional.ofNullable(toolDTO.getName()).ifPresent(tool::setName);
Optional.ofNullable(toolDTO.getUrl()).ifPresent(tool::setUrl);
Optional.ofNullable(toolDTO.getDescription()).ifPresent(tool::setDescription);
Optional.ofNullable(toolDTO.getEnabled()).ifPresent(tool::setEnabled);
}

public ToolDTO toDTO(Tool tool) {
return Optional.ofNullable(tool)
.map(t -> {
ToolDTO toolDTO = new ToolDTO();
toolDTO.setId(t.getId());
toolDTO.setName(t.getName());
toolDTO.setUrl(t.getUrl());
toolDTO.setDescription(t.getDescription());
Optional.ofNullable(tool.getCreatedBy())
.map(UserEntity::getId)
.map(userRepository::findOne)
.map(UserEntity::getName)
.ifPresent(toolDTO::setCreatedByName);
Optional.ofNullable(tool.getModifiedBy())
.map(UserEntity::getId)
.map(userRepository::findOne)
.map(UserEntity::getName)
.ifPresent(toolDTO::setModifiedByName);
toolDTO.setCreatedDate(DateUtils.dateToString(t.getCreatedDate()));
toolDTO.setModifiedDate(DateUtils.dateToString(t.getModifiedDate()));
toolDTO.setEnabled(t.getEnabled());
return toolDTO;
})
.orElse(null);
}
}
Loading
Loading