Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is this Java/Swift SDK incomplete compared to JS SDK? #21

Open
zahoork opened this issue May 21, 2018 · 16 comments
Open

Is this Java/Swift SDK incomplete compared to JS SDK? #21

zahoork opened this issue May 21, 2018 · 16 comments

Comments

@zahoork
Copy link

zahoork commented May 21, 2018

Hi,

The Java SDK and Swift SDK provided by the IAB looks incomplete to me compared to what they offer via JS SDK.
If you look at the SDK e.g. Java SDK all it offers is the ConsentStringParser(byte[] bytes), and this takes byte stream as the input.
It’s much more via the JS SDK, where you provide the vendor list, vendors allowed and purposes allowed. So are the APP developers supposed to use other API in APPs even to generate the byte stream that is needed towards ConsentStringParser.

Regards,
Zahoor

@chrispaterson
Copy link
Collaborator

@zahoork my thoughts were that Java would be used primarily to just read values coming from consent strings created by CMPs. So the JS SDK got writing functions added to it. It seems, however, that there is a growing need to support setting consent string values especially in android mobile contexts. We're trying to figure out the best way to handle this, whether it is splitting out a separate Android Consent String SDK or to have both side-by-side in this repo.

@zahoork
Copy link
Author

zahoork commented May 22, 2018

Thanks chris for your response. We e.g as publishers(who act as CMP as well) want this SDK to generate the consent string as well based on consents related to each vendor in Global vendor List/or publisher specific vendors so that we can make the consent string available to other SDKs/Adversting SDK's etc. This is exactly how we use the Javascript SDK as well, which takes the consents from vendors, purposes etc and generates the consent strings for us. So only to have consent parsing/validation part in the SDK won't help. Accordingly please add this missing part to both Java and Swift SDK's.

Regards,
Zahoor

@chrispaterson
Copy link
Collaborator

This is good feedback thank you.

@lanusau
Copy link
Collaborator

lanusau commented Jun 7, 2018

There is a VendorConsentBuilder in the version 2.0.0 which may be what you need

@jsuontaus
Copy link

@lanusau From Android perspective, the problem in VendorConsentBuilder in the version 2.0.0 is that it is dependent about Instant class which is not available until api level 26. Any plans for supporting older Android version as well?

@lanusau
Copy link
Collaborator

lanusau commented Jun 11, 2018

Android support is an open issue. There was a suggestion on the other thread that maybe we need to fork this and create another project for Android. While it may seem somewhat easy to adjust code here to make it compatible with Android initially, the truth is that a lot of backend Java programmers are not familiar with Android constraints, Java 8 is ancient history for us, we're already looking to Java 9 and 10 and beyond. So its inevitable that some code eventually will make it this project that will break android compatibility. So I think having separate project for Android would be more stable?

@ChrisKula
Copy link

The main issue is that Android supports mainly Java 1.7 but support for 1.8 exists to some extent.

For example, lambdas, methods references are backported to old versions of Android but other APIs such as Streams are not available for versions of Android lower than 7.0 (roughly only 37% of devices that visited the Google Play Store).

I started working on a PR that would backport this SDK to 1.7 but I'm not sure if it's the right thing to do as it introduces API breaking changes and also, it would be lowering this SDK to the lowest common denominator (ie Android).

Splitting this SDK in two separate projects (one for pure Java 8/9/10 and one for Android Java 1.7) means supporting two versions of basically the same thing and I'm not sure you want to do that.

@lanusau
Copy link
Collaborator

lanusau commented Jun 22, 2018

My vote is for having separate project for Android. If that seems like an overkill, then we need an automated way to test that build will work on Android.

@zahoork
Copy link
Author

zahoork commented Jun 23, 2018

Right now for publishers this SDK is useless unless Android devices are supported(and atleast from android 5.0), and we are eagerly waiting for Android support before we start to use this SDK. Therefore we also propose having a separate Repo for Android support.

@martinbonnin
Copy link
Contributor

So far, I've bumped into Streams, lambdas, method references and java.time.Instant requiring java 8.

I agree with @ChrisKula that having a single repo is better in terms of maintenance.

Regarding @lanusau remark that backend developers might continue pushing java 8 code, I guess this can be prevented by forcing sourceTargetCompatibility=1.7 in the gradle file.

@lanusau
Copy link
Collaborator

lanusau commented Jul 20, 2018

I don't have environment to compile and test this code for Android. So we need a volunteer to backport this code to java 1.7 and test in Android environment.

@tszpinda
Copy link

tszpinda commented Sep 5, 2018

I had a stub at this one and have created PR #39

@jawadst
Copy link

jawadst commented Sep 5, 2018

@lanusau @ChrisKula @jsuontaus I have a working version for Android here: https://github.com/didomi/Consent-String-SDK-Android

cc @jenniferIAB

@lanusau
Copy link
Collaborator

lanusau commented Sep 11, 2018

I have reviewed PR #39 and its ready to merge. However, if we are going to have official Consent-String-SDK-Android, then it would make better sense to leave this library as it is, because PR #39 changes signatures of some methods (from Instant to Date) and would very likely require people using this library to update their code after upgrading to new version.
@tszpinda - does Consent-String-SDK-Android code above work for you?
@jenniferIAB - I wonder what are your thoughts of having Consent-String-SDK-Android alongside this library?

@jawadst
Copy link

jawadst commented Sep 11, 2018

PR #39 is not necessarily enough to make it Android-compatible as it's hard to guarantee that the APIs used are compatible with Android unless you build for Android specifically. The project needs to include building an Android library as well which is what https://github.com/didomi/Consent-String-SDK-Android does (it uses the exact same code as the Java version except that the old main Vendor class was removed to only keep the most recent API).
I think it's overall easier to keep the Android version separate for packaging and distribution while making sure both are in sync by porting Java updates to a dedicated Android repo.

@martinbonnin
Copy link
Contributor

@jawadst, comments below

it's hard to guarantee that the APIs used are compatible with Android unless you build for Android specifically

Do you have an exemple ? Java 7 should work out of the box for android

The project needs to include building an Android library

What do you mean ? Android doesn't require a .aar. Plain jars work fine.

Overall, forking always has a cost. In addition to maintenance (having to backport), it also makes it less easy to find the correct repo. I would like it better if there could be only one repo. The use of Date instead of Instant in a breaking change but nothing too complicated to change for users.

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

No branches or pull requests

8 participants