Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
26 changes: 19 additions & 7 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ use sha1::{Digest, Sha1};
use std::path::PathBuf;
use tokio::fs::File;
use tokio_util::codec::{BytesCodec, FramedRead};
use upload::resource_type::ResourceTypes;
use upload::{result::UploadResult, UploadOptions};

pub struct Cloudinary {
Expand Down Expand Up @@ -102,20 +103,21 @@ impl Cloudinary {
/// let cloudinary = Cloudinary::new("api_key".to_string(), "cloud_name".to_string(), "api_secret".to_string() );
/// let result = cloudinary.upload_image(Source::Path("./image.jpg".into()), &options);
/// ```
pub async fn upload_image(
&self,
src: Source,
options: &UploadOptions<'_>,
) -> Result<UploadResult> {

pub async fn upload(&self, src: Source, options: &UploadOptions<'_>) -> Result<UploadResult> {
let client = Client::new();
let file = match src {
Source::Path(path) => prepare_file(&path).await?,
Source::Url(url) => Part::text(url.as_str().to_string()),
};
let multipart = self.build_form_data(options).part("file", file);
let url = format!(
"https://api.cloudinary.com/v1_1/{}/image/upload",
self.cloud_name
"https://api.cloudinary.com/v1_1/{}/{}/upload",
self.cloud_name,
options
.get_resource_type()
Copy link
Owner

@Lurk Lurk Nov 6, 2023

Choose a reason for hiding this comment

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

I don't think upload options can be 100% shared between images, videos, and sound. For example, "faces" are relevant for images only.

Copy link
Author

Choose a reason for hiding this comment

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

From my understanding of the docs, the upload parameters are somewhat general. Perhaps cloudinary's api will accept the valid options based on the resource_type or the specific endpoint called, I'm nor really sure

.unwrap_or(ResourceTypes::Image)
.to_string()
);
let response = client
.post(&url)
Expand All @@ -128,6 +130,16 @@ impl Cloudinary {
Ok(json)
}

pub async fn upload_image(
&self,
src: Source,
options: &UploadOptions<'_>,
) -> Result<UploadResult> {
let new_opts = options.clone();
let updated_options = new_opts.set_resource_type(ResourceTypes::Image);
Copy link
Owner

Choose a reason for hiding this comment

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

If options already contain resource type, we will silently overwrite it, which is a potential bug in a client application. We can catch it on compile time.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I thought since the person is using the upload_image method, then the resource_type will have to be an image whether they set it or not

Copy link
Author

Choose a reason for hiding this comment

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

We can let them know that the overwrite will happen in the docs

Copy link
Owner

Choose a reason for hiding this comment

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

How I image this API:

cloudinary.upload(Options::Image(...image_specific_options...))->UploadResultEnum;

cloudinary.upload_image(...image_specific_options...)->...image_specific_result
cloudinary.upload_audio(...audio_specific_options...)->...audio_specific_result

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I get this. So I've used the official Python SDK, and it doesn't have an upload_audio method, it just has an upload_image and an upload method that can still be used to upload images.

Copy link
Author

Choose a reason for hiding this comment

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

We'll probably need to determine the return type based on the values that are passed in as options. Hmmm.

Copy link
Author

Choose a reason for hiding this comment

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

I'm currently participating in a Hackathon that's ending Wednesday. Please I'll tackle this after.

self.upload(src, &updated_options).await
}

fn build_form_data(&self, options: &UploadOptions) -> Form {
let mut map = options.get_map();
let resource_type = map.remove("resource_type");
Expand Down
53 changes: 43 additions & 10 deletions src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,55 @@ use std::env::var;

use crate::{
upload::result::UploadResult::{Error, Success},
upload::UploadOptions,
upload::{resource_type::ResourceTypes, UploadOptions},
Cloudinary, Source,
};

#[tokio::test]
async fn test_image_upload_from_url() {
struct CloudinaryVariable {
pub api_secret: String,
pub api_key: String,
pub cloud_name: String,
}

fn load_env() -> CloudinaryVariable {
dotenv().ok();
let api_secret = var("CLOUDINARY_API_SECRET").expect("enviroment variables not set");
let api_key = var("CLOUDINARY_API_KEY").expect("enviroment variables not set");
let cloud_name = var("CLOUDINARY_CLOUD_NAME").expect("enviroment variables not set");
CloudinaryVariable {
api_secret,
api_key,
cloud_name,
}
}

#[tokio::test]
async fn test_raw_upload() {
let env_vars = load_env();
let cloudinary = Cloudinary::new(env_vars.api_key, env_vars.cloud_name, env_vars.api_secret);

let audio_url = "https://commons.wikimedia.org/wiki/File:Panama_National_Anthem.ogg";
Copy link
Owner

Choose a reason for hiding this comment

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

I would like to have a small file locally.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah that's fine, I just thought that for the test to run smoothly for everyone that clones the project, a url would work better.

Copy link
Owner

Choose a reason for hiding this comment

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

Unfortunately, URLs tend to change at the most inconvenient moment. We have https://github.com/Lurk/cloudinary_rs/tree/main/assets in the repo.

let public_id = "audio_from_url.ogg";
let options = UploadOptions::new()
.set_resource_type(ResourceTypes::Raw)
.set_public_id(String::from(public_id))
.set_overwrite(true);
let res = cloudinary
.upload(Source::Url(audio_url.try_into().unwrap()), &options)
.await
.unwrap();

match res {
Success(audio) => assert_eq!(audio.public_id, public_id.to_string()),
Error(err) => panic!("{}", err.error.message),
}
}

#[tokio::test]
async fn test_image_upload_from_url() {
let env_vars = load_env();
let cloudinary = Cloudinary::new(env_vars.api_key, env_vars.cloud_name, env_vars.api_secret);

let cloudinary = Cloudinary::new(api_key, cloud_name, api_secret);
let image_url = "https://upload.wikimedia.org/wikipedia/commons/c/ca/1x1.png";
let public_id = "image_upload_from_url";

Expand All @@ -34,12 +71,8 @@ async fn test_image_upload_from_url() {

#[tokio::test]
async fn test_image_upload_from_path() {
dotenv().ok();
let api_secret = var("CLOUDINARY_API_SECRET").expect("enviroment variables not set");
let api_key = var("CLOUDINARY_API_KEY").expect("enviroment variables not set");
let cloud_name = var("CLOUDINARY_CLOUD_NAME").expect("enviroment variables not set");

let cloudinary = Cloudinary::new(api_key, cloud_name, api_secret);
let env_vars = load_env();
let cloudinary = Cloudinary::new(env_vars.api_key, env_vars.cloud_name, env_vars.api_secret);
let image_path = "./assets/1x1.png";
let public_id = "image_upload_from_path";

Expand Down
4 changes: 2 additions & 2 deletions src/upload/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ mod categorizations;
mod data_types;
mod delivery_type;
mod raw_convert;
mod resource_type;
pub mod resource_type;
mod responsive_breakpoints;
pub mod result;

Expand All @@ -19,7 +19,7 @@ use self::{
responsive_breakpoints::ResponsiveBreakpoints,
};

#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct UploadOptions<'entry_key_lifetime> {
inner: BTreeMap<&'entry_key_lifetime str, DataType>,
}
Expand Down
6 changes: 3 additions & 3 deletions src/upload/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ pub struct Response {
pub version: usize,
pub version_id: String,
pub signature: String,
pub width: usize,
pub height: usize,
pub format: String,
pub width: Option<usize>,
pub height: Option<usize>,
pub format: Option<String>,
Copy link
Owner

Choose a reason for hiding this comment

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

Do you know if it is documented somewhere?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah files like audio files, text files etc don't have width and height. There's a an example here

Copy link
Owner

Choose a reason for hiding this comment

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

Instead of optional width params, I would like a more strict result type. If we upload an image, we know the result should have width, height, and format. And if we upload audio, it does not.

pub resource_type: String,
#[serde(deserialize_with = "deserialize_from_str")]
pub created_at: DateTime<Utc>,
Expand Down