Add content-type header to /vtrack and /status endpoints #1399

javaAndScriptDeveloper · 2021-08-02T13:40:11Z

Added content-type header to /vtrack and /status endpoints, checked code via tests

SerhiiNahornyi

Pls, add unit tests

SerhiiNahornyi · 2021-08-03T09:40:44Z

src/main/java/org/prebid/server/analytics/pubstack/model/EventType.java

+    auction, cookiesync, amp, setuid,
+    video


Redundant change

SerhiiNahornyi · 2021-08-03T09:41:08Z

testing/appnexus.sh

@@ -0,0 +1,103 @@
+#!/bin/sh


Pls remove this file

SerhiiNahornyi · 2021-08-03T09:41:23Z

src/main/java/org/prebid/server/vertx/ContextRunner.java

@@ -53,7 +53,6 @@ public ContextRunner(Vertx vertx, long timeoutMs) {
    private <T> void runOnContext(Supplier<Context> contextFactory, int times, Handler<Promise<T>> action) {
        final CountDownLatch completionLatch = new CountDownLatch(times);
        final AtomicBoolean actionFailed = new AtomicBoolean(false);
-


Redundant change

rpanchyk · 2021-08-03T12:49:50Z

src/main/java/org/prebid/server/handler/StatusHandler.java

@@ -27,6 +28,7 @@ public StatusHandler(List<HealthChecker> healthCheckers, JacksonMapper mapper) {

    @Override
    public void handle(RoutingContext routingContext) {
+        routingContext.response().putHeader(HttpUtil.CONTENT_TYPE_HEADER, HttpHeaderValues.APPLICATION_JSON);


This can be done in lambda below.

idk why, but having this done in lambda crashes test

rpanchyk · 2021-08-03T12:49:57Z

src/main/java/org/prebid/server/handler/VtrackHandler.java

@@ -189,6 +190,7 @@ private static void respondWithServerError(RoutingContext routingContext, String
    }

    private static void respondWith(RoutingContext routingContext, HttpResponseStatus status, String body) {
+        routingContext.response().putHeader(HttpUtil.CONTENT_TYPE_HEADER, HttpHeaderValues.APPLICATION_JSON);


This can be done in lambda below.

idk why, but having this done in lambda crashes test

rpanchyk · 2021-08-04T13:33:32Z

src/test/java/org/prebid/server/handler/StatusHandlerTest.java

@@ -49,20 +51,24 @@ public void shouldRespondHttp200OkWithExpectedBody() throws JsonProcessingExcept
        final ZonedDateTime testTime = ZonedDateTime.now(Clock.systemUTC());
        statusHandler = new StatusHandler(Arrays.asList(healthCheck, healthCheck, healthCheck), jacksonMapper);

-        given(routingContext.response()).willReturn(httpResponse);
+        //given(routingContext.response()).willReturn(httpResponse);


rpanchyk · 2021-08-04T13:33:59Z

src/test/java/org/prebid/server/handler/StatusHandlerTest.java

        // then
+


Redundant line.
PS. Please, keep your code clean.

rpanchyk · 2021-08-04T13:36:40Z

src/test/java/org/prebid/server/handler/VtrackHandlerTest.java

@@ -94,10 +98,25 @@ public void shouldRespondWithBadRequestWhenAccountParameterIsMissing() {
        verify(httpResponse).end(eq("Account 'a' is required query parameter and can't be empty"));
    }

+    @Test
+    public void contentTypeHeaderAdded() {


Test method must be in style: shouldRespondWithExpectedHeaders.
Please see existing tests as example.

rpanchyk · 2021-08-04T13:37:17Z

src/test/java/org/prebid/server/handler/VtrackHandlerTest.java

+    @Test
+    public void contentTypeHeaderAdded() {
+        // given
+        given(httpResponse.putHeader(HttpUtil.CONTENT_TYPE_HEADER, HttpHeaderValues.APPLICATION_JSON))


rpanchyk · 2021-08-04T13:42:26Z

src/test/java/org/prebid/server/handler/VtrackHandlerTest.java

    @Test
    public void shouldRespondWithBadRequestWhenBodyIsEmpty() {
        // given
        given(routingContext.getBody()).willReturn(null);
+        given(httpResponse.putHeader(HttpUtil.CONTENT_TYPE_HEADER, HttpHeaderValues.APPLICATION_JSON))


You should create @Before method to setup conditions for each test.

rpanchyk · 2021-08-04T13:44:18Z

src/test/java/org/prebid/server/handler/VtrackHandlerTest.java

@@ -129,7 +150,10 @@ public void shouldRespondWithBadRequestWhenBidIdIsMissing() throws JsonProcessin
        // given
        given(routingContext.getBody())
                .willReturn(givenVtrackRequest(identity()));
-
+        given(httpResponse.putHeader(HttpUtil.CONTENT_TYPE_HEADER, HttpHeaderValues.APPLICATION_JSON))


Why you check this here?
Test is about not parsable body.

Consider to change all below tests.

rpanchyk · 2021-08-09T09:31:27Z

src/test/java/org/prebid/server/handler/StatusHandlerTest.java

@@ -78,4 +82,23 @@ public void shouldRespondWithNoContentWhenMessageWasNotDefined() {
        // then
        verify(httpResponse).setStatusCode(eq(204));
    }
+
+    @Test
+    public void shouldRespondWithContentTypeHeaders() throws JsonProcessingException {


Redundant throws.

rpanchyk · 2021-08-09T09:32:22Z

src/test/java/org/prebid/server/handler/StatusHandlerTest.java

+    @Test
+    public void shouldRespondWithContentTypeHeaders() throws JsonProcessingException {
+        // given
+        statusHandler = new StatusHandler(Arrays.asList(healthCheck), jacksonMapper);


Use singletonList (with static method import) instead.

rpanchyk · 2021-08-09T14:07:20Z

src/test/java/org/prebid/server/handler/StatusHandlerTest.java

+        given(healthCheck.status()).willReturn(StatusResponse.of("healthCheckStatus", null));
+
+        given(routingContext.response()).willReturn(httpResponse);
+        given(httpResponse.putHeader(HttpUtil.CONTENT_TYPE_HEADER, HttpHeaderValues.APPLICATION_JSON))


This is not required since the last point of test is to verify putHeader method.

rpanchyk · 2021-08-09T14:08:35Z

src/test/java/org/prebid/server/handler/VtrackHandlerTest.java

@@ -165,6 +178,9 @@ public void shouldRespondWithInternalServerErrorWhenFetchingAccountFails() throw
        given(applicationSettings.getAccountById(any(), any()))
                .willReturn(Future.failedFuture("error"));

+        given(httpResponse.putHeader(HttpUtil.CONTENT_TYPE_HEADER, HttpHeaderValues.APPLICATION_JSON))


rpanchyk · 2021-08-09T14:11:46Z

src/test/java/org/prebid/server/handler/StatusHandlerTest.java

@@ -50,6 +52,8 @@ public void shouldRespondHttp200OkWithExpectedBody() throws JsonProcessingExcept
        statusHandler = new StatusHandler(Arrays.asList(healthCheck, healthCheck, healthCheck), jacksonMapper);

        given(routingContext.response()).willReturn(httpResponse);
+        given(httpResponse.putHeader(HttpUtil.CONTENT_TYPE_HEADER, HttpHeaderValues.APPLICATION_JSON))


Pls replace with:

given(httpResponse.putHeader(any(CharSequence.class), any(AsciiString.class))).willReturn(httpResponse);

since mocking conditions can be more generic.

Adding headers to vtrack and status

4a47351

rpanchyk changed the title ~~Adding headers to vtrack and status~~ Add content-type header to /vtrack and /status endpoints Aug 2, 2021

rpanchyk self-requested a review August 2, 2021 14:09

This commit added headers to /vtrack and /status

375223b

SerhiiNahornyi suggested changes Aug 3, 2021

View reviewed changes

rpanchyk suggested changes Aug 3, 2021

View reviewed changes

javaAndScriptDeveloper added 4 commits August 3, 2021 17:09

Working

d65aeba

Continue working progress

1a93c52

Updated tests and moved adding headers to lambdas

3ce34b5

Moved fields in event type

51c836c

rpanchyk suggested changes Aug 4, 2021

View reviewed changes

javaAndScriptDeveloper added 3 commits August 6, 2021 03:10

Fixed tests

eb65a4f

Fixed tests

4d7c51c

Fixed imports

71db88d

rpanchyk suggested changes Aug 9, 2021

View reviewed changes

fixed tests

8e8b6c2

rpanchyk approved these changes Aug 10, 2021

View reviewed changes

rpanchyk merged commit 1a44239 into master Aug 10, 2021

rpanchyk deleted the l_branch branch August 10, 2021 08:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add content-type header to /vtrack and /status endpoints #1399

Add content-type header to /vtrack and /status endpoints #1399

javaAndScriptDeveloper commented Aug 2, 2021 •

edited

Loading

SerhiiNahornyi left a comment

SerhiiNahornyi Aug 3, 2021

SerhiiNahornyi Aug 3, 2021

SerhiiNahornyi Aug 3, 2021

rpanchyk Aug 3, 2021

javaAndScriptDeveloper Aug 3, 2021

rpanchyk Aug 3, 2021

javaAndScriptDeveloper Aug 3, 2021

rpanchyk Aug 4, 2021

rpanchyk Aug 4, 2021

rpanchyk Aug 4, 2021

rpanchyk Aug 4, 2021

rpanchyk Aug 4, 2021

rpanchyk Aug 4, 2021

rpanchyk Aug 9, 2021

rpanchyk Aug 9, 2021

rpanchyk Aug 9, 2021

rpanchyk Aug 9, 2021

rpanchyk Aug 9, 2021

Add content-type header to /vtrack and /status endpoints #1399

Add content-type header to /vtrack and /status endpoints #1399

Conversation

javaAndScriptDeveloper commented Aug 2, 2021 • edited Loading

SerhiiNahornyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javaAndScriptDeveloper commented Aug 2, 2021 •

edited

Loading