2.X.X deprecates UUIDParam, SizeParam, LocalDateParam, InstantParam, DurationParam, DateTimeParam, BooleanParam and does not give clear alternatives

[mikeholler]

I'm trying to upgrade Dropwizard from 1.3.23 to 2.0.10 and am running into some issues around the use of parameters. Specifically, I see that this commit deprecated a number of *Param classes.

Some of these, like IntParam are straightfoward. Using OptionalInt as a replacement makes sense. However, taking non-primitive parameters as an example, Optional<UUID> is NOT a sufficient replacement for UUIDParam. Using it results in me getting a 404 when using a non-UUID as input (where I previously got 400 using UUIDParam).

Additionally, and maybe this is a separate issue, I found the compatibility-preserving use of UUIDParam is strange. When diving into the source code, I found this resource method[1]:

        @GET
        @Path("/uuid")
        public String getUUID(@QueryParam("uuid") Optional<UUIDParam> uuid) {
            return uuid.orElse(new UUIDParam("d5672fa8-326b-40f6-bf71-d9dacf44bcdc")).get().toString();
        }

This construct seems an extremely clunky way to manage this. In 1.3.X, I could write code like this instead:

        @GET
        @Path("/uuid")
        public String getUUID(@Nullable @QueryParam("uuid") UUIDParam uuid) {
            if (uuid != null) {
                return uuid.get()
            } else {
                return "d5672fa8-326b-40f6-bf71-d9dacf44bcdc"
            }
        }

You can see in the above that there is a lot of unnecessary conversion going on (we construct a UUID via UUIDParam and validate it, even though we just want to return a String). I use Kotlin primarily, my preferred example above would be even simpler:

uuid.get()?.toString ?: "d5672fa8-326b-40f6-bf71-d9dacf44bcdc"

What I'd prefer overall is something that worked like this:

        @GET
        @Path("/uuid")
        public String getUUID(@Nullable @QueryParam("uuid") Optional<UUID> uuid) {
            return uuid.map { it.toString() }.orElse("d5672fa8-326b-40f6-bf71-d9dacf44bcdc")
        }

More than anything, really, I just want some guidance for the right way to handle various param types, especially in light of the deprecation. The documentation is light on this (I found the MR saying AbstractParam and child classes will be removed, but nothing saying why, and no code showing what would replace the more complicated use-cases like UUIDParam above). I would love to see this improved. Until then, I'm hesitant to upgrade to 2.X.X. Param handling is still confusing in 1.X.X, but at least I won't have to worry about behavior changes.

[joschi]

The code in OptionalQueryParamResourceTest is not very nice.

If you want to have it a little more idiomatic, you can write:

@GET
@Path("/uuid")
public String getUUID(@QueryParam("uuid") Optional<UUIDParam> uuid) {
  return uuid.map(UUIDParam::get).map(UUID::toString).orElse("d5672fa8-326b-40f6-bf71-d9dacf44bcdc");
}

If Optional<java.util.UUID> works for you (java.util.UUID behaves slightly different than UUIDParam), you could also write:

@GET
@Path("/uuid")
public String getUUID(@QueryParam("uuid") Optional<UUID> uuid) {
  return uuid.map(UUID::toString).orElse("d5672fa8-326b-40f6-bf71-d9dacf44bcdc");
}

[mikeholler]

@joschi thanks for the snippet. I understand that I can do that. I am seeing a substantial behavioral difference that is preventing my existing code from being compatible. Particularly, this suite of tests used to pass in 1.3.23 and now fails in 2.0.10. It's like this for the *Param types built into Dropwizard as well, but I wanted to show this use case because:

1. The use-case for migrating AbstractParam subclasses is not described in the Upgrade Notes for Dropwizard 2.0.X document.
2. For us this is a breaking compatibility change.
3. I don't understand how we should move forward, or what the guidance is.

import io.dropwizard.jersey.params.AbstractParam
import io.dropwizard.testing.junit5.DropwizardExtensionsSupport
import io.dropwizard.testing.junit5.ResourceExtension
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.ValueSource
import org.threeten.extra.Interval
import strikt.api.expectThat
import strikt.api.expectThrows
import strikt.assertions.isEqualTo
import strikt.assertions.isNull
import javax.ws.rs.GET
import javax.ws.rs.Path
import javax.ws.rs.QueryParam

@ExtendWith(DropwizardExtensionsSupport::class)
class Dropwizard1To2AbstractParamImplCompatibilityTest {

    class IntervalParam @JvmOverloads constructor(
        input: String?,
        parameterName: String = "Parameter"
    ) : AbstractParam<Interval>(input, parameterName) {
        override fun errorMessage(e: Exception): String {
            return "%s must be in a ISO-8601 format."
        }

        @Throws(Exception::class)
        override fun parse(input: String?): Interval? {
            return input?.let { Interval.parse(input) }
        }
    }

