Git Product home page Git Product logo

conjure-java's Introduction

Autorelease

Conjure-Java License

CLI to generate Java POJOs and interfaces from Conjure API definitions.

Note that all Java APIs in this repository (including conjure-java-core) are considered "internal" in the sense that they may change at any time and without prior warning or deprecation notice. These packages are published for convenience, but users must assume responsibility for adapting their code when dependencies change.

Usage

The recommended way to use conjure-java is via a build tool like gradle-conjure. However, if you don't want to use gradle-conjure, there is also an executable which conforms to RFC 002, published on maven central.

Usage: conjure-java generate [...options] <input> <output>

Generate Java bindings for a Conjure API
    <input>      Path to the input IR file
    <output>     Output directory for generated source

Options:
    --objects    Generate POJOs for Conjure type definitions
    --jersey     Generate jax-rs annotated interfaces for client or server-usage
    --undertow   Generate undertow handlers and interfaces for server-usage
    --requireNotNullAuthAndBodyParams
                 Generate @NotNull annotations for AuthHeaders and request body params
    --undertowServicePrefixes
                 Generate service interfaces for Undertow with class names prefixed 'Undertow'
    --undertowListenableFutures
                 Generate Undertow services which return Guava ListenableFuture for asynchronous processing
    --useImmutableBytes
                 Generate binary fields using the immutable 'Bytes' type instead of 'ByteBuffer'
    --strictObjects
                 Generate POJOs that by default will fail to deserialize unknown fields
    --nonNullCollections
                 Generate POJOs that by default will fail to deserialize collections with null values
    --useStagedBuilders
                 Generates compile-time safe builders to ensure all attributes are set, except those of type list,
                 set, map, or optional. If --useStrictStagedBuilders is set, this option is ignored.
    --useStrictStagedBuilders
                 Generates compile-time safe builders to ensure all attributes are set.
    --jakartaPackages
                 Generates jax-rs annotated interfaces which use the newer 'jakarta` packages instead of the
                 legacy 'javax' packages.
    --externalFallbackTypes
                 Java external type imports are generated using their fallback type.

Known Tag Values

Endpoint Tags

  • incubating: Describes an endpoint as incubating and likely to change. These endpoints are generated with an @Incubating annotation.
  • server-request-context: Opt into an additional RequestContext parameter in conjure-undertow interfaces, which allows request metadata to be read, and additional arguments to be associated with the request log.
  • server-async: Opt into asynchronous request processing in conjure-undertow. The generated interface returns a ListenableFuture of the defined return type, allowing processing to occur in the background without blocking the request thread.
  • deprecated-for-removal: For endpoint definitions that are marked deprecated, indicates that the endpoint will be removed in a future release. This has the effect of Java code being generated with an @Deprecated(forRemoval = true) annotation on the endpoint method in the Dialogue service interface.

Endpoint Argument Tags

  • server-squelch: Opts out of attaching safe non-body parameters to the request. By default, all known log-safe non-body inputs are included.

The following argument tags are deprecated, replaced by safety declarations.

  • safe: Annotates parameters as @Safe to log using safe-logging annotations. Implementations may add this data to the request log.
  • unsafe: Annotates parameters as @Unsafe to log using safe-logging annotations. Implementations may add this data to the request log.

Feature Flags

Conjure-java supports feature flags to enable additional opt-in features. To enable features provided by a feature flag, simply supply the feature flag as an additional parameter when executing conjure-java. Available feature flags can be found in the Options class.

Example generated objects

Conjure-java objects are always immutable and thread-safe. Fields are never null; instead, Java 8 Optionals are used. JSON serialization is handled using Jackson annotations.

  • Conjure object: ManyFieldExample

    Objects can only be instantiated using the builder pattern:

    ManyFieldExample example = ManyFieldExample.builder()
            .string("foo")
            .integer(123)
            .optionalItem("bar")
            .addAllItems(iterable)
            .build();

    Or using Jackson via conjure-jackson-serialization:

    ObjectMapper mapper = ObjectMappers.newServerObjectMapper();
    ManyFieldExample fromJson = mapper.readValue("{\"string\":\"foo\", ...}", ManyFieldExample.class);
  • Conjure union: UnionTypeExample

    Union types can be one of a few variants. To interact with a union value, users should use the .accept method and define a Visitor that handles each of the possible variants, including the possibility of an unknown variant.

    Foo output = unionTypeExample.accept(new Visitor<Foo>() {
    
        public Foo visitStringExample(StringExample value) {
            // your logic here!
        }
    
        public Foo visitSet(Set<String> value) {}
    
        // ...
    
        public Foo visitUnknown(String unknownType) {}
    
    });

    Visitors may seem clunky in Java, but they have the upside of compile-time assurance that you've handled all the possible variants. If you upgrade an API dependency and the API author added a new variant, the Java compiler will force you to explicitly deal with this new variant. We intentionally avoid switch statements and instanceof checks for this exact reason.

    There is also a more concise visitor builders pattern that can be used to create a visitor:

    Foo output = unionTypeExample.accept(Visitor.<Foo>builder()
        .stringExample(value -> ...)
        .set(value -> ...)
        .throwOnUnknown()
        .build());
    });
  • Conjure enum: EnumExample

    Enums are subtly different from regular Java enums because they tolerate deserializing unknown values. This is important because it ensures server authors can add new variants to an enum without causing runtime errors for consumers that use older API jars.

    EnumExample one = EnumExample.ONE;
    System.out.println(one); // prints: 'ONE'
    
    EnumExample fromJson = mapper.readValue("\"XYZ\"", EnumExample.class);
    System.out.println(fromJson); // prints: 'XYZ'
  • Conjure alias: StringAliasExample

    Aliases have exactly the same JSON representation as their inner type, so they are useful for making error-prone function signatures more bulletproof:

    -doSomething(String, String, String);
    +doSomething(ProductId, UserId, EmailAddress);

Example Jersey interfaces

Conjure-java generates Java interfaces with JAX-RS annotations, so they can be used on the client side or on the server-side.

Example jersey interface: EteService

@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
@Path("/")
@Generated("com.palantir.conjure.java.services.JerseyServiceGenerator")
public interface EteService {
    @GET
    @Path("base/string")
    String string(@HeaderParam("Authorization") AuthHeader authHeader);
}

Jersey clients

Use conjure-java-runtime's JaxRsClient which configures Feign with sensible defaults:

