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

Nullability and javadocs #293

Merged
merged 16 commits into from
Aug 4, 2017
Merged
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ before_script:
- adb shell input keyevent 82 &

script:
- ./gradlew connectedCheck --project-prop sonatypeUsername=none --project-prop sonatypePassword=none --project-prop sonatypeRepo=none
- ./gradlew connectedCheck

before_cache:
- rm -f $HOME/.gradle/caches/modules-2/modules-2.lock
Expand Down
7 changes: 5 additions & 2 deletions library/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ android {
dependencies {
androidTestCompile 'com.android.support.test:runner:0.5'
androidTestCompile 'com.android.support.test:rules:0.5'
compile 'com.android.support:support-annotations:25.3.1'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am personally very much in support of adding nullability annotations. Useful for us Java users, too!

}

configurations.archives.extendsFrom configurations.default
Expand Down Expand Up @@ -69,8 +70,10 @@ uploadArchives {
repositories.mavenDeployer {
beforeDeployment { MavenDeployment deployment -> signing.signPom(deployment) }

repository(url: sonatypeRepo) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

sonatypeRepo is not found when building. If this is in a properties file that is gitignore, maybe add a check so other users can compile their forks

Copy link
Collaborator

Choose a reason for hiding this comment

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

I ran into the same issue. I end up keeping this in git stash and undoing it when I push a PR. Would be great if the build file were updated to allow others to build without needing to do that.

Copy link
Member

Choose a reason for hiding this comment

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

Create a gradle.properties file and put a blank value there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@serggl if this is in your gradle property file, how about I just add a check to see if the value is defined? It'll be something along the lines of

if property file exists or variable exists in System.env (for travis), then execute the call

Copy link
Member

Choose a reason for hiding this comment

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

True. Can you add it?:)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@serggl done. It looks like you also added fake properties in the travis yml, so I removed those as it won't be necessary

authentication(userName: sonatypeUsername, password: sonatypePassword)
if (project.hasProperty('sonatypeRepo')) {
repository(url: sonatypeRepo) {
authentication(userName: sonatypeUsername, password: sonatypePassword)
}
}

pom.project {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import android.os.Bundle;
import android.os.IBinder;
import android.os.RemoteException;
import android.support.annotation.Nullable;
import android.text.TextUtils;
import android.util.Log;

Expand All @@ -36,6 +37,7 @@
import org.json.JSONObject;

import java.util.ArrayList;
import java.util.Calendar;
import java.util.Collections;
import java.util.Date;
import java.util.List;
Expand All @@ -49,17 +51,17 @@ public class BillingProcessor extends BillingBase
*/
public interface IBillingHandler
{
void onProductPurchased(String productId, TransactionDetails details);
void onProductPurchased(String productId, @Nullable TransactionDetails details);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Check if productId is nullable. As of now I don't think it is


void onPurchaseHistoryRestored();

void onBillingError(int errorCode, Throwable error);
void onBillingError(int errorCode, @Nullable Throwable error);

void onBillingInitialized();
}

private static final Date DATE_MERCHANT_LIMIT_1 = new Date(2012, 12, 5); //5th December 2012
private static final Date DATE_MERCHANT_LIMIT_2 = new Date(2015, 7, 20); //21st July 2015
private static final Date DATE_MERCHANT_LIMIT_1 = new Date(1354694400000L); //5th December 2012
private static final Date DATE_MERCHANT_LIMIT_2 = new Date(1437375600000L); //21st July 2015
Copy link
Contributor Author

@AllanWang AllanWang Jul 31, 2017

Choose a reason for hiding this comment

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

Replace with millis, since using the original notation may not produce expected results. See stack overflow. Based on PST

Copy link
Member

Choose a reason for hiding this comment

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

Are there any other options as a replacement? This is a bit less readable code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@serggl see latest commit. I'm 99% sure that that is how calendars are used, so it should be good. The date will likely vary based on timezones, but it shouldn't be off by more than a day.


private static final int PURCHASE_FLOW_REQUEST_CODE = 32459;
private static final String LOG_TAG = "iabv3";
Expand Down Expand Up @@ -296,10 +298,14 @@ private boolean loadPurchasesByType(String type, BillingCache cacheStorage)
return false;
}

/**
* Attempt to fetch purchases from the server and update our cache if successful
* @return {@code true} if retrieval occurs, {@code false} otherwise
* todo check if loading should return true if response code isn't successful
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@serggl I'm referring to this line where return true occurs after the if statement that checks if the response is ok. Is that expected behaviour?

Copy link
Member

@serggl serggl Aug 1, 2017

Choose a reason for hiding this comment

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

oh, right. Should return false here if response was not successful

Copy link
Contributor Author

@AllanWang AllanWang Aug 1, 2017

Choose a reason for hiding this comment

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

Resolved b281e51

*/
public boolean loadOwnedPurchasesFromGoogle()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, the load methods return true so long as a response is received, regardless of whether the response was successful or not. I also removed the first isInitialized call as it is called in both load methods. It therefore doesn't need to be verified a third time beforehand

{
return isInitialized() &&
loadPurchasesByType(Constants.PRODUCT_TYPE_MANAGED, cachedProducts) &&
return loadPurchasesByType(Constants.PRODUCT_TYPE_MANAGED, cachedProducts) &&
loadPurchasesByType(Constants.PRODUCT_TYPE_SUBSCRIPTION, cachedSubscriptions);
}

Expand Down Expand Up @@ -328,7 +334,7 @@ public boolean subscribe(Activity activity, String productId, String developerPa
*
* @param activity the activity calling this method
* @param productId the product id to purchase
* @param extraParamsBundle A bundle object containing extra parameters to pass to
* @param extraParams A bundle object containing extra parameters to pass to
* getBuyIntentExtraParams()
* @see <a href="https://developer.android.com/google/play/billing/billing_reference.html#getBuyIntentExtraParams">extra
* params documentation on developer.android.com</a>
Expand All @@ -352,7 +358,7 @@ public boolean purchase(Activity activity, String productId, String developerPay
*
* @param activity the activity calling this method
* @param productId the product id to purchase
* @param extraParamsBundle A bundle object containing extra parameters to pass to getBuyIntentExtraParams()
* @param extraParams A bundle object containing extra parameters to pass to getBuyIntentExtraParams()
* @see <a href="https://developer.android.com/google/play/billing/billing_reference.html#getBuyIntentExtraParams">extra
* params documentation on developer.android.com</a>
* @return {@code false} if the billing system is not initialized, {@code productId} is empty or if an exception occurs.
Expand Down Expand Up @@ -610,6 +616,7 @@ private boolean purchase(Activity activity, List<String> oldProductIds, String p
purchaseType,
purchasePayload);
}
//todo check if null check should be removed or if else should be removed
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will always be nonnull given the if statement above

Copy link
Member

Choose a reason for hiding this comment

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

yep. A remove candidate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I take it you mean that the null check if unnecessary. Resolved here 329c287

else // API v7+ supported
{
if (extraParamsBundle == null)
Expand Down Expand Up @@ -735,6 +742,7 @@ private boolean checkMerchant(TransactionDetails details)
return merchantId.compareTo(developerMerchantId) == 0;
}

@Nullable
private TransactionDetails getPurchaseTransactionDetails(String productId, BillingCache cache)
{
PurchaseInfo details = cache.getDetails(productId);
Expand Down Expand Up @@ -865,11 +873,13 @@ public List<SkuDetails> getSubscriptionListingDetails(ArrayList<String> productI
return getSkuDetails(productIdList, Constants.PRODUCT_TYPE_SUBSCRIPTION);
}

@Nullable
public TransactionDetails getPurchaseTransactionDetails(String productId)
{
return getPurchaseTransactionDetails(productId, cachedProducts);
}

@Nullable
public TransactionDetails getSubscriptionTransactionDetails(String productId)
{
return getPurchaseTransactionDetails(productId, cachedSubscriptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public PurchaseInfo(String responseData, String signature)
}

/**
* @deprecated dont call it directly, use {@see purchaseData}} instead.
* @deprecated don't call it directly, use {@see purchaseData} instead.
*/
@Deprecated
public PurchaseData parseResponseData()
Expand Down
2 changes: 1 addition & 1 deletion sample/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<resources>

<string name="app_name">In-app Billing v3 Sample</string>
<string name="wait">wait...</string>
<string name="wait">wait</string>
<string name="title">Sample Activity #%d</string>

</resources>