Skip to content

Conversation

@edeandrea
Copy link
Contributor

@edeandrea edeandrea commented Oct 23, 2025

An idea for api design. This still allows using Java records to implement the model objects, but also makes these objects backwards-compatible by introducing interfaces & builders.

NOTE I've only applied the pattern to the DocumentResponse so that we can decide if this is a good approach or not. If so, we can apply this pattern to the others.

FIxes #23
Fixes #24

@edeandrea
Copy link
Contributor Author

@ThomasVitale @lordofthejars take a look and let me know what you think of this approach. I didn't get into ServiceLoaders or anything like that. I think that overcomplicates things.

@edeandrea edeandrea force-pushed the api-design branch 3 times, most recently from 145e42f to 14aae2d Compare October 23, 2025 20:08
@edeandrea edeandrea marked this pull request as draft October 23, 2025 20:19
Copy link

@lordofthejars lordofthejars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow almost everything is done hahaha we are almost ready to go. Looks good to me just one small comment.

@edeandrea
Copy link
Contributor Author

Wow almost everything is done hahaha we are almost ready to go.

Well, not really :) This is just a design idea that I've only implemented for one of the model objects to show the idea/pattern. If we want to adopt this pattern then we need to implement it across the board.

Then we need documentation, release automation, etc :)

@lordofthejars
Copy link

Wow almost everything is done hahaha we are almost ready to go.

Well, not really :) This is just a design idea that I've only implemented for one of the model objects to show the idea/pattern. If we want to adopt this pattern then we need to implement it across the board.

Then we need documentation, release automation, etc :)

Perfect, looks good to me, then we erge this one and we adapt the rest of the model, or we push ito this PR?

@edeandrea
Copy link
Contributor Author

Wow almost everything is done hahaha we are almost ready to go.

Well, not really :) This is just a design idea that I've only implemented for one of the model objects to show the idea/pattern. If we want to adopt this pattern then we need to implement it across the board.
Then we need documentation, release automation, etc :)

Perfect, looks good to me, then we erge this one and we adapt the rest of the model, or we push ito this PR?

If we like this model then I'll fill in this PR with the rest of the API (its currently marked as draft). I'd like Thomas to weigh in first though, since he was the one who built the initial version.

@ThomasVitale
Copy link
Contributor

ThomasVitale commented Oct 26, 2025