RecipeBookService recipes = JaxRsClient.create(
        RecipeBookService.class,
        userAgent,
        hostMetrics,
        clientConfiguration);

List<Recipe> results = recipes.getRecipes();

Jersey servers

You need a Jersey-compatible server. We recommend Dropwizard (which is based on Jetty), but Grizzly, Tomcat, etc should also work fine. Use conjure-java-runtime's ConjureJerseyFeature to configure Jackson and Exception mappers appropriately. Then, you just need to implement the interface:

public final class RecipeBookResource implements RecipeBookService {

    @Override
    public List<Recipe> getRecipes() {
        // ...
    }
}

Then in your server main method, register your resource. For example, using Dropwizard:

 public void run(YourConfiguration configuration, Environment environment) {
     // ...
+    environment.jersey().register(new RecipeBookResource());
 }

Undertow

In the undertow setting, for a ServiceName conjure defined service, conjure will generate an interface: ServiceName to be extended by your resource and an UndertowService named ServiceNameEndpoints

To avoid conflicts with the equivalent jersey interface (when you are generating both), use in your build.gradle:

conjure {
    java {
        undertowServicePrefixes = true
    }
}

With this option, Undertow generated interfaces will be prefixed with Undertow. Here the interface would be called: UndertowServiceName

To use the generated handlers:

public static void main(String[] args) {
    Undertow server = Undertow.builder()
            .addHttpListener(8080, "0.0.0.0")
            .setHandler(ConjureHandler.builder()
                    .services(RecipeBookServiceEndpoints.of(new RecipeBookResource()))
                    .build())
            .build();
    server.start();
}

Asynchronous Request Processing

The Conjure Undertow generator supports asynchronous request processing allowing all service methods to return a Guava ListenableFuture. These methods may be implemented synchronously by replacing return object with return Futures.immediateFuture(object).

Asynchronous request processing decouples the HTTP request lifecycle from server task threads, allowing you to replace waiting on shared resources with callbacks, reducing the number of required threads.

This feature is enabled on individual endpoints using the server-async endpoint tag:

getEvents:
  http: GET /events
  returns: list<Event>
  tags:
    - server-async

Or for all endpoints using the undertowListenableFutures generator flag:

conjure {
    java {
        undertowListenableFutures = true
    }
}

Timeouts

By default, asynchronous request processing imposes a 3-minute timeout on the asynchronous component of the request, canceling the return future after this duration is exceeded. A custom duration may be used by extending the tag value using the form server-async{timeout=5 minutes}.

Examples

Asynchronous request processing is helpful for endpoints which do not need a thread for the entirety of the request.

๐Ÿ‘ Delegation to an asynchronous client, for instance dialogue ๐Ÿ‘

@Override
public ListenableFuture<String> getValue() {
    // Assuming this dialogue client was compiled with --undertowListenableFutures
    return dialogueClient.getValue();
}

๐Ÿ‘ Long polling ๐Ÿ‘

Long polling provides lower latency than simple repeated polling, but requests take a long time relative to computation. A single thread can often handle updating all polling requests without blocking N request threads waiting for results.

@Override
public ListenableFuture<String> getValue() {
    SettableFuture<String> result = SettableFuture.create();
    registerFuture(result);
    return result;
}

๐Ÿ‘Ž Not for delegation to synchronous operations, Feign clients for example ๐Ÿ‘Ž

This example is less efficient than return Futures.immediateFuture(feignClient.getValue()) because you pay an additional cost to switch threads, and maintain an additional executor beyond the configured server thread pool.

ListeningExecutor executor = MoreExecutors.listeningDecorator(Executors.newCachedThreadPool());

@Override
public ListenableFuture<String> getValue() {
    // BAD: do not do this!
    return executor.submit(() -> feignClient.getValue());
}

๐Ÿ‘Ž Not waiting on another thread ๐Ÿ‘Ž

This is even less efficient than the previous example because it requires two entire threads for the duration of the request. It's reasonable to defer computation to an executor bounded to the work, but you should wrap the executor with MoreExecutors.listeningDecorator and return the future to avoid blocking a server worker thread for both the queued and execution durations of the task.

ExecutorService executor = Executors.newFixedThreadPool(CORES);

@Override
public ListenableFuture<BigInteger> getValue() {
    // BAD: do not do this!
    Future<BigInteger> future = executor.submit(() -> complexComputation());
    BigInteger result = Uninterruptibles.getUninterruptibly(future);
    return Futures.immediateFuture(result);
}

Request Metadata

The RequestContext may be requested on an opt-in basis per-endpoint using the server-request-context conjure endpoint tag:

services:
  ExampleService:
    name: Example
    package: com.palantir.example
    base-path: /example

    endpoints:
      context:
        http: GET /ping
        returns: string
        tags: [server-request-context]

This tag generates an additional parameter in the generated service interface:

public interface ExampleService {
  String ping(RequestContext context);
}

Implementations may read arbitrary headers and query parameters, or associate additional data with the request log:

import java.util.concurrent.atomic.AtomicInteger;

public final class ExampleResource implements ExampleService {
  private final AtomicInteger requestIndex = new AtomicInteger();

  @Override
  public String ping(RequestContext context) {
    // Apply a unique index value to the request log:
    context.requestArg(SafeArg.of("requestNumber", requestIndex.getAndIncrement()));
    // Read the User-Agent header from the current request:
    String userAgent = context.firstHeader("User-Agent");
    // And add it to the request log:
    context.requestArg(SafeArg.of("userAgent", userAgent));
    return "pong";
  }
}

Conjure-Undertow Annotation Processor

The conjure-undertow annotation processor generates Undertow handler for services that can not easily be represented using Conjure. You can find more details in the respective readme.

conjure-lib Bytes class

By default, conjure-java will use java.nio.ByteByffer to represent fields of Conjure type binary. However, the ByteBuffer class has many subtleties, including interior mutability.

To avoid many of these foot-guns, we recommend using the useImmutableBytes feature flag to opt-in to using the com.palantir.conjure.java.lib.Bytes class. This will become the default behaviour in a future major release of conjure-java.

Contributing

For instructions on how to set up your local development environment, check out the Contributing document.

License

This project is made available under the Apache 2.0 License.

conjure-java's People

Contributors