    private val params: MutableList<IntervalParam?> = mutableListOf()

    @BeforeEach
    fun setUp() {
        params.clear()
    }

    @Path("/")
    inner class ExampleResource {
        @GET
        @Path("interval")
        fun intervalNotNull(
            @QueryParam("interval")
            interval: IntervalParam
        ) {
            params.add(interval)
        }

        @GET
        @Path("interval-nullable")
        fun intervalNullable(
            @QueryParam("interval")
            interval: IntervalParam
        ) {
            params.add(interval)
        }
    }

    private var resources: ResourceExtension = ResourceExtension.builder().apply {
        addResource(ExampleResource())
    }.build()

    @ValueSource(strings = [
        "2019-01-01T00:00:00Z/2020-01-01T00:00:00Z",
        "2019-01-01T00:00:00.123456789Z/2020-01-01T00:00:00.123456789Z",
        "2019-01-01T00:00:00Z/2020-01-01T00:00:00.1Z",
        "2019-01-01T00:00:00Z/2020-01-01T00:00:00.12Z",
        "2019-01-01T00:00:00Z/2020-01-01T00:00:00.123Z",
        "2019-01-01T00:00:00Z/2020-01-01T00:00:00.1234Z",
        "2019-01-01T00:00:00Z/2020-01-01T00:00:00.12345Z",
        "2019-01-01T00:00:00Z/2020-01-01T00:00:00.123456Z",
        "2019-01-01T00:00:00Z/2020-01-01T00:00:00.1234567Z",
        "2019-01-01T00:00:00Z/2020-01-01T00:00:00.12345678Z",
        "2019-01-01T00:00:00Z/2020-01-01T00:00:00.123456789Z",
        "2019-01-01T00:00:00.1Z/2020-01-01T00:00:00Z",
        "2019-01-01T00:00:00.12Z/2020-01-01T00:00:00Z",
        "2019-01-01T00:00:00.123Z/2020-01-01T00:00:00Z",
        "2019-01-01T00:00:00.1234Z/2020-01-01T00:00:00Z",
        "2019-01-01T00:00:00.12345Z/2020-01-01T00:00:00Z",
        "2019-01-01T00:00:00.123456Z/2020-01-01T00:00:00Z",
        "2019-01-01T00:00:00.1234567Z/2020-01-01T00:00:00Z",
        "2019-01-01T00:00:00.12345678Z/2020-01-01T00:00:00Z",
        "2019-01-01T00:00:00.123456789Z/2020-01-01T00:00:00Z"
    ])
    @ParameterizedTest
    fun `providing a valid interval works`(value: String) {
        val response = resources.target("/interval").queryParam("interval", value).request().get()
        expectThat(response.status).isEqualTo(204)
        expectThat(params.size).isEqualTo(1)
        expectThat(params.first()!!.get()).isEqualTo(Interval.parse(value))
    }

    @ValueSource(strings = [
        "2019-01-01T00:00:00/2020-01-01T00:00:00",
        "2019-01-01T00:00:00Z/2020-01-01",
        "2019-01-01/2020-01-01"
    ])
    @ParameterizedTest
    fun `providing a invalid interval fails with 400`(value: String) {
        val response = resources.target("/interval").queryParam("interval", value).request().get()
        expectThat(response.status).isEqualTo(400)
    }

    @Test
    fun `missing interval when interval param is non-nullable`() {
        // WARNING: Take care to never use this approach, and always supply
        // either @DefaultValue and a non-nullable, or a nullable query param.
        expectThrows<Exception> {
            resources.target("/interval").request().get()
        }
    }

    @Test
    fun `missing interval when interval param is nullable`() {
        val response = resources.target("/interval-nullable").request().get()
        expectThat(response.status).isEqualTo(204)
        expectThat(params.size).isEqualTo(1)
        expectThat(params.first()).isNull()
    }
}

The specific test failures from this suite are the last two tests:

• missing interval when interval param is non-nullable used to fail in the way shown in the test case, and now fails with NullPointerException because AbstractParam.toString() is implemented with this line: return value.toString() so when value is null and you call .toString() on AbstractParam it will fail.
• missing interval when interval param is nullable used to succeed, and now fails with a 400 status code and a message saying <interval> must be in IDO-8601 format.. This error message is occurring because parse is being called with input = null which did not happen in Dropwizard 1.3.27. In that version, there was no invocation of the parse method at all.

These changes have serious implications on our large API, as we will have to manually update many parameters and test cases. I'm not really sure what the right solution is here, and would appreciate your support. I wouldn't be surprised if others were having problems as well.

P.S. Kotlin is my primary language nowadays, but if you need me to convert anything to Java or are confused about my code please let me know.

[mikeholler]

I've also taken the time to put together the same test for UUIDParam (see below). In this case, I would expect a deprecated feature to work as it did before, but again the below test suite passes in 1.3.72 and fails in 2.0.10 on the last two tests.