Thanks for this PR! In general, I like the approach, but I have two concerns:

  • If we configure serialisers/deserializers in the API, then we are forcing the usage of a certain Jackson version and that might be problematic (I guess that was why Initial architecture & purpose #2 (comment)). If we stick to the annotations from the com.fasterxml.jackson.annotation package, they are the same across Jackson 2 and 3 (it's the only module that was not changed), so libraries will be free to use the API with their favourite version of Jackson (or even with another tool).
  • I understand the idea behind the design based on interfaces and default implementation, but I'm a bit worried about the effort it would take us to maintain it manually. Existing generators like openapi-generator or openapi-processor don't have support for this kind of design (as far as I know), so even if we make the OpenAPI spec from Docling work, we might not be able to switch to the generated model in a straightforward way. I know that one of the reason for this design is to ensure backward compatibility, so I started a list of requirements in API Design #23 (comment) to help us decide on the final design. I have a hard time imagining situations where, as a library author, I would want to implement the interfaces myself for the data models (typically, it's the client interface that I would customise). But if that's a use case we want to support, let's add it to the list of requirements. What do you think? @edeandrea @lordofthejars

@edeandrea
Copy link
Contributor Author

Thanks for this PR! In general, I like the approach, but I have two concerns:

  • If we configure serialisers/deserializers in the API, then we are forcing the usage of a certain Jackson version and that might be problematic (I guess that was why Initial architecture & purpose #2 (comment)). If we stick to the annotations from the com.fasterxml.jackson.annotation package, they are the same across Jackson 2 and 3 (it's the only module that was not changed), so libraries will be free to use the API with their favourite version of Jackson (or even with another tool).

Agreed. And I realized that doing this required "leaking" jackson-databind into the api (@tools.jackson.databind.annotation.JsonDeserialize(builder = DocumentResponse.Builder.class) and @tools.jackson.databind.annotation.JsonPOJOBuilder(withPrefix = "")

That being said, could that be an implementation detail? Since they are annotations, could we support both jackson databind 2.x & 3.x? But then that would violate #23 (comment).

We definitely need to support both 2.x & 3.x, AND I think backwards-compatibility & immutability is super important.

  • I understand the idea behind the design based on interfaces and default implementation, but I'm a bit worried about the effort it would take us to maintain it manually. Existing generators like openapi-generator or openapi-processor don't have support for this kind of design (as far as I know), so even if we make the OpenAPI spec from Docling work, we might not be able to switch to the generated model in a straightforward way. I know that one of the reason for this design is to ensure backward compatibility, so I started a list of requirements in API Design #23 (comment) to help us decide on the final design. I have a hard time imagining situations where, as a library author, I would want to implement the interfaces myself for the data models (typically, it's the client interface that I would customise). But if that's a use case we want to support, let's add it to the list of requirements. What do you think? @edeandrea @lordofthejars

I agree with what you've said. In a perfect world I'd like to have everything, so maybe we need to prioritize our list of requirements? We all know that software engineering is mostly about understanding that there isn't a "perfect" solution to anything - its about knowing & evaluating trade-offs. I think we need to prioritize our requirements by importance. I've taken a stab at what I think.

Maybe the best solution here is to go back to simple Java Pojos for the model classes and stop trying to be "cute" about it? The code isn't as clean and it allows for mutability, but it also allows for the most customization/generalization?

@lordofthejars
Copy link

Maybe we can start with POJOs and if we see taht the project got a high level of adoption, that lot of different Java frameworks are adopting and so on, then we can always refactor.

@edeandrea edeandrea force-pushed the api-design branch 2 times, most recently from 24a7767 to 6029285 Compare October 27, 2025 18:56
Signed-off-by: Eric Deandrea <eric.deandrea@ibm.com>
Signed-off-by: Eric Deandrea <eric.deandrea@ibm.com>
Signed-off-by: Eric Deandrea <eric.deandrea@ibm.com>
Support both Jackson 2 & Jackson 3

Signed-off-by: Eric Deandrea <eric.deandrea@ibm.com>
Support both Jackson 2 & Jackson 3

Signed-off-by: Eric Deandrea <eric.deandrea@ibm.com>
@edeandrea
Copy link
Contributor Author

edeandrea commented Oct 28, 2025

@ThomasVitale @lordofthejars I've found a way to support both Jackson 2 & 3 in the API & client

Since they are annotations, I can have both Jackson 2 & 3 at the API level. Then at the client level I can support both 2 & 3 through a Factory interface, which can detect which client to give based on whats on the classpath.

Support both Jackson 2 & Jackson 3

Signed-off-by: Eric Deandrea <eric.deandrea@ibm.com>
Support both Jackson 2 & Jackson 3

Signed-off-by: Eric Deandrea <eric.deandrea@ibm.com>
Support both Jackson 2 & Jackson 3

Signed-off-by: Eric Deandrea <eric.deandrea@ibm.com>
Convert all to POJOs

Signed-off-by: Eric Deandrea <eric.deandrea@ibm.com>
@edeandrea
Copy link
Contributor Author

@ThomasVitale @lordofthejars @oscerd I've gone ahead and converted everything to simple POJOs, removing all the builders completely.

It isn't as clean, but like @ThomasVitale mentioned at some point we may want to try and auto-generate these somehow, so I wanted to keep them as simple as possible.

@edeandrea edeandrea marked this pull request as ready for review October 29, 2025 17:39
@lordofthejars
Copy link

Yes go ahead. For me it is fine, especially to have a first release and start the integration with other projects. Then if sometimes we are able to automate, perfect, and if not, well it is a valid approach after all

Rename modules

Signed-off-by: Eric Deandrea <eric.deandrea@ibm.com>
@edeandrea
Copy link
Contributor Author

I also thought that as part of this we should clean up folder/project names a bit

api -> docling-api
client -> docling-serve-client
testing -> docling-testing
testcontainers -> docling-testcontainers

The reason for docling-serve-client is that I envision (or I would like to envision :) ) an alternate implementation which uses Docling directly (either via the cli, GraalPy, or some other way).

Fix build

Signed-off-by: Eric Deandrea <eric.deandrea@ibm.com>
@ThomasVitale
Copy link
Contributor

I like and agree with the plan of starting with simple POJOs. It's a good foundation and should also make it less complicated to automate the generation in the future.

I also like the new naming strategy for the modules. Besides supporting more use cases in the future, it's cleaner to have the folder and the Maven module named the same.

Tomorrow, I'll have a better look at the PR and complete the review.

@edeandrea
Copy link
Contributor Author

edeandrea commented Oct 30, 2025

I like and agree with the plan of starting with simple POJOs. It's a good foundation and should also make it less complicated to automate the generation in the future.

I also like the new naming strategy for the modules. Besides supporting more use cases in the future, it's cleaner to have the folder and the Maven module named the same.

Tomorrow, I'll have a better look at the PR and complete the review.

Here's a crazy thought I just had. I'm really not in love with all the "noise" or the POJO approach, even if it is the best approach.

As much as I hate admitting this to myself, what about if we used something like lombok? We kept it self-contained an unexposed, but it could help make the code we're maintaining very clean and get the benefits of immutability with builders while still having very clean code.

I'm not a lombok fan in general, but this might not be a bad use case for it? We could even keep our own code in src/main/lombok and use delombok to generate the actual sources, that way the sources jar contains the de-lombok'ed version if we wanted to.

@lordofthejars
Copy link

I am not a big fan of Lombok, but given the nature of the project I'd say taht everything that can help us on less maintainance is well welcomed.

@edeandrea
Copy link
Contributor Author

edeandrea commented Oct 30, 2025

I am not a big fan of Lombok, but given the nature of the project I'd say taht everything that can help us on less maintainance is well welcomed.

Me neither, but as an experiment I took the HealthCheckResponse class, which looks like this:

package ai.docling.api.health;

import org.jspecify.annotations.Nullable;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;

@JsonInclude(JsonInclude.Include.NON_EMPTY)
public class HealthCheckResponse {
  @JsonProperty("status")
  private String status;

  @Nullable
  public String getStatus() {
    return status;
  }

  public void setStatus(@Nullable String status) {
    this.status = status;
  }

  public HealthCheckResponse withStatus(@Nullable String status) {
    setStatus(status);
    return this;
  }

  @Override
  public String toString() {
    return "HealthCheckResponse{" +
        "status=" + (status == null ? "null" : "'" + status + "'") +
        '}';
  }
}

Using lombok (& not exposing it outside of the api module), we could just have this in our source tree (& it supports both Jackson 2 & 3)

package ai.docling.api.health;

import org.jspecify.annotations.Nullable;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;

import ai.docling.api.health.HealthCheckResponse.Builder;

@JsonInclude(JsonInclude.Include.NON_EMPTY)
@tools.jackson.databind.annotation.JsonDeserialize(builder = Builder.class)
@lombok.extern.jackson.Jacksonized
@lombok.Builder(toBuilder = true)
@lombok.Getter
@lombok.ToString
public class HealthCheckResponse {
  @JsonProperty("status")
  @Nullable
  private String status;

  @tools.jackson.databind.annotation.JsonPOJOBuilder(withPrefix = "")
  public static class Builder { }
}

And this is what would end up in the jar file:

package ai.docling.api.health;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonInclude.Include;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;
import org.jspecify.annotations.Nullable;

@JsonInclude(Include.NON_EMPTY)
@JsonDeserialize(
  builder = Builder.class
)
@tools.jackson.databind.annotation.JsonDeserialize(
  builder = Builder.class
)
public class HealthCheckResponse {
  @JsonProperty("status")
  private @Nullable String status;

  HealthCheckResponse(@Nullable String status) {
    this.status = status;
  }

  public static Builder builder() {
    return new Builder();
  }

  public Builder toBuilder() {
    return (new Builder()).status(this.status);
  }

  public @Nullable String getStatus() {
    return this.status;
  }

  public String toString() {
    return "HealthCheckResponse(status=" + this.getStatus() + ")";
  }

  @JsonPOJOBuilder(
    withPrefix = ""
  )
  @tools.jackson.databind.annotation.JsonPOJOBuilder(
    withPrefix = ""
  )
  public static class Builder {
    private String status;

    Builder() {
    }

    @JsonProperty("status")
    public Builder status(@Nullable String status) {
      this.status = status;
      return this;
    }

    public HealthCheckResponse build() {
      return new HealthCheckResponse(this.status);
    }

    public String toString() {
      return "HealthCheckResponse.Builder(status=" + this.status + ")";
    }
  }
}

If new attributes were needed then we'd just have to add the attributes to the class. Lombok would take care of everything else.

@lordofthejars
Copy link

I'd say go ahead, let's keep things simple for now, moreover, I thikn we should monitor the number of downloads and if we see there is a massive adoption, then, let's figure out to make things more beauty

Use lombok

Signed-off-by: Eric Deandrea <eric.deandrea@ibm.com>
@edeandrea
Copy link
Contributor Author

I went ahead and did the lombok thing across the API. It fully supports jackson 2 & 3 nor does it leak any Lombok things outside of the API.

It has a nice builder approach and it uses immutable objects

@ThomasVitale
Copy link
Contributor

ThomasVitale commented Oct 31, 2025

I'm not a fan of Lombok either, but I understand the context and see the value in this case :)

@edeandrea
Copy link
Contributor Author

@ThomasVitale are you good with this approach and everything I've done here?

- client
- testing
- testcontainers
- docling-api
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the idea is to possibly have a docling-client module in the future using Docling directly (no Docling Serve like in docling-serve-client), I wonder if we should similarly have a docling-serve-api module (for all the code currently in docling-api) and a docling-api module that would only include the Docling core APIs (e.g. Document). In alternative, single module, but two sub packages (core, serve)?

Copy link
Contributor Author

@edeandrea edeandrea Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I've totally changed this comment after thinking about it a bit.

That might not be a bad idea. We can just rename docling-api to docling-serve-api for now. We can build a placeholder for docling-api (or introduce it later on).

Copy link
Contributor Author

@edeandrea edeandrea Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ThomasVitale i've gone ahead and restructured this. I created a new top-level folder docling-serve and moved both docling-serve-api (formerly docling-api) and docling-serve-client into this folder.

I also changed the package structure for docling-serve-api (formerly docling-api) to ai.docling.api.serve (formerly ai.docling.api).

See dfa6755

@ThomasVitale
Copy link
Contributor

@edeandrea I reviewed API and Testcontainers modules, and it all looks good to me. Only had the one comment about the naming of the API module/namespace

I'm still going through the Client module changes.

@edeandrea edeandrea changed the title feat: idea for api design feat: initial api & client implementation Nov 4, 2025
@edeandrea
Copy link
Contributor Author

@ThomasVitale are you ok if I go ahead and merge this? Since it involves directory/name changes I'd like to get this in as I'm working on other things (version tester, release automation, etc) that will be impacted by this.

We can always improve it as we move along.

@ThomasVitale
Copy link
Contributor

Yes, let's merge this!

@edeandrea edeandrea merged commit 3586b97 into docling-project:main Nov 4, 2025
16 checks passed
@edeandrea edeandrea deleted the api-design branch November 4, 2025 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Client design API Design

4 participants