aioobe avatar ash211 avatar bjlaub avatar bmarcaur avatar bulldozer-bot[bot] avatar carterkozak avatar dansanduleac avatar dsd987 avatar ellisjoe avatar esword avatar fawind avatar ferozco avatar hesselink avatar iamdanfox avatar j-baker avatar jaceklach avatar lsingh123 avatar markelliot avatar mglazer avatar mpritham avatar mswintermeyer avatar pkoenig10 avatar qinfchen avatar robert3005 avatar schlosna avatar shibi-bala avatar svc-autorelease avatar svc-excavator-bot avatar tdeitch avatar tjb9dc avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

conjure-java's Issues

Generated service interface does not compile when optional int query param is present

This is on 1.3.0.

search:
  http: POST /tables
  args:
    searchRequest:
      type: TableSearchRequest
    pagingToken:
      type: optional<PagingToken>
      param-type: query
    pageSize:
      type: optional<integer>
      param-type: query
      markers:
        - Safe
  returns: TableSearchResponse

generates

@POST
@Path("search/tables")
TableSearchResponse search(
        @HeaderParam("Authorization") AuthHeader authHeader,
        @QueryParam("pagingToken") Optional<PagingToken> pagingToken,
        @QueryParam("pageSize") @Safe OptionalInt pageSize,
        TableSearchRequest searchRequest);

@Deprecated
default TableSearchResponse search(AuthHeader authHeader, TableSearchRequest searchRequest) {
    return search(authHeader, Optional.empty(), Optional.empty(), searchRequest);
}

@Deprecated
default TableSearchResponse search(
        AuthHeader authHeader,
        Optional<PagingToken> pagingToken,
        TableSearchRequest searchRequest) {
    return search(authHeader, pagingToken, Optional.empty(), searchRequest);
}

The generation of these @Deprecated methods seems new. They don't compile however, because they're delegating with Optional.empty() instead of OptionalInt.empty() for the pageSize query param.

[http-remoting] Serialize Conjure error parameters to JSON rather than with Objects.toString()

Currently in Java implementation error parameters are serialized with Objects.toString() (see SerializableError.forException(ServiceException exception)).
This leads to different formatting of the same error depending on the language used for the error implementation.

I suggest serializing parameters (safe/unsafe error arguments) as JSON - this representation is consistent across languages because parameters are Conjure types.
This way it is also possible to deserialize parameter back into proper Conjure type.

Posting here given this should be expected behavior for all Conjure implementation.

Provide a way to consider unknown union types invalid

Currently Jackson object mapper will happily deserialize an object with bogus union types. This is desirable in certain cases as it allows for additive changes. However, on the server side quite often we want to discard such objects.

Track removing JsonIgnoreProperties from beans

What happened?

Due to a regression introduced earlier on, we generate @JsonIgnoreProperties on all beans:

AnnotationSpec.builder(JsonIgnoreProperties.class)
.addMember("ignoreUnknown", "$L", true)

which overrides the distinct configuration we had intended for servers as opposed to clients:

https://github.com/palantir/conjure-java-runtime/blob/f5c8e7e56f874b47b6707a7ce214f7a36dfdc2d8/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java#L67

What did you want to happen?

#121 should be merged once we are certain that this won't break existing clients.

values() method for enum-types in Java

This would be super valuable. Right now, I'm finding myself reaching to the embedded Value type, which feels kind of inelegant and I have to filter out the UNKNOWN value.

For example, retrieving the string value names for display in a drop-down window:

    public static <T extends Enum<T>> String[] valueNamesWithoutUnknown(Class<T> underlyingConjureEnumType) {
        List<String> ret = Lists.newArrayList();
        for (T val : underlyingConjureEnumType.getEnumConstants()) {
            if (!val.name().equals("UNKNOWN")) {
                ret.add(val.name());
            }
        }
        return ret.toArray(new String[0]);
    }

Also, writing a test that creates jobs with different types, which includes not only having to filter out UNKNOWN but also having to convert back into the actual type itself:

        List<PeeringActionType.Value> peeringTypes = Lists.newArrayList(PeeringActionType.Value.values());
        // <snip>
        for (PeeringActionType.Value type : peeringTypes) {
            if (type != PeeringActionType.MINIMAL_EXPORT.get() && type != PeeringActionType.Value.UNKNOWN) {
                QueuedPeeringJobMetadata first = createPeeringJobMetadata(i++, 14, PeeringActionType.valueOf(type.name()));
                // <snip>
            }
        }

docs on ArgumentDefinition are ignored

What happened?

docs: Description of argument is ignored by the java generator.

What did you want to happen?

I expect @param javadoc markers with the specified values.

Consideration: generate code for built-in types?

Currently, generated Java code hard-codes imports for types like SafeLong, ResourceIdentifier, etc. For example:

package com.palantir.product;

import com.palantir.conjure.java.lib.SafeLong;
import com.palantir.ri.ResourceIdentifier;
import com.palantir.tokens.auth.AuthHeader;
import com.palantir.tokens.auth.BearerToken;
....

    @GET
    @Path("base/string")
    String string(@HeaderParam("Authorization") @NotNull AuthHeader authHeader);

This could cause issues because the resolution of these imports is dependent on the project setup.

That is, 2 projects that use the exact same Conjure definition and Conjure generator of the same version can generate incompatible code depending on the version of ResourceIdentifier that is resolved within their project.

The only way to truly avoid this is to always generate the built-in types as part of the Conjure generation process (and clients would need to write code to convert between the generated built-in types and the "real" types like com.palantir.tokens.auth.BearerToken).

An intermediate solution would be to define all of these types in the conjure.java project (as is done with SafeLong) and make it a requirement that, if code is generated using a given version of Conjure, then that version of conjure.java.lib must be imported in the project.

As it stands right now, the true compatibility graph is spread out among multiple repos -- you have to track the compatibility between versions of com.palantir.ri and Conjure, com.palantir.tokens and Conjure, etc. This isn't an issue right now because there are no breaking changes, but if there ever were to be then this would be extremely hard to reason through.

This came up in a discussion between @bmoylan and I when we were discussing analogous issues in Go.

Error classes do not have private constructors

What happened?

Generated objects do not enforce non-instantiability through a private constructor even though the classes are final and all of the functions on the class are static.

What did you want to happen?

There is also a private constructor in those objects.

Conjure-generated client does not properly serialize nested aliases in PLAIN context

Generate a Conjure client using the following definition:

