Prepare Android Header

[cdr-chakotay]

Prepare sending of additional SDK information with the Transloadit Client Header.
e.G. if you are using the Android SDK.
See issue in the Android SDK

[Acconut]

Thank you for the quick PR! This is a bit more complex than I anticipated, in particular the validation part. I am wondering if we actually need that. Wouldn't it be sufficient to allow setting a string, which then gets appended to the Transloadit-Client header?

The implementation I had in mind when we talked about it, looks a bit like this:

• The Transloadit constructor parses the version.properties file obtain the current version (we can simply move this code from the Request class into Transloadit for that:

  java-sdk/src/main/java/com/transloadit/sdk/Request.java

  Lines 57 to 66 in 34a6f48

  	InputStream in = getClass().getClassLoader().getResourceAsStream("version.properties");
  	try {
  	prop.load(in);
  	version = "java-sdk:" + prop.getProperty("versionNumber").replace("'", "");
  	in.close();
  	} catch (IOException e) {
  	throw new RuntimeException(e);
  	} catch (NullPointerException npe) {
  	version = "java-sdk:unknown";
  	}

  In general, I don´t think the Request class is the correct spot for this)
• The Transloadit class also has an appendVersionInformation(String) (or something similar) were the Android SDK can append its version.
• The Transloadit class also has a String getVersionInformation() method, which is used by the Request class to get the full value for Transloadit-Client.
• We can also think about making the last two methods protected, so not everyone but rather the AndroidTransloadit subclass can change that information. It's not fully protected against modifications that way, but this is not important.

[cdr-chakotay]

Thank you for the quick PR! This is a bit more complex than I anticipated, in particular the validation part. I am wondering if we actually need that. Wouldn't it be sufficient to allow setting a string, which then gets appended to the Transloadit-Client header?

The implementation I had in mind when we talked about it, looks a bit like this:

• The Transloadit constructor parses the version.properties file obtain the current version (we can simply move this code from the Request class into Transloadit for that:

  java-sdk/src/main/java/com/transloadit/sdk/Request.java

  Lines 57 to 66 in 34a6f48

  	InputStream in = getClass().getClassLoader().getResourceAsStream("version.properties");
  	try {
  	prop.load(in);
  	version = "java-sdk:" + prop.getProperty("versionNumber").replace("'", "");
  	in.close();
  	} catch (IOException e) {
  	throw new RuntimeException(e);
  	} catch (NullPointerException npe) {
  	version = "java-sdk:unknown";
  	}

  
  In general, I don´t think the Request class is the correct spot for this)
• The Transloadit class also has an appendVersionInformation(String) (or something similar) were the Android SDK can append its version.
• The Transloadit class also has a String getVersionInformation() method, which is used by the Request class to get the full value for Transloadit-Client.
• We can also think about making the last two methods protected, so not everyone but rather the AndroidTransloadit subclass can change that information. It's not fully protected against modifications that way, but this is not important.

Sounds good, I am not done anyway. So I will try to follow your suggestions :)
Regarding the validation part: Yes it is a little more complicated, but it preserves some uniformity :)

[cdr-chakotay]

We may need to release this in order to use it in the Android-SDK

[Acconut]

Apologies for my delayed response! I just had another idea how we could simplify this version handling even further: What if, instead of having a getter/setter for additionalTransloaditClientHeaderContent, we only have the protected loadVersionInfo method. The Android SDK then overwrites this method to include its version information. Something like this:

// Java SDK
class Transloadit {
  private String versionInfo;
  
  constructor() {
    this.versionInfo = loadVersionInfo();
  }
  
  prototected String loadVersionInfo() {
    // Load from version.properties file...
    return ...;
  }
  
  // This method is package-private so only the Request class can access it.
  String getVersionInfo() {
    return this.versionInfo;
  }
}

class Request {
  constructor() {
    transloadit.getVersionInfo();
    ...
  }
}

// Android SDK
class AndroidTransloadit extends Transloadit {
  @Overwrite
  protected String loadVersionInfo() {
    // Load from version.properties for Android SDK...
    // Combine version from parent class and from Android SDK
    return super.loadVersionInfo() + androidVersionInfo;
  }
}

Hopefully that approach is understandable for you. The benefit is that we have no getter/setters (no code is the best code) and the version.properties files are only accessed once when the client is initialized and not for every request being sent.

In addition, I would remove the version number validation as it could pose a problem in the future. Imagine we want to make a pre-release version v1.2.3-pre1 for the Android SDK. The validation will not accept this and throw an error, causing the entire SDK to not work.

What do you think?

[cdr-chakotay]

Can do this :) sounds good !

I'll try that by chance

[cdr-chakotay]

@Acconut have implemented it according to the proposal.
You can find the corresponding Android SDK counterpart here:
https://github.com/transloadit/android-sdk/pull/11

[Acconut]

Awesome work! Thanks for the quick turnaround! The test is failing. Could you briefly look into that? If we determine it unrelated or you fixed it, we can merge this baby :)

[cdr-chakotay]

I am willing to merge this and release it as 0.4.2 :) Than we can add this dependency to the android SDK as well and are done with the headers 👍🏻 @Acconut

[Acconut]

Awesome work! Thank you very much!