Skip to content

Conversation

@rpanackal
Copy link
Member

Context

AI/ai-sdk-java-backlog#239.

Since RestTemplate is missing a built-in handler for HttpMessage to Fileconverison, calls to export (download) endoint fails.

Feature scope:

  • Add a FileHttpMessageConverter
  • Test and verify export endpoint fix

Definition of Done

  • Functionality scope stated & covered
  • Tests cover the scope above
  • Error handling created / updated & covered by the tests above
  • Aligned changes with the JavaScript SDK
  • Documentation updated
  • Release notes updated

@rpanackal
Copy link
Member Author

rpanackal commented Nov 21, 2025

Alternative Considered

  • We can set the following configuration in the pom.xml. (After merging: feat: [OpenAPI] Type and Import mapping support cloud-sdk-java#1010 )

    <typeMappings>
      <typeMapping>File=byte[]</typeMapping> // replace all files with bytes arrays
    </typeMappings>
  • The above config will fix the export methods but break the import methods as its internal logic doesnt compile with byte[] input.

    We can resolve that by enabling the following

    <additionalProperties>
      <useAbstractionForFiles>true</useAbstractionForFiles> // use Resource class instead of File in request classes
      <!-- ....  -->
    </additionalProperties>
  • When useAbstractionForFiles=true, all methods that accepts File as request parameter, will instead expect org.springframework.core.io.Resource.

I discarded the approach due to the side effect that a Spring class gets bound to our public API. Nonetheless, it's still a viable alternative to adding a FileHttpMessageConverter. So, if you have an opinion or suggestion, let me know..

Comment on lines +66 to +71
// export
final var exportedTemplate = controller.exportTemplate();

final var resource = new ClassPathResource("prompt-template.yaml");
final JsonNode expectedYaml = YAML_MAPPER.readTree(resource.getContentAsString(UTF_8));
assertThat(YAML_MAPPER.readTree(exportedTemplate)).isEqualTo(expectedYaml);
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this?

Copy link
Member Author

@rpanackal rpanackal Nov 21, 2025

Choose a reason for hiding this comment

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

The prompt-template.yaml is uploaded with importTemplate() method (part of same test) and then downloaded when exportTemplate() is invoked. Direct String comparison fails as yaml file's by nature don't have order to the properties defined in it. Consequently, returned file has a different order of properties and can't be directly compared without parsing. (Similar to json comparison)

I can also instead resort to checking existence of some string within the returned file content. I just went for a complete equivalence check.

protected File readInternal(
@Nonnull final Class<? extends File> clazz, @Nonnull final HttpInputMessage inputMessage)
throws IOException, HttpMessageNotReadableException {
final var tempFile = File.createTempFile("tmp", ".bin");
Copy link
Contributor

Choose a reason for hiding this comment

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

(Question)

Is there life-cycle in Temp File? Do we need to delete this file? Does it exist physically on disk - or virtually in memory?

Copy link
Member Author

@rpanackal rpanackal Nov 21, 2025

Choose a reason for hiding this comment

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

File.createTempFile() creates a physical file on disk in OS's temporary directory. My understanding was that if a File object is returned there needs to be a physical file it references.

Copy link
Member Author

@rpanackal rpanackal Nov 21, 2025

Choose a reason for hiding this comment

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

We cannot delete the File and its upto the caller of PromptTemplateApi.exportPromptTemplate() to do so.

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.

3 participants