types:
  definitions:
    default-package: com.palantir.product
    objects:
      AliasOne:
        alias: string
      AliasTwo:
        alias: optional<AliasOne>
      AliasThree:
        alias: AliasTwo

services:
  EteService:
    name: Ete Service
    package: com.palantir.product
    base-path: /base

    endpoints:
      aliasThree:
        http: GET /aliasThree
        args:
          queryParamName:
            param-type: query
            type: AliasThree
        returns: AliasThree

Make the following client call:

eteService.aliasThree(AliasThree.of(AliasTwo.of(Optional.empty()))

Expected

Calls the endpoint with no query parameters (because serialized equivalent is Optional.empty())

Actual

Calls the endpoint with a query parameter that consists of the literal string Optional.empty

The same issue exists for non-optional calls:

eteService.aliasThree(
                AliasThree.of(AliasTwo.of(Optional.of(AliasOne.of("hello"))))

Expected

Query parameter value is hello

Actual

Query parameter value is Optional[hello]


Based on this, it appears that PLAIN serialization does not properly work for alias types nested in optionals.

Generated code should transform markdown to html javadoc

What happened?

The conjure specification suggests that docs may use markdown, however service authors tend to use syntax for their favorite/current language instead, which leads to low quality documentation across other targets.

What did you want to happen?

Markdown in conjure yml should be rendered into html for javadoc.

Publish jar file for core Java generator code

Publishing a jar for core Java generator code will allow other service targets (e.g. Undertow, Netty) to make use of the same infrastructure and type definitions while replacing the specific service outputs.

We might also consider splitting the type generators from the service generators -- but either way need things like the TypeMapper infrastructure to be available for services so they can refer to types.

Overloaded methods for optional query params may silently allow compilation in some unsafe cases

With #21, we generate @Deprecated methods with the intention of avoiding compile breaks in cases where new query params are added. However, in some cases we might silently let compilation go through when it shouldn't. Specifically, if we add another optional query params with the same type as an existing one and in the api.yml declare it right before the existing one.

As an example, lets say we have the following API:

getTransactionsPage:
  http: POST /get-page
  args:
    query: string
    optionalEnd:
      type: optional<ResourceIdentifier>
      param-type: query
  returns: set<ResourceIdentifier>
-----
@POST
@Path("catalog/get-page")
Set<ResourceIdentifier> getTransactionsPage(
        @HeaderParam("Authorization") AuthHeader authHeader,
        @QueryParam("optionalEnd") Optional<ResourceIdentifier> optionalEnd,
        String query);

@Deprecated
default Set<ResourceIdentifier> getTransactionsPage(AuthHeader authHeader, String query) {
    return getTransactionsPage(authHeader, Optional.empty(), query);
}

and this is being used as follows in the client code:

Set<ResourceIdentifier> transactions = getTransactionsPage(authHeader, endTransactionRid, query);

Lets say the API was included to also have an optionalStart query param:

getTransactionsPage:
  http: POST /get-page
  args:
    query: string
    optionalStart:
      type: optional<ResourceIdentifier>
      param-type: query
    optionalEnd:
      type: optional<ResourceIdentifier>
      param-type: query
  returns: set<ResourceIdentifier>
-----
@POST
@Path("catalog/get-page")
Set<ResourceIdentifier> getTransactionsPage(
        @HeaderParam("Authorization") AuthHeader authHeader,
        @QueryParam("optionalStart") Optional<ResourceIdentifier> optionalStart,
        @QueryParam("optionalEnd") Optional<ResourceIdentifier> optionalEnd,
        String query);

@Deprecated
default Set<ResourceIdentifier> getTransactionsPage(AuthHeader authHeader, String query) {
    return getTransactionsPage(authHeader, Optional.empty(), Optional.empty(), query);
}

@Deprecated
default Set<ResourceIdentifier> getTransactionsPage(
        AuthHeader authHeader, Optional<ResourceIdentifier> optionalStart, String query) {
    return getTransactionsPage(authHeader, optionalStart, Optional.empty(), query);
}

Compilation of the client code wouldn't break but it is now doing something semantically different.

To prevent, we'd have to ensure that whenever devs add new optional query params they always declare it last. However, I don't think thats something we do at the moment.

I discussed this @iamdanfox and he had a proposal around using builders instead to pass optional query and header params.

Allow optional alias as a header parameter

I have a type which is an alias of a string

      MyTokenType:
        alias: string

and then I use it as a header parameter

    endpoints:
      myEndpoint:
        http: POST /
        args:
          someToken:
            param-id: My-Token
            param-type: header
            type: optional<MyTokenType>
          request: MyEndpointRequest

Exception in thread "main" java.lang.IllegalStateException: Header parameters must be primitives, aliases or optional primitive: "temporaryCredentialsAuthToken" is not allowed

I am able to switch the alias type to be itself alias: optional<string> (and take non-optional MyTokenType as the header param type) but I'm not sure the generated code then works (I haven't yet confirmed with a failing test) - @JsonCreator annotation ends up taking an Optional<String>

Alias of list<T> tolerates null items

What happened

The following Conjure type:

MyList
  alias: list<string>

Deserialized from [null, null] without any errors, leading to nasty nulls in my codebase:

System.out.println(ObjectMappers.newServerObjectMapper()
    .readValue("[null, null]", MyList.class)); // prints [null, null]

What should have happened

These nulls should have been rejected. This matches guava's ImmutableList behaviour, which nullchecks every item you add (even if you do copyOf).

Conjure primitive with kebab-cased field name caused java-poet exception

In 1.0.0 this was working, but after upgrading to 2.0.0, the following definition started breaking:

SomethingConfig:
  fields:
    bin-count:
      docs: example
      type: integer
> Task :product-api:compileConjureObjects
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
Exception in thread "main" java.lang.IllegalArgumentException: not a valid name: _bin-countInitialized
        at com.squareup.javapoet.Util.checkArgument(Util.java:53)
        at com.squareup.javapoet.FieldSpec.builder(FieldSpec.java:91)
        at com.palantir.conjure.java.types.BeanBuilderGenerator.lambda$primitivesInitializedFields$0(BeanBuilderGenerator.java:163)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
        at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:177)
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1492)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
        at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
        at com.palantir.conjure.java.types.BeanBuilderGenerator.primitivesInitializedFields(BeanBuilderGenerator.java:167)
        at com.palantir.conjure.java.types.BeanBuilderGenerator.generate(BeanBuilderGenerator.java:89)
        at com.palantir.conjure.java.types.BeanBuilderGenerator.generate(BeanBuilderGenerator.java:77)
        at com.palantir.conjure.java.types.BeanGenerator.generateBeanType(BeanGenerator.java:109)
        at com.palantir.conjure.java.types.ObjectGenerator.lambda$generateTypes$0(ObjectGenerator.java:38)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1492)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
        at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
        at com.palantir.conjure.java.types.ObjectGenerator.generateTypes(ObjectGenerator.java:52)
        at com.palantir.conjure.java.types.TypeGenerator.generate(TypeGenerator.java:37)
        at com.palantir.conjure.java.types.TypeGenerator.emit(TypeGenerator.java:51)
        at com.palantir.conjure.java.cli.ConjureJavaCli.generate(ConjureJavaCli.java:87)
        at com.palantir.conjure.java.cli.ConjureJavaCli.main(ConjureJavaCli.java:46)

