Skip to content

Commit afb7a76

Browse files
Shyam-VishwakarmaShyam-Vishwakarma
authored
fix: avoid creating duplicate endorsements, add integration-tests for endorsement creation (#198)
* fix: avoid creating duplicate endorsements, add integration tests * fix: update name of the test * fix: update formatting * test: add test to validate skill creation on a diff skill than those that have endorsements * fix: update query formatting * fix: update query to scan only endorsement table, as joining user-skills is redundant * test: add test-case to test flow when user is unauthorized * fix: update test name, createUrl(), log * add class to contain constant messages across app * fix: use constants defined in Constants.java * fix: use constants defined in Constants.java * fix: make constructor of Constants private to prevent instantiation * fix: import ExceptionMessages from Constants * fix: update status from 409 to 405 * fix: update query, subquery is redundant * fix: add logging level for each envioronment * fix: avoid appending timestamps, message etc, update error message * fix: add constructor for GenericResponse to create obj with message only
1 parent 55c32a2 commit afb7a76

11 files changed

Lines changed: 472 additions & 78 deletions

File tree

‎skill-tree/src/main/java/com/RDS/skilltree/exceptions/EndorsementAlreadyExistsException.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package com.RDS.skilltree.exceptions;
2+
3+
public class EndorsementAlreadyExistsException extends RuntimeException {
4+
public EndorsementAlreadyExistsException(String message) {
5+
super(message);
6+
}
7+
}

‎skill-tree/src/main/java/com/RDS/skilltree/exceptions/GlobalExceptionHandler.java‎

Lines changed: 43 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,7 @@
22

33
import com.RDS.skilltree.utils.GenericResponse;
44
import jakarta.validation.ConstraintViolationException;
5-
import java.time.LocalDateTime;
6-
import java.util.HashMap;
75
import java.util.List;
8-
import java.util.Map;
96
import lombok.extern.slf4j.Slf4j;
107
import org.apache.tomcat.websocket.AuthenticationException;
118
import org.springframework.http.HttpStatus;
@@ -16,16 +13,15 @@
1613
import org.springframework.web.bind.MethodArgumentNotValidException;
1714
import org.springframework.web.bind.annotation.ControllerAdvice;
1815
import org.springframework.web.bind.annotation.ExceptionHandler;
19-
import org.springframework.web.context.request.WebRequest;
2016

2117
@Slf4j
2218
@ControllerAdvice
2319
public class GlobalExceptionHandler {
20+
2421
@ExceptionHandler({NoEntityException.class})
2522
public ResponseEntity<GenericResponse<Object>> handleNoEntityException(NoEntityException ex) {
26-
log.error("NoEntityException - Error : {}", ex.getMessage(), ex);
27-
return ResponseEntity.status(HttpStatus.NOT_FOUND)
28-
.body(new GenericResponse<>(null, ex.getMessage()));
23+
log.error("NoEntityException - Error : {}", ex.getMessage());
24+
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(new GenericResponse<>(ex.getMessage()));
2925
}
3026

3127
@ExceptionHandler({AuthenticationException.class, InsufficientAuthenticationException.class})
@@ -40,25 +36,21 @@ public ResponseEntity<GenericResponse<Object>> handleInvalidBearerTokenException
4036
@ExceptionHandler({AccessDeniedException.class})
4137
public ResponseEntity<GenericResponse<Object>> handleAccessDeniedException(
4238
AccessDeniedException ex) {
43-
return ResponseEntity.status(HttpStatus.FORBIDDEN)
44-
.body(new GenericResponse<>(null, ex.getMessage()));
39+
return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new GenericResponse<>(ex.getMessage()));
4540
}
4641

4742
@ExceptionHandler({EntityAlreadyExistsException.class})
4843
public ResponseEntity<GenericResponse<Object>> handleEntityAlreadyExistsException(
4944
EntityAlreadyExistsException ex) {
50-
log.error("EntityAlreadyExistsException - Error : {}", ex.getMessage(), ex);
51-
return ResponseEntity.status(HttpStatus.CONFLICT)
52-
.body(new GenericResponse<>(null, ex.getMessage()));
45+
log.error("EntityAlreadyExistsException - Error : {}", ex.getMessage());
46+
return ResponseEntity.status(HttpStatus.CONFLICT).body(new GenericResponse<>(ex.getMessage()));
5347
}
5448

5549
@ExceptionHandler({RuntimeException.class})
5650
public ResponseEntity<GenericResponse<Object>> handleRuntimeException(RuntimeException ex) {
57-
log.error("Runtime Exception - Error : {}", ex.getMessage(), ex);
51+
log.error("Runtime Exception - Error : {}", ex.getMessage());
5852
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
59-
.body(
60-
new GenericResponse<>(
61-
null, "Runtime Exception - Something went wrong, please try again."));
53+
.body(new GenericResponse<>("Runtime Exception - Something went wrong, please try again."));
6254
}
6355

6456
@ExceptionHandler({MethodArgumentNotValidException.class})
@@ -73,85 +65,81 @@ public ResponseEntity<GenericResponse<Object>> handleMethodArgumentNotValidExcep
7365
}
7466
log.error("MethodArgumentNotValidException Exception - Error : {}", ex.getMessage(), ex);
7567
return ResponseEntity.status(HttpStatus.BAD_REQUEST)
76-
.body(new GenericResponse<>(null, errorString.toString().trim()));
68+
.body(new GenericResponse<>(errorString.toString().trim()));
7769
}
7870

