Add constraints on model entities names #295
Conversation
masayag
changed the title
workflow-service/src/main/java/com/redhat/parodos/project/controller/ProjectController.java
Outdated
Show resolved
Hide resolved
masayag
force-pushed
the
entities_constraints
branch
from
3e44e37 to
a626a65
Compare
| @@ -54,6 +55,10 @@ public ProjectServiceImpl(ProjectRepository projectRepository, WorkFlowRepositor | |||
|
|
|||
| @Override | |||
| public ProjectResponseDTO save(ProjectRequestDTO projectRequestDTO) { | |||
| if (projectRepository.findByName(projectRequestDTO.getName()).isPresent()) { | |||
There was a problem hiding this comment.
This is perhaps the best time to address the case sensitiveness by simple ignoring case.
There was a problem hiding this comment.
Do you think we should enforce it on DB level as well?
| @@ -26,4 +28,6 @@ | |||
| */ | |||
| public interface ProjectRepository extends JpaRepository<Project, UUID> { | |||
|
|
|||
| Optional<Project> findByName(String name); | |||
There was a problem hiding this comment.
In line with my comment below, can we switch to findByNameIgnoreCase this time?
| if (projectRepository.findByName(projectRequestDTO.getName()).isPresent()) { | ||
| throw new EntityExistsException( | ||
| String.format("Project with name: %s already exists", projectRequestDTO.getName())); | ||
| } | ||
| // get user from security utils and set on project | ||
| Project project = projectRepository.save(Project.builder().name(projectRequestDTO.getName()) | ||
| .description(projectRequestDTO.getDescription()).createDate(new Date()).modifyDate(new Date()).build()); |
There was a problem hiding this comment.
Moving forwards, what do you think of setting createDate and modifyDate via sysdate at the db level?
There was a problem hiding this comment.
That's a tricky one :-)
It depends if the clock of the Postgres-db server is in sync with the workflow service.
If the importance is when the request is sent by the user - then I'd keep the current code.
If the importance is the actual record creation time - then use sysdate.
There was a problem hiding this comment.
Agreed with your points! Since we don't specific requirements on this, let's keep as-is :)
We may revisit later
| @@ -28,6 +29,6 @@ | |||
|
|
|||
| public interface UserRepository extends JpaRepository<User, UUID> { | |||
|
|
|||
| List<User> findByUsername(String username); | |||
| Optional<User> findByUsername(String username); | |||
There was a problem hiding this comment.
Should we ignore case here? It makes more sense in project name from my comment above.
There was a problem hiding this comment.
How users are being created in the system? will there be a need to import users from other systems?
There was a problem hiding this comment.
It's still work in progress. Happy to keep it as-is based in the discussion today.
Users and projects should be uniquely identified by their names in the system. The PRs adds data-base level and entity reousrce constraints to prevent a creation of the same entities (project and user) with the same name. Signed-off-by: Moti Asayag <masayag@redhat.com>
Since project-name should be unique, the run_example.sh script should produce a unique name for each test. Signed-off-by: Moti Asayag <masayag@redhat.com>
masayag
force-pushed
the
entities_constraints
branch
from
a626a65 to
481c9b7
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: anludke The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
Users and projects should be uniquely identified by their names in the system.
To enforce this, unique constraints should be implemented at the database level.
When requests are made to the controller's endpoints, they should fail with an HTTP status code of 409 (Conflict) if the constraints are violated.
To simplify JPA testing,
RepositoryTestBasewas added to serve as a base test class for the *Repository test classes.Which issue(s) this PR fixes *:
Fixes: #FLPATH-341
Change type
Impacted services
Checklist