Likely introduced in #36

FR: wrap() method for union types

Given the union type:

MyUnionType:
  union:
    type1: UnionType1
    type2: UnionType2
    type3: UnionType3

We could have UnionType1, UnionType2 and UnionType3 implement some interface, say

interface MyUnionTypeMember {
  MyUnionType wrapAsMyUnionType();
}

With implementations

class UnionType1 {
...
  MyUnionType wrapAsMyUnionType() {
    return MyUnionType.unionType1(this);
  }
}

etc.

I've come across a few cases now where this would have solved some boilerplate.

Granted there are some limitations - it wouldn't work for union members that are primitive types or imports - and it may not be worth the extra complexity. But worth a thought.

Make primitive fields required on builder.build()?

For non-primitive, non-optional, non-collection type fields, we require that the user explicitly set the field on the builder, or else throw an exception. I get that logic, cause not saying optional implies required, means they should be set. However, we do not do the same check for primitive fields. That is to say, the following type definition:

Foo:
  fields:
    bar: integer

will allow the following builder invocation to build, successfully:

Foo.builder().build();

this seems inconsistent with the idea that the server controls default parameters. Can we make primitive fields required to be called as well?

One implementation for this can be found in internal gerrit #306681 , can port to here if it's reasonable.

Introduce default visitUnknown for enum visitors

What happened?

The Visitor generated for enums requires implementations to handle the visitUnknown case, but we literally never do anything other than throw a generic exception in the unknown case.

What did you want to happen?

Either provide a default implementation for visitUnknown or generate a StrictVisitor where accept(StrictVisitor) handles the unknown case instead of passing the responsibility on to the implementation.

Add more helper methods for dealing with union types

What happened?

We have a lot of data structures that are unions with many possible value types (sometimes over a dozen). Working with these can be very difficult since code needs to implement a visitor for all possible types, even if the code wants to ignore most of those types, or in the cases of tests assert that the object has a specific type (in tests it isn't very obvious how you would even construct a Matcher for these without implementing a visitor that defines every single method -- even the ones you don't want to match against). Using just object equals in tests is often not an option since the test might only want to assert on parts of the value, or some fields might be things like timestamps that change (in the case of integration tests etc). The current workaround is often for testing purposes to define a visitor that throws in every method, and then in tests extend that base visitor and @Override the one (or several) methods you want to handle in the test.

What did you want to happen?

Consider adding generated methods to union types that allow you to easily test if a union object has a specific type, and retrieve the value from it. These would work by allowing the user to create a java.util.function.Consumer that accepts that type.

As a concrete example, consider this (partial) conjure object definition:

MyData:
  union:
    foo: FooType
    # ... many other subtypes follow

Methods like this could be generated on MyData:

public void ifFoo(Consumer<FooType> consumer) {
    if (value instanceof FooWrapper) {
        consumer.accept((FooWrapper) value).value);
    }
}

Similarly we could generate a method like this to throw if the object does not have that type:

public void requireFoo(Consumer<FooType> consumer) {
    if (value instanceof FooWrapper) {
        consumer.accept((FooWrapper) value).value);
    } else {
        throw new IllegalStateException(...);
    }
}

Another alternative is to auto generate an abstract visitor that throws in every method, so that you can extend that and @Override the specific methods you want.

Undertow/Invalid argument with no request body

What happened?

  • have an undertow endpoint with an optional request body (i.e., Optional<BodyType>)
  • calling the endpoint without passing any request body throws an exception
INFO  [2019-04-03T11:01:14.46Z]  com.palantir.conjure.java.undertow.runtime.ConjureExceptionHandler: Error handling request (errorInstanceId: 2e7227fe-9233-4bab-995f-f86d937aee07, errorName: Default:InvalidArgument, throwable1_type: map[actualTypeArguments:[BodyType] rawType:java.util.Optional typeName:java.util.Optional<BodyType>]) (throwable2_message: No content to map due to end-of-input
 at [Source: (io.undertow.io.UndertowInputStream); line: 1, column: 0])
com.palantir.conjure.java.api.errors.ServiceException: ServiceException: INVALID_ARGUMENT (Default:InvalidArgument)
	at com.palantir.conjure.java.undertow.runtime.ConjureExceptionHandler.frameworkException(ConjureExceptionHandler.java:140)
	at com.palantir.conjure.java.undertow.runtime.ConjureExceptionHandler.handleRequest(ConjureExceptionHandler.java:74)
	at com.palantir.tracing.undertow.TracedOperationHandler.handleRequest(TracedOperationHandler.java:68)
	at com.palantir.conjure.java.undertow.runtime.LoggingContextHandler.handleRequest(LoggingContextHandler.java:40)
	at io.undertow.server.Connectors.executeRootHandler(Connectors.java:364)
	at io.undertow.server.HttpServerExchange$1.run(HttpServerExchange.java:830)
	at org.jboss.threads.EnhancedQueueExecutor.safeRun(EnhancedQueueExecutor.java:1871)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1451)
	at java.lang.Thread.run(Thread.java:834)
Caused by: com.palantir.conjure.java.undertow.runtime.FrameworkException: Failed to deserialize response stream. Syntax error?
	at com.palantir.conjure.java.undertow.runtime.FrameworkException.unprocessableEntity(FrameworkException.java:40)
	at com.palantir.conjure.java.undertow.runtime.Encodings$AbstractJacksonEncoding.lambda$deserializer$1(Encodings.java:67)
	at com.palantir.conjure.java.undertow.runtime.ConjureBodySerDe$EncodingDeserializerRegistry.deserialize(ConjureBodySerDe.java:148)

