Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 20 additions & 57 deletions cloudinary-core/src/main/java/com/cloudinary/Cloudinary.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import com.cloudinary.api.signing.NotificationRequestSignatureVerifier;
import com.cloudinary.strategies.AbstractApiStrategy;
import com.cloudinary.strategies.AbstractUploaderStrategy;
import com.cloudinary.strategies.StrategyLoader;
import com.cloudinary.utils.Analytics;
import com.cloudinary.utils.ObjectUtils;
import com.cloudinary.utils.StringUtils;
Expand All @@ -18,15 +17,7 @@
import static com.cloudinary.Util.buildMultiParams;

@SuppressWarnings({"rawtypes", "unchecked"})
public class Cloudinary {

private static List<String> UPLOAD_STRATEGIES = new ArrayList<String>(Arrays.asList(
"com.cloudinary.android.UploaderStrategy",
"com.cloudinary.http5.UploaderStrategy"));
public static List<String> API_STRATEGIES = new ArrayList<String>(Arrays.asList(
"com.cloudinary.android.ApiStrategy",
"com.cloudinary.http5.ApiStrategy"));

public abstract class Cloudinary {
public final static String CF_SHARED_CDN = "d3jpl91pxevbkh.cloudfront.net";
public final static String OLD_AKAMAI_SHARED_CDN = "cloudinary-a.akamaihd.net";
public final static String AKAMAI_SHARED_CDN = "res.cloudinary.com";
Expand All @@ -37,8 +28,8 @@ public class Cloudinary {
public final static String USER_AGENT_JAVA_VERSION = "(Java " + System.getProperty("java.version") + ")";

public final Configuration config;
private AbstractUploaderStrategy uploaderStrategy;
private AbstractApiStrategy apiStrategy;
private final AbstractUploaderStrategy uploaderStrategy;
private final AbstractApiStrategy apiStrategy;
private String userAgent = USER_AGENT_PREFIX+"/"+ VERSION + " "+USER_AGENT_JAVA_VERSION;
public Analytics analytics = new Analytics();
public Uploader uploader() {
Expand All @@ -57,51 +48,10 @@ public SearchFolders searchFolders() {
return new SearchFolders(this);
}

public static void registerUploaderStrategy(String className) {
if (!UPLOAD_STRATEGIES.contains(className)) {
UPLOAD_STRATEGIES.add(className);
}

}

public static void registerAPIStrategy(String className) {
if (!API_STRATEGIES.contains(className)) {
API_STRATEGIES.add(className);
}
}

private void loadStrategies() {
if (!this.config.loadStrategies) return;
uploaderStrategy = StrategyLoader.find(UPLOAD_STRATEGIES);

if (uploaderStrategy == null) {
throw new UnknownError("Can't find Cloudinary platform adapter [" + StringUtils.join(UPLOAD_STRATEGIES, ",") + "]");
}

apiStrategy = StrategyLoader.find(API_STRATEGIES);
if (apiStrategy == null) {
throw new UnknownError("Can't find Cloudinary platform adapter [" + StringUtils.join(API_STRATEGIES, ",") + "]");
}
}

public Cloudinary(Map config) {
this.config = new Configuration(config);
loadStrategies();
}

public Cloudinary(String cloudinaryUrl) {
this.config = Configuration.from(cloudinaryUrl);
loadStrategies();
}

public Cloudinary() {
String cloudinaryUrl = System.getProperty("CLOUDINARY_URL", System.getenv("CLOUDINARY_URL"));
if (cloudinaryUrl != null) {
this.config = Configuration.from(cloudinaryUrl);
} else {
this.config = new Configuration();
}
loadStrategies();
public Cloudinary(Configuration config, AbstractUploaderStrategy uploaderStrategy, AbstractApiStrategy apiStrategy) {
this.config = config;
this.apiStrategy = apiStrategy;
this.uploaderStrategy = uploaderStrategy;
}

public Url url() {
Expand Down Expand Up @@ -395,4 +345,17 @@ private String buildUrl(String base, Map<String, Object> params) throws Unsuppor
}
return urlBuilder.toString();
}

/**
* Attempts to load a cloudinary config from a URL present in the Environment Property "CLOUDINARY_URL".
* If no property is set in this JVM then a default config is returned.
*/
protected static Configuration loadDefaultConfig() {
String cloudinaryUrl = System.getProperty("CLOUDINARY_URL", System.getenv("CLOUDINARY_URL"));
if (cloudinaryUrl != null) {
return Configuration.from(cloudinaryUrl);
} else {
return new Configuration();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package com.cloudinary.legacy;

import com.cloudinary.Cloudinary;
import com.cloudinary.Configuration;
import com.cloudinary.strategies.AbstractApiStrategy;
import com.cloudinary.strategies.AbstractUploaderStrategy;
import com.cloudinary.strategies.StrategyLoader;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

/**
* Allows for construction of the abstract Cloudinary class using the legacy method of loading classes dynamically
* using reflection. This is needed to support the cloudinary-taglib library.
*
* @since some version number
* <p>
* Replaced by {@link com.cloudinary.http5.CloudinaryClient}
* @deprecated this method of instantiation is deprecated and not recommended for new projects.
*/
@Deprecated
public class LegacyCloudinaryLoader extends Cloudinary {
private static final List<String> UPLOAD_STRATEGIES = new ArrayList<>(Arrays.asList(
"com.cloudinary.android.UploaderStrategy",
"com.cloudinary.http5.UploaderStrategy"));
public static final List<String> API_STRATEGIES = new ArrayList<>(Arrays.asList(
"com.cloudinary.android.ApiStrategy",
"com.cloudinary.http5.ApiStrategy"));


public LegacyCloudinaryLoader() {
super(loadDefaultConfig(), getUploaderStrategy(loadDefaultConfig()), getApiStrategy(loadDefaultConfig()));
}

public LegacyCloudinaryLoader(Map<String, Object> configuration) {
super(new Configuration(configuration), getUploaderStrategy(new Configuration(configuration)), getApiStrategy(new Configuration(configuration)));
}

public LegacyCloudinaryLoader(String url) {
super(Configuration.from(url), getUploaderStrategy(Configuration.from(url)), getApiStrategy(Configuration.from(url)));
}
Comment on lines +33 to +43

@campbellbartlett campbellbartlett Jan 15, 2025 •

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

The load_strategies configuration property introduces unnecessary complexity to these constructors. Due to Java 8's requirement that super() must be the first statement in a constructor, combined with the conditional nature of the legacy strategy loading, we're forced into a more complicated implementation than is ideal.

I think that this property only exists for testing purposes in CloudinaryTest.java. If it were up to me I would consider removing it entirely and restructuring those tests accordingly. This would lead to cleaner, more maintainable constructor logic.


private static AbstractApiStrategy getApiStrategy(Configuration configuration) {
if (!configuration.loadStrategies) {
return null;
}

return StrategyLoader.find(API_STRATEGIES);
}

private static AbstractUploaderStrategy getUploaderStrategy(Configuration configuration) {
if (!configuration.loadStrategies) {
return null;
}

return StrategyLoader.find(UPLOAD_STRATEGIES);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.cloudinary.strategies;

import com.cloudinary.utils.StringUtils;

import java.util.List;

public class StrategyLoader {
Expand All @@ -22,8 +24,7 @@ public static <T> T find(List<String> strategies) {
return strategy;
}
}
return null;

throw new UnknownError("Can't find Cloudinary platform adapter [" + StringUtils.join(strategies, ",") + "]");
}

public boolean exists(List<String> strategies) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.cloudinary;

import com.cloudinary.legacy.LegacyCloudinaryLoader;
import com.cloudinary.utils.ObjectUtils;

import org.hamcrest.CoreMatchers;
Expand Down Expand Up @@ -30,7 +31,7 @@ public class AuthTokenTest {
@Before
public void setUp() {
System.out.println("Running " + this.getClass().getName() + "." + currentTest.getMethodName());
this.cloudinary = new Cloudinary("cloudinary://a:b@test123?load_strategies=false&analytics=false");
this.cloudinary = new LegacyCloudinaryLoader("cloudinary://a:b@test123?load_strategies=false&analytics=false");
final AuthToken authToken = new AuthToken(KEY).duration(300);
authToken.startTime(11111111); // start time is set for test purposes
cloudinary.config.authToken = authToken;
Expand Down Expand Up @@ -99,7 +100,7 @@ public void testAuthenticatedUrl() {

@Test
public void testConfiguration() {
cloudinary = new Cloudinary("cloudinary://a:b@test123?load_strategies=false&auth_token[key]=aabbcc112233&auth_token[duration]=200");
cloudinary = new LegacyCloudinaryLoader("cloudinary://a:b@test123?load_strategies=false&auth_token[key]=aabbcc112233&auth_token[duration]=200");
assertEquals(cloudinary.config.authToken, new AuthToken("aabbcc112233").duration(200));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.cloudinary.AuthToken;
import com.cloudinary.Cloudinary;
import com.cloudinary.legacy.LegacyCloudinaryLoader;
import com.cloudinary.utils.Analytics;
import org.junit.*;
import org.junit.rules.TestName;
Expand All @@ -20,7 +21,7 @@ public class AnalyticsTest {
@Before
public void setUp() {
System.out.println("Running " + this.getClass().getName() + "." + currentTest.getMethodName());
this.cloudinary = new Cloudinary("cloudinary://a:b@test123?load_strategies=false");
this.cloudinary = new LegacyCloudinaryLoader("cloudinary://a:b@test123?load_strategies=false");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.cloudinary.test;

import com.cloudinary.*;
import com.cloudinary.legacy.LegacyCloudinaryLoader;
import com.cloudinary.transformation.*;
import com.cloudinary.utils.ObjectUtils;
import junitparams.JUnitParamsRunner;
Expand Down Expand Up @@ -45,7 +46,7 @@ public class CloudinaryTest {
@Before
public void setUp() {
System.out.println("Running " + this.getClass().getName() + "." + currentTest.getMethodName());
this.cloudinary = new Cloudinary("cloudinary://a:b@test123?load_strategies=false&analytics=false");
this.cloudinary = new LegacyCloudinaryLoader("cloudinary://a:b@test123?load_strategies=false&analytics=false");
}

@Test
Expand Down Expand Up @@ -726,7 +727,7 @@ public void testDownloadSprite() throws Exception{

@Test
public void testDownloadMulti() throws Exception{
cloudinary = new Cloudinary("cloudinary://571927874334573:yABWqlfSV2d5pRW4ujHJYA7SD34@nitzanj?load_strategies=false");
cloudinary = new LegacyCloudinaryLoader("cloudinary://571927874334573:yABWqlfSV2d5pRW4ujHJYA7SD34@nitzanj?load_strategies=false");

final String multiTestTag = "multi_test_tag";
final String url1 = "https://res.cloudinary.com/demo/image/upload/sample";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.cloudinary.Cloudinary;
import com.cloudinary.Transformation;
import com.cloudinary.legacy.LegacyCloudinaryLoader;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
Expand All @@ -19,7 +20,7 @@ public class LayerTest {

@Before
public void setUp() {
this.cloudinary = new Cloudinary("cloudinary://a:b@test123?load_strategies=false&analytics=false");
this.cloudinary = new LegacyCloudinaryLoader("cloudinary://a:b@test123?load_strategies=false&analytics=false");
}

@After
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.cloudinary.utils.ObjectUtils;
import org.apache.hc.client5.http.classic.methods.*;
import org.apache.hc.client5.http.entity.UrlEncodedFormEntity;
import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient;
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse;
import org.apache.hc.client5.http.impl.classic.HttpClients;
Expand All @@ -34,17 +35,23 @@ public class ApiStrategy extends AbstractApiStrategy {

private static final String APACHE_HTTP_CLIENT_VERSION = System.getProperty("apache.http.client.version", "5.3.1");

private CloseableHttpClient client;

@Override
public void init(Api api) {
super.init(api);
private final CloseableHttpClient client;

public ApiStrategy() {
this.client = HttpClients.custom()
.setUserAgent(this.api.cloudinary.getUserAgent() + " ApacheHttpClient/" + APACHE_HTTP_CLIENT_VERSION)
.build();
}

public ApiStrategy(CloseableHttpClient client) {
this.client = client;
}

@Override
public void init(Api api) {
super.init(api);
}

@SuppressWarnings({"rawtypes", "unchecked"})
public ApiResponse callApi(Api.HttpMethod method, String apiUrl, Map<String, ?> params, Map options, String autorizationHeader) throws Exception {
HttpUriRequestBase request = prepareRequest(method, apiUrl, params, options);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.cloudinary.http5;

import com.cloudinary.Cloudinary;
import com.cloudinary.Configuration;
import com.cloudinary.strategies.AbstractApiStrategy;
import com.cloudinary.strategies.AbstractUploaderStrategy;
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;

import java.util.Map;

public class CloudinaryClient extends Cloudinary {
public CloudinaryClient() {
super(loadDefaultConfig(), new UploaderStrategy(), new ApiStrategy());
}

public CloudinaryClient(Configuration config, AbstractUploaderStrategy uploaderStrategy, AbstractApiStrategy apiStrategy) {
super(config, uploaderStrategy, apiStrategy);
}

public CloudinaryClient(Map config) {
super(new Configuration(config), new UploaderStrategy(), new ApiStrategy());
}

public CloudinaryClient(String url) {
super(Configuration.from(url), new UploaderStrategy(), new ApiStrategy());
}

public CloudinaryClient(Configuration configuration, CloseableHttpClient httpClient) {
super(configuration, new UploaderStrategy(httpClient), new ApiStrategy(httpClient));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,23 @@ public class UploaderStrategy extends AbstractUploaderStrategy {

private static final String APACHE_HTTP_CLIENT_VERSION = System.getProperty("apache.http.client.version", "5.3.1");

private CloseableHttpClient client;
private final CloseableHttpClient client;

@Override
public void init(Uploader uploader) {
super.init(uploader);
public UploaderStrategy(CloseableHttpClient client) {
this.client = client;
}

public UploaderStrategy() {
this.client = HttpClients.custom()
.setUserAgent(cloudinary().getUserAgent() + " ApacheHttpClient/" + APACHE_HTTP_CLIENT_VERSION)
.build();
}

@Override
public void init(Uploader uploader) {
super.init(uploader);
}

@SuppressWarnings({"rawtypes", "unchecked"})
@Override
public Map callApi(String action, Map<String, Object> params, Map options, Object file, ProgressCallback progressCallback) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.cloudinary;

import com.cloudinary.legacy.LegacyCloudinaryLoader;

/**
* This class contains a singleton in a generic way. This class is used by the tags to
* retrieve the Cloudinary configuration.
Expand All @@ -24,7 +26,7 @@ public static void deregisterCloudinary() {
}

private static class DefaultCloudinaryHolder {
public static final Cloudinary INSTANCE = new Cloudinary();
public static final Cloudinary INSTANCE = new LegacyCloudinaryLoader();
}

public static Cloudinary getCloudinary() {
Expand Down
Loading