7971
@ExceptionHandler({Exception.class})
8072
public ResponseEntity<GenericResponse<Object>> handleException(Exception ex) {
81-
log.error("Exception - Error : {}", ex.getMessage(), ex);
73+
log.error("Exception - Error : {}", ex.getMessage());
8274
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
83-
.body(new GenericResponse<>(null, "Something unexpected happened, please try again."));
75+
.body(new GenericResponse<>("Something unexpected happened, please try again."));
8476
}
8577

8678
@ExceptionHandler({InvalidParameterException.class})
8779
public ResponseEntity<GenericResponse<Object>> handleException(InvalidParameterException ex) {
88-
log.error("Exception - Error : {}", ex.getMessage(), ex);
80+
log.error("InvalidParameterException - Error : {}", ex.getMessage());
8981
return ResponseEntity.status(HttpStatus.BAD_REQUEST)
90-
.body(new GenericResponse<>(null, ex.getMessage()));
82+
.body(new GenericResponse<>(ex.getMessage()));
9183
}
9284

9385
@ExceptionHandler({ConstraintViolationException.class})
9486
public ResponseEntity<GenericResponse<Object>> handleException(ConstraintViolationException ex) {
95-
log.error("Exception - Error : {}", ex.getMessage(), ex);
87+
log.error("ConstraintViolationException - Error : {}", ex.getMessage());
9688
return ResponseEntity.status(HttpStatus.BAD_REQUEST)
97-
.body(new GenericResponse<>(null, ex.getMessage()));
89+
.body(new GenericResponse<>(ex.getMessage()));
9890
}
9991