......


Caused by: com.fasterxml.jackson.databind.exc.MismatchedInputException: No content to map due to end-of-input
 at [Source: (io.undertow.io.UndertowInputStream); line: 1, column: 0]
	at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:59)
	at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1342)
	at com.fasterxml.jackson.databind.ObjectReader._initForReading(ObjectReader.java:358)
	at com.fasterxml.jackson.databind.ObjectReader._bindAndClose(ObjectReader.java:1593)
	at com.fasterxml.jackson.databind.ObjectReader.readValue(ObjectReader.java:1185)
	at com.palantir.conjure.java.undertow.runtime.Encodings$AbstractJacksonEncoding.lambda$deserializer$1(Encodings.java:62)
	at com.palantir.conjure.java.undertow.runtime.ConjureBodySerDe$EncodingDeserializerRegistry.deserialize(ConjureBodySerDe.java:148)

What did you want to happen?

When using Jersey instead of undertow, the body is mapped to null, so undertow should preserve the same behavior.

The error seems to be coming from this line https://github.com/palantir/conjure-java/blob/develop/conjure-java-undertow-runtime/src/main/java/com/palantir/conjure/java/undertow/runtime/Encodings.java#L62

Bean map fields should not allow null values

What happened?

MapExample.builder().items("foo", null).build() produces MapExample{items: {foo=null}}

What did you want to happen?

I expected a SafeNullPointerException("value cannot be null") to be thrown

Server request body parameters of types map, set, and list should not be mutable

What happened?

Using both conjure-jersey and conjure-undertow server generators I can modify request body parameter with type List, Set, or Mapin server implementations.

What did you want to happen?

