Skip to content

Commit 8f9d79e

Browse files
rostilosrostilos
committed
feat: Update validation constraints for analysis type and enhance error handling in request processing
1 parent 40ec362 commit 8f9d79e

6 files changed

Lines changed: 71 additions & 8 deletions

File tree

‎java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/dto/request/processor/BranchProcessRequest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public class BranchProcessRequest implements AnalysisProcessRequest {
1414
@NotBlank(message = "Commit hash is required")
1515
public String commitHash;
1616

17-
@NotBlank(message = "Specify analysis type")
17+
@NotNull(message = "Specify analysis type")
1818
public AnalysisType analysisType;
1919

2020
/**

‎java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/dto/request/processor/PrProcessRequest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public class PrProcessRequest implements AnalysisProcessRequest {
2323
@NotBlank(message = "Commit hash is required")
2424
public String commitHash;
2525

26-
@NotBlank(message = "Specify analysis type")
26+
@NotNull(message = "Specify analysis type")
2727
public AnalysisType analysisType;
2828

2929
/**

‎java-ecosystem/libs/analysis-engine/src/test/java/org/rostilos/codecrow/analysisengine/dto/request/processor/BranchProcessRequestTest.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package org.rostilos.codecrow.analysisengine.dto.request.processor;
22

3+
import jakarta.validation.constraints.NotBlank;
4+
import jakarta.validation.constraints.NotNull;
35
import org.junit.jupiter.api.DisplayName;
46
import org.junit.jupiter.api.Nested;
57
import org.junit.jupiter.api.Test;
@@ -114,4 +116,18 @@ void shouldImplementAnalysisProcessRequest() {
114116
assertThat(request).isInstanceOf(AnalysisProcessRequest.class);
115117
}
116118
}
119+
120+
@Nested
121+
@DisplayName("Validation")
122+
class ValidationTests {
123+
124+
@Test
125+
@DisplayName("should use enum-compatible constraint for analysis type")
126+
void shouldUseEnumCompatibleConstraintForAnalysisType() throws NoSuchFieldException {
127+
var analysisTypeField = BranchProcessRequest.class.getField("analysisType");
128+
129+
assertThat(analysisTypeField.getAnnotation(NotNull.class)).isNotNull();
130+
assertThat(analysisTypeField.getAnnotation(NotBlank.class)).isNull();
131+
}
132+
}
117133
}

‎java-ecosystem/libs/analysis-engine/src/test/java/org/rostilos/codecrow/analysisengine/dto/request/processor/PrProcessRequestTest.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package org.rostilos.codecrow.analysisengine.dto.request.processor;
22

3+
import jakarta.validation.constraints.NotBlank;
4+
import jakarta.validation.constraints.NotNull;
35
import org.junit.jupiter.api.DisplayName;
46
import org.junit.jupiter.api.Nested;
57
import org.junit.jupiter.api.Test;
@@ -109,4 +111,18 @@ void shouldImplementAnalysisProcessRequest() {
109111
assertThat(request).isInstanceOf(AnalysisProcessRequest.class);
110112
}
111113
}
114+
115+
@Nested
116+
@DisplayName("Validation")
117+
class ValidationTests {
118+
119+
@Test
120+
@DisplayName("should use enum-compatible constraint for analysis type")
121+
void shouldUseEnumCompatibleConstraintForAnalysisType() throws NoSuchFieldException {
122+
var analysisTypeField = PrProcessRequest.class.getField("analysisType");
123+
124+
assertThat(analysisTypeField.getAnnotation(NotNull.class)).isNotNull();
125+
assertThat(analysisTypeField.getAnnotation(NotBlank.class)).isNull();
126+
}
127+
}
112128
}

‎java-ecosystem/services/pipeline-agent/src/it/java/org/rostilos/codecrow/pipelineagent/PipelineAgentSecurityIT.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,17 +111,14 @@ void processingEndpoint_validAuth_notUnauthorized() {
111111
projectAuthRequest(projectId)
112112
.body("""
113113
{
114-
"projectId": %d,
115-
"pullRequestId": 1,
116-
"sourceBranch": "feature",
117-
"targetBranch": "main"
114+
"projectId": %d
118115
}
119116
""".formatted(projectId))
120117
.when()
121118
.post("/api/processing/webhook/pr")
122119
.then()
123-
// Should pass authentication; may still fail for other reasons
124-
.statusCode(not(401));
120+
// Authentication passed; the intentionally incomplete body fails validation.
121+
.statusCode(400);
125122
}
126123
}
127124
}

‎java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/generic/controller/ProviderPipelineActionController.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import jakarta.servlet.http.HttpServletResponse;
44
import jakarta.validation.Valid;
55
import org.rostilos.codecrow.core.dto.project.ProjectDTO;
6+
import org.rostilos.codecrow.core.model.codeanalysis.AnalysisType;
67
import org.rostilos.codecrow.core.model.job.Job;
78
import org.rostilos.codecrow.analysisengine.dto.request.processor.AnalysisProcessRequest;
89
import org.rostilos.codecrow.analysisengine.dto.request.processor.BranchProcessRequest;
@@ -149,6 +150,9 @@ public ResponseEntity<StreamingResponseBody> handleBranchWebhook(
149150
} catch (IOException e) {
150151
log.error("Failed to parse request or read archive", e);
151152
return createErrorResponse(HttpServletResponse.SC_BAD_REQUEST, "invalid_request", e.getMessage());
153+
} catch (IllegalArgumentException e) {
154+
log.error("Invalid webhook request: {}", e.getMessage());
155+
return createErrorResponse(HttpServletResponse.SC_BAD_REQUEST, "invalid_request", e.getMessage());
152156
}
153157
}
154158

@@ -164,7 +168,9 @@ private ResponseEntity<StreamingResponseBody> processWebhookWithJob(
164168
JobAwareWebhookProcessor webhookProcessorFunc
165169
) {
166170
try {
171+
validateProjectIdPresent(payload);
167172
validateAuthentication(authenticationPrincipal, payload);
173+
validateProcessPayload(payload);
168174

169175
if (job != null) {
170176
pipelineJobService.getJobService().startJob(job);
@@ -269,6 +275,12 @@ private ResponseEntity<StreamingResponseBody> processWebhookWithJob(
269275
}
270276
}
271277

278+
private void validateProjectIdPresent(AnalysisProcessRequest payload) {
279+
if (payload == null || payload.getProjectId() == null) {
280+
throw new IllegalArgumentException("Project ID is required");
281+
}
282+
}
283+
272284
private void validateAuthentication(ProjectDTO authenticationPrincipal, AnalysisProcessRequest payload) {
273285
if (!payload.getProjectId().equals(authenticationPrincipal.id())) {
274286
log.warn("Request body projectId {} does not match JWT projectId {}",
@@ -277,6 +289,27 @@ private void validateAuthentication(ProjectDTO authenticationPrincipal, Analysis
277289
}
278290
}
279291

292+
private void validateProcessPayload(AnalysisProcessRequest payload) {
293+
requireNotBlank(payload.getCommitHash(), "Commit hash is required");
294+
requireNotBlank(payload.getTargetBranchName(), "Target branch name is required");
295+
if (payload.getAnalysisType() == null) {
296+
throw new IllegalArgumentException("Analysis type is required");
297+
}
298+
299+
if (payload instanceof PrProcessRequest prPayload) {
300+
requireNotBlank(prPayload.getSourceBranchName(), "Source branch name is required");
301+
if (prPayload.getAnalysisType() == AnalysisType.PR_REVIEW && prPayload.getPullRequestId() == null) {
302+
throw new IllegalArgumentException("Pull Request ID is required for PR_REVIEW analysis type");
303+
}
304+
}
305+
}
306+
307+
private void requireNotBlank(String value, String message) {
308+
if (value == null || value.isBlank()) {
309+
throw new IllegalArgumentException(message);
310+
}
311+
}
312+
280313
private PipelineActionProcessor.EventConsumer createEventConsumer(LinkedBlockingQueue<String> queue) {
281314
return event -> {
282315
try {
@@ -371,6 +404,7 @@ private void enqueueEOF(LinkedBlockingQueue<String> queue) {
371404

372405
private ResponseEntity<StreamingResponseBody> createErrorResponse(int status, String error, String message) {
373406
return ResponseEntity.status(status)
407+
.contentType(MediaType.APPLICATION_JSON)
374408
.body(outputStream -> {
375409
PrintWriter w = new PrintWriter(outputStream, true, StandardCharsets.UTF_8);
376410
try {

0 commit comments

Comments
 (0)