Make usage of http clients (apache, OkHttp, Jetty, Asynch) easier

Open
  • Is something wrong?
#343 6 comments 0 reactions 0 assignees View on GitHub

@ManishS6 is already working on this.

Since Oct 4, 2024.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
25/100
Issue type
Feature
Clarity
Needs clarification
Activity status
Stale
Tech stack
java
Domain
api

Research direction

Start by reading Cloudinary.loadStrategies(), Cloudinary.class, and Configuration.class to understand the current strategy loading and Map-based configuration. Review how Apache, OkHttp, Jetty, and Asynch strategies are selected and how configuration reaches the constructor. Done should provide a decided, documented way for consumers to select HTTP clients and configure them without an opaque Map.

Written by the indexing model from the issue text.

Description

Feature request for Cloudinary Java SDK

General idea: making the settings of http client library to be used easier for consumers of the SDK,
by enabling configuration via new Cloudinary(...) constructor.

Explain your use case

I want to use plain java http client, instead of Apache one.

Describe the problem you’re trying to solve

In Cloudinary.loadStrategies() there's a fixed list of strings with classnames, which a consumer of the SDK can't change.
This means even if I provide my own strategy, this will not loaded and I still being forced to add dependency for apache http.
(hint: technically: when the class om.cloudinary.http5.ApiStrategy is not found, an UnknownError is thrown, which is bad)

Also this brings me to another point: the constructor of Cloudinary should better accept Configuration object, instead of Map config. It's very annoying for me as an SDK user, to provide a Map with configs.
I get no auto-completion on Maps and must find details in either source code or documentation (which is just cumbersome).
So I don't know immediately what config options are available, that I could e.g. provide HTTP proxy address.

(Sorry, if I mix two problems here ... I came to these two, when I thought about a solution proposal, below)

Do you have a proposed solution?

YES.
The constructor of Cloudinary.class (or better the Configuration.class) could have some new field apiStrategy of type class<AbstractApiStrategy>. And another one for the UploadStrategy.
Then, the loadStragies() method will not load by name (which can contain typos), but rather by class(type) directly.

On the second problem, adding a new (overloaded) constructor to Cloudinary is easy to add, but that's not sufficient.
Ideally, the Configuration class will provide a convenient Builder function, so users of the SDK are better guided,
and plausibility checks can easier be implemented.

I'm happy to provide a PR, if this resonates with you.

Dominant language
Java
Stars
176
Forks
113
Avg merge
9d 20h
Merged PRs (30d)
2

Getting set up

  • No Dockerfile or Docker Compose file
  • Has a pull request template
  • No contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from cloudinary/cloudinary_java

All issues in cloudinary/cloudinary_java

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.