10092
@ExceptionHandler(UserNotFoundException.class)
101-
public ResponseEntity<?> handleUserNotFoundException(
102-
UserNotFoundException ex, WebRequest request) {
103-
log.error("Exception - Error : {}", ex.getMessage(), ex);
104-
return new ResponseEntity<>(new GenericResponse<>(null, ex.getMessage()), HttpStatus.NOT_FOUND);
93+
public ResponseEntity<?> handleUserNotFoundException(UserNotFoundException ex) {
94+
log.error("UserNotFoundException - Error : {}", ex.getMessage());
95+
return new ResponseEntity<>(new GenericResponse<>(ex.getMessage()), HttpStatus.NOT_FOUND);
10596
}
10697

10798
@ExceptionHandler(SkillAlreadyExistsException.class)
108-
public ResponseEntity<?> handleSkillAlreadyExistsException(
109-
SkillAlreadyExistsException ex, WebRequest request) {
110-
log.error("Exception - Error : {}", ex.getMessage(), ex);
111-
return new ResponseEntity<>(new GenericResponse<>(null, ex.getMessage()), HttpStatus.CONFLICT);
99+
public ResponseEntity<?> handleSkillAlreadyExistsException(SkillAlreadyExistsException ex) {
100+
log.error("SkillAlreadyExistsException - Error : {}", ex.getMessage());
101+
return new ResponseEntity<>(new GenericResponse<>(ex.getMessage()), HttpStatus.CONFLICT);
112102
}
113103

114104
@ExceptionHandler(SelfEndorsementNotAllowedException.class)
115105
public ResponseEntity<?> handleSelfEndorsementNotAllowedException(
116-
SelfEndorsementNotAllowedException ex, WebRequest request) {
117-
log.error("Exception - Error : {}", ex.getMessage(), ex);
106+
SelfEndorsementNotAllowedException ex) {
107+
log.error("SelfEndorsementNotAllowedException - Error : {}", ex.getMessage());
118108
return new ResponseEntity<>(
119-
new GenericResponse<>(null, ex.getMessage()), HttpStatus.METHOD_NOT_ALLOWED);
109+
new GenericResponse<>(ex.getMessage()), HttpStatus.METHOD_NOT_ALLOWED);
120110
}
121111

122112
@ExceptionHandler(SkillNotFoundException.class)
123-
public ResponseEntity<?> handleSkillNotFoundException(
124-
SkillNotFoundException ex, WebRequest request) {
125-
log.error("Exception - Error : {}", ex.getMessage(), ex);
126-
return new ResponseEntity<>(new GenericResponse<>(null, ex.getMessage()), HttpStatus.NOT_FOUND);
113+
public ResponseEntity<?> handleSkillNotFoundException(SkillNotFoundException ex) {
114+
log.error("SkillNotFoundException - Error : {}", ex.getMessage());
115+
return new ResponseEntity<>(new GenericResponse<>(ex.getMessage()), HttpStatus.NOT_FOUND);
127116
}
128117

129118
@ExceptionHandler(EndorsementNotFoundException.class)
130-
public ResponseEntity<?> handleEndorsementNotException(
131-
EndorsementNotFoundException ex, WebRequest request) {
132-
log.error("Exception - Error : {}", ex.getMessage(), ex);
133-
return new ResponseEntity<>(new GenericResponse<>(null, ex.getMessage()), HttpStatus.NOT_FOUND);
119+
public ResponseEntity<?> handleEndorsementNotFoundException(EndorsementNotFoundException ex) {
120+
log.error("EndorsementNotFoundException - Error : {}", ex.getMessage());
121+
return new ResponseEntity<>(new GenericResponse<>(ex.getMessage()), HttpStatus.NOT_FOUND);
134122
}
135123

136124
@ExceptionHandler(ForbiddenException.class)
137-
public ResponseEntity<?> handleForbiddenException(ForbiddenException ex, WebRequest request) {
138-
log.error("Exception - Error : {}", ex.getMessage(), ex);
139-
return new ResponseEntity<>(new GenericResponse<>(null, ex.getMessage()), HttpStatus.FORBIDDEN);
125+
public ResponseEntity<?> handleForbiddenException(ForbiddenException ex) {
126+
log.error("ForbiddenException - Error : {}", ex.getMessage());
127+
return new ResponseEntity<>(new GenericResponse<>(ex.getMessage()), HttpStatus.FORBIDDEN);
140128
}
141129

142130
@ExceptionHandler(InternalServerErrorException.class)
143-
public ResponseEntity<?> handleInternalServerErrorException(
144-
InternalServerErrorException ex, WebRequest request) {
131+
public ResponseEntity<?> handleInternalServerErrorException(InternalServerErrorException ex) {
145132
log.error("Internal Server Error", ex);
146-
// Create a more specific error message based on the exception type or cause
147133
String errorMessage = "An unexpected error occurred.";
134+
return new ResponseEntity<>(
135+
new GenericResponse<>(errorMessage), HttpStatus.INTERNAL_SERVER_ERROR);
136+
}
148137

149-
// Consider adding more details to the response for debugging
150-
Map<String, Object> errorDetails = new HashMap<>();
151-
errorDetails.put("timestamp", LocalDateTime.now());
152-
errorDetails.put("message", errorMessage);
153-
errorDetails.put("details", ex.getMessage()); // Include exception details for debugging
154-
155-
return new ResponseEntity<>(errorDetails, HttpStatus.INTERNAL_SERVER_ERROR);
138+
@ExceptionHandler(EndorsementAlreadyExistsException.class)
139+
public ResponseEntity<?> handleEndorsementAlreadyExistsException(
140+
EndorsementAlreadyExistsException ex) {
141+
log.error("EndorsementAlreadyExistsException - Error : {}", ex.getMessage());
142+
return new ResponseEntity<>(
143+
new GenericResponse<>(ex.getMessage()), HttpStatus.METHOD_NOT_ALLOWED);
156144
}
157145
}

‎skill-tree/src/main/java/com/RDS/skilltree/repositories/EndorsementRepository.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,23 @@
33
import com.RDS.skilltree.models.Endorsement;
44
import java.util.List;
55
import org.springframework.data.jpa.repository.JpaRepository;
6+
import org.springframework.data.jpa.repository.Query;
7+
import org.springframework.data.repository.query.Param;
68

79
public interface EndorsementRepository extends JpaRepository<Endorsement, Integer> {
810
List<Endorsement> findBySkillId(Integer skillId);
911

1012
List<Endorsement> findByEndorseIdAndSkillId(String endorseId, Integer skillId);
13+
14+
@Query("""
15+
SELECT (COUNT(*) > 0) AS exists
16+
FROM Endorsement e
17+
WHERE e.endorserId = :endorserId
18+
AND e.endorseId = :endorseId
19+
AND e.skill.id = :skillId
20+
""")
21+
boolean existsByEndorseIdAndEndorserIdAndSkillId(
22+
@Param("endorseId") String endorseId,
23+
@Param("endorserId") String endorserId,
24+
@Param("skillId") Integer skillId);
1125
}

‎skill-tree/src/main/java/com/RDS/skilltree/services/EndorsementServiceImplementation.java‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.RDS.skilltree.services;
22

33
import com.RDS.skilltree.dtos.RdsGetUserDetailsResDto;
4+
import com.RDS.skilltree.exceptions.EndorsementAlreadyExistsException;
45
import com.RDS.skilltree.exceptions.EndorsementNotFoundException;
56
import com.RDS.skilltree.exceptions.SelfEndorsementNotAllowedException;
67
import com.RDS.skilltree.exceptions.SkillNotFoundException;
@@ -12,6 +13,7 @@
1213
import com.RDS.skilltree.repositories.SkillRepository;
1314
import com.RDS.skilltree.repositories.UserSkillRepository;
1415
import com.RDS.skilltree.services.external.RdsService;
16+
import com.RDS.skilltree.utils.Constants.ExceptionMessages;
1517
import com.RDS.skilltree.viewmodels.CreateEndorsementViewModel;
1618
import com.RDS.skilltree.viewmodels.EndorsementViewModel;
1719
import com.RDS.skilltree.viewmodels.UpdateEndorsementViewModel;
@@ -63,7 +65,6 @@ public List<EndorsementViewModel> getAllEndorsementsBySkillId(Integer skillId) {
6365
}
6466

6567
@Override
66-
// TODO : add a check for when a endorsement is already created by a user for a particular skill.
6768
public EndorsementViewModel create(CreateEndorsementViewModel endorsementViewModel) {
6869
String message = endorsementViewModel.getMessage();
6970
Integer skillId = endorsementViewModel.getSkillId();
@@ -75,19 +76,28 @@ public EndorsementViewModel create(CreateEndorsementViewModel endorsementViewMod
7576
String endorserId = jwtDetails.getRdsUserId();
7677

7778
if (Objects.equals(endorseId, endorserId)) {
78-
log.info(
79+
log.warn(
7980
"Self endorsement not allowed, endorseId: {}, endorserId: {}", endorseId, endorserId);
80-
throw new SelfEndorsementNotAllowedException("Self endorsement not allowed");
81+
throw new SelfEndorsementNotAllowedException(ExceptionMessages.SELF_ENDORSEMENT_NOT_ALLOWED);
8182
}
8283

8384
Optional<Skill> skillDetails = skillRepository.findById(skillId);
8485

8586
if (skillDetails.isEmpty()) {
86-
log.info(String.format("Skill id: %s not found", skillId));
87-
throw new SkillNotFoundException("Skill does not exist");
87+
log.info("Skill id: {} not found", skillId);
88+
throw new SkillNotFoundException(ExceptionMessages.SKILL_NOT_FOUND);
89+
}
90+
91+
if (endorsementRepository.existsByEndorseIdAndEndorserIdAndSkillId(
92+
endorseId, endorserId, skillId)) {
93+
log.info(
94+
"Endorsement already exists for endorseId: {}, endorserId: {}, skillId: {}",
95+
endorseId,
96+
endorserId,
97+
skillId);
98+
throw new EndorsementAlreadyExistsException(ExceptionMessages.ENDORSEMENT_ALREADY_EXISTS);
8899
}
89100

90-
// Get endorse(person being endorsed) & endorser(person endorsing) details from RDS
91101
RdsGetUserDetailsResDto endorseDetails = rdsService.getUserDetails(endorseId);
92102
RdsGetUserDetailsResDto endorserDetails = rdsService.getUserDetails(endorserId);
93103

@@ -125,7 +135,7 @@ public EndorsementViewModel update(Integer endorsementId, UpdateEndorsementViewM
125135

126136
if (exitingEndorsement.isEmpty()) {
127137
log.info(String.format("Endorsement with id: %s not found", endorsementId));
128-
throw new EndorsementNotFoundException("Endorsement not found");
138+
throw new EndorsementNotFoundException(ExceptionMessages.ENDORSEMENT_NOT_FOUND);
129139
}
130140

131141
Endorsement endorsement = exitingEndorsement.get();

‎skill-tree/src/main/java/com/RDS/skilltree/utils/Constants.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package com.RDS.skilltree.utils;
2+
3+
public class Constants {
4+
private Constants() {}
5+
6+
public static final class ExceptionMessages {
7+
public static final String SELF_ENDORSEMENT_NOT_ALLOWED = "Self endorsement not allowed";
8+
public static final String SKILL_NOT_FOUND = "Skill does not exist";
9+
public static final String ENDORSEMENT_ALREADY_EXISTS = "Endorsement already exists";
10+
public static final String ENDORSEMENT_NOT_FOUND = "Endorsement not found";
11+
}
12+
}

‎skill-tree/src/main/java/com/RDS/skilltree/utils/GenericResponse.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,8 @@
1414
public class GenericResponse<T> {
1515
private T data;
1616
private String message;
17+
18+
public GenericResponse(String message) {
19+
this.message = message;
20+
}
1721
}

‎skill-tree/src/main/resources/application-production.properties‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
cookieName=rds-session-v2
1+
cookieName=rds-session-v2
2+
logging.level.root=ERROR

‎skill-tree/src/main/resources/application-staging.properties‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
cookieName=rds-session-v2-staging
1+
cookieName=rds-session-v2-staging
2+
logging.level.root=WARN

‎skill-tree/src/main/resources/application-test.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@ cookieName=rds-session-v2-development
22
test.db.mysql-image=mysql:8.1.0
33
spring.flyway.enabled=true
44
spring.flyway.locations=classpath:db/migrations
5+
logging.level.root=WARN

‎skill-tree/src/main/resources/logback-test.xml‎

Lines changed: 0 additions & 14 deletions
This file was deleted.

0 commit comments

Comments
 (0)