Human defined implementations of conjure interfaces should only receive immutable list, set, and map objects. Note I am not suggesting that we should change the signature of service methods, only the implementation we pass.
If we choose to use the guava ImmutableList, ImmutableSet, and ImmutableMap, we will also gain validation that null values are not allowed (see #105)

Consider Staged Builders

What happened?

If we neglect to set all required fields in a bean builder, currently it results in failure at runtime.

What did you want to happen?

We can use staged builders to cause compilation failures in cases that currently only fail at runtime.
Downside of this approach is that existing builders would have to be re-ordered, so it would have to exist behind a feature flag.

Foo.Of method makes all arguments mandatory, including optionals

What happened?

Adding an optional field to an object definition in yml causes the generated object to require the field in its of method. This technically makes any optional field addition breaking.

e.g. going from

      DuplicateRequest:
        fields:
          parentFolderId: optional<string>

to

      DuplicateRequest:
        fields:
          parentFolderId: optional<string>
          foo: optional<string>

generates

@JsonDeserialize(builder = DuplicateRequest.Builder.class)
@Generated("com.palantir.conjure.java.types.BeanGenerator")
public final class DuplicateRequest {
 ...
    public static DuplicateRequest of(String parentFolderId, String foo) {
        return builder().parentFolderId(Optional.of(parentFolderId)).foo(Optional.of(foo)).build();
    }
}

A few things seem off wrong here:

  • of shouldn't require optional fields - they should be optional or hidden via overloading
  • adding an optional field should preserve the previous of methods in case consumers were already using them - removing them

What did you want to happen?

Some combination of:

  • The of method should require optionals for optional fields.
  • The of method should be overloaded for all possible argument combinations given optional arguments
  • The of method only takes required fields, ignoring optional ones so there's no chance of breaks
  • No of method in the first place

I'm making the assumption that the method signature change here is a (small) break.

Retrofit service parameters are not sorted

What happened?

Retrofit service interface parameters are not sorted and do not provide backwards compatibility default methods with guarantees matching jersey.

What did you want to happen?

Backwards compatibility guarantees for retrofit and jersey should match

FR: Defaulting visitors in java

Motivation

For a union type of A, B, C, D, it isn't unusual do want to do if (value is A) do X else do Y.

It would be nice if conjure supported this OOTB.

Note that this is already possible with typescript.

Proposal

instead of generating visitors as such:

interface Visitor<T> {
    T visitA(A a);
    T visitB(B b);
    T visitC(C c);
    T visitUnknown(String type);
}

I propose

interface Visitor<T> {
    default T visitA(A a) {
        return visitDefault();
    }

    default T visitB(B b) {
        return visitDefault();
    }

    default T visitC(C c) {
        return visitDefault();
    }

    default T visitUnknown(String type) {
        return visitDefault();
    }

    T visitDefault();
}

A downside of the proposal is that users are then forced to implement visitDefault() even when they don't need it (when they have an implementation for each type in the union). One solution would be to have that have a default implementation too (throw new UnsupportedOperationException()?) although that feels like it would lead to runtime breaks. A better solution might be to make the above a separate DefaultingVisitor<T> extends Visitor<T> interface, so users only have to implement visitDefault() when they need to.

Retrofit should handle optional<binary> responses

What happened?

Retrofit fails to decode optional because it expects JSON and receives application/octet-stream data.

What did you want to happen?

Retrofit should allow the consumer to stream a response if present.

Object builders allow sneaky mutations of maps inside created POJOs

conjure-java generates Builder classes for plain object types, which contain mutable collections (e.g. HashMap, ArrayList, HashSet). In the .build() method, we pass these mutable collections to the POJO's constructor, which uses Collections.unmodifiableMap(...) to prevent mutations from POJO methods HOWEVER, the builder still has references to the underlying mutable collections, so these can still be mutated!

Foo:
  fields: 
    keys: map<string, integer>
    @Test
    public void sad() {
        Foo.Builder builder = Foo.builder();
        Foo schema = builder.build(); // create a Foo containing an empty 'keys' map

        builder.keys("this call mutates a underlying list", 1234);

        assertThat(schema.getKeys()).isEmpty(); // <- ๐Ÿ”ฅ๐Ÿ”ฅ this fails
    }

Reported by @j-baker

Generated object builders don't guard against method name conflicts

Consider the following object definition:

types:
  definitions:
    objects:
      ConflictedObject:
        foo: list<string>
        addAllFoo: list<string>

The generated Java implementation fails to compile, as the builder ends up with the following pair of methods:

        public Builder addAllFoo(Iterable<String> foo) {
            ConjureCollections.addAll(this.foo, Objects.requireNonNull(foo, "foo cannot be null"));
            return this;
        }

        @JsonSetter("addAllFoo")
        public Builder addAllFoo(Iterable<String> addAllFoo) {
            this.addAllFoo.clear();
            ConjureCollections.addAll(
                    this.addAllFoo, Objects.requireNonNull(addAllFoo, "addAllFoo cannot be null"));
            return this;
        }

It seems like the first method's name should be munged in some way to avoid the conflict, like addAllFoo_.

Conjure-generated service does not start if endpoint contains certain alias definitions

Example input definition:

types:
  definitions:
    default-package: com.palantir.product
    objects:
      AliasOne:
        alias: string
      AliasTwo:
        alias: optional<AliasOne>
services:
  EteService:
    name: Ete Service
    package: com.palantir.product
    default-auth: header
    base-path: /base

    endpoints:
      optionalAliasOne:
        http: GET /optionalAliasOne
        args:
          queryParamName:
            param-type: query
            type: optional<AliasOne>
        returns: optional<AliasOne>

      aliasTwo:
        http: GET /aliasTwo
        args:
          queryParamName:
            param-type: query
            type: AliasTwo
        returns: AliasTwo

Note that, form a serialization/semantic standpoint, the optionalAliasOne and aliasTwo are equivalent.

However, attempting to start a Conjure-generated server with these definitions fails with the following exception:

Caused by: org.glassfish.jersey.server.model.ModelValidationException: Validation of the application resource model has failed during application initialization.
[[FATAL] No injection source found for a parameter of type public com.palantir.product.AliasTwo com.palantir.conjure.java.EteResource.aliasTwo(com.palantir.tokens.auth.AuthHeader,com.palantir.product.AliasTwo) at index 1.; source='ResourceMethod{httpMethod=GET, consumedTypes=[application/json], producedTypes=[application/json], suspended=false, suspendTimeout=0, suspendTimeoutUnit=MILLISECONDS, invocable=Invocable{handler=ClassBasedMethodHandler{handlerClass=class com.palantir.conjure.java.EteResource, handlerConstructors=[org.glassfish.jersey.server.model.HandlerConstructor@6ac4c3f7]}, definitionMethod=public abstract com.palantir.product.AliasTwo com.palantir.product.EteService.aliasTwo(com.palantir.tokens.auth.AuthHeader,com.palantir.product.AliasTwo), parameters=[Parameter [type=class com.palantir.tokens.auth.AuthHeader, source=Authorization, defaultValue=null], Parameter [type=class com.palantir.product.AliasTwo, source=queryParamName, defaultValue=null]], responseType=class com.palantir.product.AliasTwo}, nameBindings=[]}']

If the aliasTwo endpoint is commented out, the server starts up as expected.

The aliasTwo types and definitions should be legal per the Conjure spec, so the Java generator creating a server that does not start based on these valid definitions is a bug.

Consider a structured approach to enriching/modifying endpoint behavior

What happened?

No built-in way to modify endpoint behavior today. #406 proposes a way to wholesale replace functionality.

What did you want to happen?

Consider a structured aspect approach to enriching/modifying endpoint behavior:

interface EndpointAspect<C> {
    interface Factory {
        /** 
         * Returns an {@link Optional} containing a new {@link EndpointAspect} instance for 
         * {@link Endpoint} if this aspect is relevant for the specified endpoint, and empty otherwise.
         */
        <Context> Optional<? extends EndpointAspect<Context>> create(Endpoint endpoint);
    }

    /**
     * Invoked prior to the underlying {@link HttpHandler#handle(HttpServerExchange)} call
     * and returns a context object of type {@code C}.
     */
    default C before(HttpServerExchange exchange) {
        return null;
    }

    /** 
     * Returns true to prevent calling the underlying {@link HttpHandler} implementation.
     * Defaults to {@code false}.
     */
    default boolean haltCall(C context, HttpServerExchange exchange) {
        return false;
    }

    /**
     * Invoked after the underlying {@link HttpHandler#handle(HttpServerExchange)} call,
     * if the call does not throw, and returns a context object of type {@code C}.
     */
    default C onSuccess(C context, HttpServerExchange exchange) {
        return null;
    }

    /**
     * Invoked after the underlying {@link HttpHandler#handle(HttpServerExchange)} call,
     * if the call throws, and returns a context object of type {@code C}.
     */
    default C onThrow(C context, HttpServerExchange exchange, Exception e) {
        return null;
    }

    /**
     * Invoked as a completion listener to the the handler call chain.
     */
    default void completion(C context, HttpServerExchange exchange) {}
}

Which is implemented as:

service.endpoints(runtime).stream()
    .map(endpoint ->
        factory.create(endpoint)
            .map(aspect ->
                Endpoint.builder()
                    .from(endpoint)
                    .handler(exchange -> {
                        Object context = aspect.before(exchange);
                        if (!aspect.haltCall(context, exchange) {
                            try {
                                endpoint.handler().handleRequest(exchange);
                                onSuccess(context, exchange);
                                exchange.addCompletionListener(completion(context, exchange));
                            } catch (Exception e) {
                                onThrow(context, exchange, e);
                            }
                        }
                    })
                    .build())
            })
            .orElse(endpoint.handler())
    .collect(ImmutableList.toImmutableList()));

Undocumented breaking change between 1.0.0 and 1.1.0

There is an undocumented breaking change between 1.0.0 and 1.1.0 that causes any code that has query parameters appear before any required parameters to fail compilation. The easiest workaround is to reorder the parameters, however that can cause confusion to end users where the type safety may be weaker than in Java (aka TypeScript).

Having an optional<any> generates an unchecked cast

What happened?

Defining an object like this:

MyObject:
  fields:
    myfield: optional<any>

Results in the following Java code:

    @Generated("com.palantir.conjure.java.types.BeanBuilderGenerator")
    @JsonIgnoreProperties(ignoreUnknown = true)
    public static final class Builder {
        ...
        @JsonSetter("detail")
        public Builder detail(Optional<?> detail) {
            this.detail =
                    (Optional<Object>) Preconditions.checkNotNull(detail, "detail cannot be null");
            return this;
        }

        public Builder detail(Object detail) {
            this.detail = Optional.of(Preconditions.checkNotNull(detail, "detail cannot be null"));
            return this;
        }
    }

Where Optional<?> is cast into an Optional<Object>.