import io.dropwizard.jersey.params.UUIDParam
import io.dropwizard.testing.junit5.DropwizardExtensionsSupport
import io.dropwizard.testing.junit5.ResourceExtension
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.ValueSource
import strikt.api.expectThat
import strikt.api.expectThrows
import strikt.assertions.isEqualTo
import strikt.assertions.isNull
import java.util.UUID
import javax.ws.rs.GET
import javax.ws.rs.Path
import javax.ws.rs.QueryParam

@ExtendWith(DropwizardExtensionsSupport::class)
class Dropwizard1To2AbstractParamImplCompatibilityTest {

    private val params: MutableList<UUIDParam?> = mutableListOf()

    @BeforeEach
    fun setUp() {
        params.clear()
    }

    @Path("/")
    inner class ExampleResource {
        @GET
        @Path("uuid")
        fun uuidNotNull(
            @QueryParam("uuid")
            uuid: UUIDParam
        ) {
            params.add(uuid)
        }

        @GET
        @Path("uuid-nullable")
        fun uuidNullable(
            @QueryParam("uuid")
            uuid: UUIDParam?
        ) {
            params.add(uuid)
        }
    }

    private var resources: ResourceExtension = ResourceExtension.builder().apply {
        addResource(ExampleResource())
    }.build()

    @Test
    fun `providing a valid uuid works`() {
        val myUUID = UUID.fromString("653ac4a4-6e72-4faf-a054-1ca232c76b0d")
        val response = resources.target("/uuid").queryParam("uuid", myUUID.toString()).request().get()
        expectThat(response.status).isEqualTo(204)
        expectThat(params.size).isEqualTo(1)
        expectThat(params.first()!!.get()).isEqualTo(myUUID)
    }

    @ValueSource(strings = [
        "653ac4a4_6e72_4faf_a054_1ca232c76b0d",
        "653ac4a4 6e72 4faf a054 1ca232c76b0d"
    ])
    @ParameterizedTest
    fun `providing a invalid uuid fails with 400`(value: String) {
        val response = resources.target("/uuid").queryParam("uuid", value).request().get()
        expectThat(response.status).isEqualTo(400)
    }

    @Test
    fun `missing uuid when uuid param is non-nullable`() {
        // WARNING: Take care to never use this approach, and always supply
        // either @DefaultValue and a non-nullable, or a nullable query param.
        expectThrows<Exception> {
            resources.target("/uuid").request().get()
        }
    }

    @Test
    fun `missing uuid when uuid param is nullable`() {
        val response = resources.target("/uuid-nullable").request().get()
        expectThat(response.status).isEqualTo(204)
        expectThat(params.size).isEqualTo(1)
        expectThat(params.first()).isNull()
    }
}

[sleberknight]

We also have experienced similar problems but with regular old long parameters, e.g. OptionalLong and Optional<Long> which behave completely differently. (The same goes for OptionalInt and Optional<Integer> etc). Here is a sample endpoint using optionals:

    @GET
    @Path("/optionalsTest/{param1}/{param2}")
    @Produces(MediaType.APPLICATION_JSON)
    public Response optionalsTest(@PathParam("param1") OptionalLong param1,
                                  @PathParam("param2") Optional<Long> param2) {

        var entity = Map.of(
                "param1", param1.getAsLong(),
                "param2", param2.get()
        );

        return Response.ok(entity).build();
    }

Observed behavior:

If you do GET /optionalsTest/42/84, you get a 200 response with the expected response entity.

If you do GET /optionalsTest/abcd/84, you get a 500 response.

If you do GET /optionalsTest/42/abcd, you get a 404 response.

So, when using OptionalLong and supplying a non-numeric value, you get a 500 which is very confusing behavior. It should be a 400 as LongParam would do.

But, when using Optional<Long> and supplying a non-numeric value, you get a 404 which is maybe even more confusing than getting a 400 or 500.

Using LongParam always gives a 400 response and gives a nice error message as well. For this sample endpoint:

    @GET
    @Path("longParamTest/{param1}")
    @Produces(MediaType.APPLICATION_JSON)
    public Response optionalsTest(@PathParam("param1") LongParam param1) {

        var entity = Map.of("param1", param1.get());

        return Response.ok(entity).build();
    }

If you do GET /longParamTest/abcd you get a 400 response with the following body:

{
    code: 400,
    message: "path param param1 is not a number."
}

So while I have no problem using OptionalLong or Optional<Long> etc. (other than both IntelliJ and Sonar complaining about using optionals for method arguments), the completely different behavior (i.e. the 500 for OptionalLong and 400 for Optional<Long) is confusing and not as clear as the behavior LongParam exhibits (as well as all the other XxxParam classes).

[github-actions]

This issue is stale because it has been open 90 days with no activity. Remove the "stale" label or comment or this will be closed in 14 days.