What did you want to happen?

    public static final class Builder {
        ...
        @JsonSetter("detail")
        public Builder detail(Optional<Object> detail) {
            this.detail = Preconditions.checkNotNull(detail, "detail cannot be null");
            return this;
        }
		...
    }

Auto-fix type name 'Object'

This should not create a class called 'Object.java' otherwise it clashes horribly!

      Object:
        fields:
          string:
            type: string

Enum types in Java should implement some common interface

Some sort of interface a la Enum<T>, call it ConjureEnum<T>. It doesn't even necessarily need the same methods (except for a values method as in #78, and the valueOf method [perhaps with the guarantee that ConjureEnum#valueOf applied to ConjureEnum#toString always gives you back the same value])

This is useful in a few cases where I want to write some utility code that applies generally to enums, but after replacing with Conjure enums, I have to either abandon that or pass two classes (the internal enum value class as well as the outer class).

Examples:

Getting string values for printing in a drop-down menu:

    public static <T extends Enum<T>> String[] valueNamesWithoutUnknown(Class<T> underlyingConjureEnumType) {
        List<String> ret = Lists.newArrayList();
        for (T val : underlyingConjureEnumType.getEnumConstants()) {
            if (!val.name().equals("UNKNOWN")) {
                ret.add(val.name());
            }
        }
        return ret.toArray(new String[0]);
    }

We frequently have enum classes with util methods that round-trip the values through an id (e.g. for serializing to a DB), so we unit test that the round-trip is stable:

public class PeeringConjureEnumUtilBaseTest<T, U extends Enum<U>> extends BaseTest {
    // <snip>
    protected final void testIdRoundTrips(Function<T, Long> getIdFunc,
                                Function<Long, T> fromIdFunc,
                                Class<U> clazz,
                                Function<U, T> fromEnumFunc) {
        try (AutoCloseableSoftAssertions softAssert = new AutoCloseableSoftAssertions()) {
            for (U enumValue : clazz.getEnumConstants()) {
                if (!enumValue.name().equals("UNKNOWN")) {
                    T value = fromEnumFunc.apply(enumValue);
                    try {
                        T roundTrippedValue = fromIdFunc.apply(getIdFunc.apply(value));
                        softAssert.assertThat(roundTrippedValue)
                                .as("Round-tripping through converters should provide same value.")
                                .isEqualTo(value);

                    } catch (Throwable e) {
                        softAssert.assertThat(true)
                                .as("Failed to roundtrip value %s with exception %s",
                                        value,
                                        e.getMessage())
                                .isFalse();
                    }
                }
            }
        }
}

In a property/configuration class where I know the values will take on the values of an enum and I want to validate that as well as return the right value:

    @Override
    public <T extends Enum<T>, U> U getEnum(PeerPropertyLookup remoteSystemId,
                                            NPPeerProperty prop,
                                            Class<T> enumClass,
                                            Class<U> returnClass) {
        Validate.isTrue(prop.getConjureEnumReturnType().equals(returnClass));
        // This validates that it's a valid entry in the enum.
        T enumReturn = getEnum(remoteSystemId, prop, enumClass);
        // This is a tricksy way to call #valueOf that doesn't rely on reflection but does rely on the fairly safe
        // assumption that Conjure will continue to make their enums Jackson-serializable in the future.
        try {
            return new ObjectMapper().readValue("\"" + enumReturn.name() + "\"", returnClass);
        } catch (IOException e) {
            throw new RuntimeException("Failed to parse enum for peer property " + prop + " into class " + returnClass.getName(),
                    e);
        }
    }

I think all of these would be significantly more simple and readable if I could rely on a <T extends ConjureEnum<T>> and I don't see a downside to doing so.

Generated Java code should not import java.lang classes

Generated service definitions currently include imports for java.lang classes like Object, String, Deprecated, and Void. The java.lang package is imported implicitly, so code formatters like https://github.com/diffplug/spotless/ will reject these imports. For now people can workaround this by configuring their formatter to skip the generated files, but it would be a better experience if the generated services have the correct imports to begin with.

This bug does not appear to happen in generated types, only in generated services.

Errors not handled properly when streaming responses

Currently, when an endpoint returns binary we render a StreamingOutput in the jersey interface. Because the Content-Length is unknown, Jetty properly responds using chunked transfer encoding which allows the client to determine whether the stream has been fully received or failed part way through. There's an issue in Jersey, though, which actually closes the stream when an exception is thrown after some bytes have been written to the client which in turn causes Jetty to close the stream as if it were completed successfully: https://en.wikipedia.org/wiki/Chunked_transfer_encoding

There's not much we can do about streaming back things of unknown size, but we should be able to support streams where the size is known. I'd propose that we add a BinaryStream or Binary or something which takes the place of StreamingOutput in conjure generated interfaces and has two methods:

of(StreamingOutput)
of(StreamingOutput, long contentLength)

think it may be reasonable to stick this in remoting, but sounds like these two projects are converging and we're going to need to change the conjure code gen at some point.

This change also doesn't affect clients of these endpoints because they must use the Retrofit client when consuming streaming endpoints.

Optional field builder should be covariant

Currently conjure generates builders for optionals like so:

        private Optional<Object> lt = Optional.empty();

        /** Optional exclusive upper bound for the value of the Field. */
        @JsonSetter("lt")
        public Builder lt(Optional<Object> lt) {
            this.lt = Objects.requireNonNull(lt, "lt cannot be null");
            return this;
        }

        /** Optional exclusive upper bound for the value of the Field. */
        public Builder lt(Object lt) {
            this.lt = Optional.of(Objects.requireNonNull(lt, "lt cannot be null"));
            return this;
        }

Since we know optionals are read-only, it's safe to be covariant here:

        private Optional<Object> lt = Optional.empty();

        /** Optional exclusive upper bound for the value of the Field. */
        @JsonSetter("lt")
        public Builder lt(Optional<? extends Object> lt) {
            this.lt = Objects.requireNonNull(lt, "lt cannot be null").flatMap(Optional::<Object>of);
            return this;
        }

        /** Optional exclusive upper bound for the value of the Field. */
        public Builder lt(Object lt) {
            this.lt = Optional.of(Objects.requireNonNull(lt, "lt cannot be null"));
            return this;
        }

which avoids a gotcha for Object specifically (passing Optional<String> into .lt and getting back Optional<Optional<String>>), and makes the interface nicer for all optionals.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.