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

Why you call finishTransaction inside SDK #96

Closed
oxozle opened this issue Oct 19, 2016 · 14 comments
Closed

Why you call finishTransaction inside SDK #96

oxozle opened this issue Oct 19, 2016 · 14 comments
Labels
answered Questions which have accepted answers. type: enhancement type: question

Comments

@oxozle
Copy link

oxozle commented Oct 19, 2016

I should manually call finishTransaction after I receive response from my server. If I buy product and my server return 500 error for example SwiftyStoreKit says apple that everything is OK

@bizz84
Copy link
Owner

bizz84 commented Oct 19, 2016

This has already been discussed here: #50

I would suggest that you keep track of when a purchase has been completed until you can successfully deliver the content from your server.

@bizz84 bizz84 added type: question answered Questions which have accepted answers. labels Oct 19, 2016
@artemshuba
Copy link

I have read that discussion but still don't see why did you decide to call finishTransaction in paymentQueue before app can verify it with server. Could you please explain it?

@bizz84
Copy link
Owner

bizz84 commented Nov 10, 2016

Ok so to clarify:

Your application should call finishTransaction(_:) only after it has successfully processed the transaction and unlocked the functionality purchased by the user.
  • The purchaseProduct() method was originally conceived for cases where the content can be delivered directly from the app (no server request needed).
  • To better support delivery of content loaded from the server I could do some changes:
public enum PurchaseResult {
     case purchased(productId: String, paymentTransaction: SKPaymentTransaction)
     case success(productId: String)
     case error(error: PurchaseError)
}
/*
 * Purchase a product. Usage:
 * Use atomically = true for content that can be delivered from the app.
 * This will result in .success(productId) if the purchase is successful.
 * Use atomically = false for content that will be delivered by a server. 
 * This will result in .purchased(productId, paymentTransaction) if the purchase is successful.
 * Clients are responsible for keeping track of the paymentTransaction until the content is delivered, and must call finishTransaction() to clear the purchase from the transaction queue and finalise the purchase.
 */
func purchaseProduct(productId: String, atomically: Bool = true, completion: (PurchaseResult) -> ())
func finishTransaction(paymentTransaction: SKPaymentTransaction)

Some other considerations

If a clients starts a non atomic purchase and quits the app before delivering the content from the server, the completeTransactions() API (to be used when the app starts) also calls finishTransaction() by default.

Maybe it would make sense to also do this?

func completeTransactions(atomically: Bool = true, _ completion: ([CompletedTransaction]) -> ())

I would then assume that if the client says completeTransactions(atomically: false) then it has the responsibility of calling finishTransaction() on each transaction.

@Stealth2012 Would these proposed API changes meet your requirements?

@tbaranes
Copy link

tbaranes commented Nov 10, 2016

Don't know for @Stealth2012, but I think that would be a great move 👍

@bizz84
Copy link
Owner

bizz84 commented Nov 10, 2016

Ok cool. I'll start working on this. Given my current timescales I think it will take me a week to get this done.

@artemshuba
Copy link

Yeah, looks great!

@bizz84
Copy link
Owner

bizz84 commented Nov 13, 2016

I'm not sure about what is the most desirable API for this. Some feedback is welcome.

First of all, I have updated the PurchaseResult type returned by SwiftyStoreKit.purchaseProduct() to look like this:

public enum PurchaseResult {
    case success(productId: String, transaction: SKPaymentTransaction?)
    case error(error: PurchaseError)
}
func purchaseProduct(productId: String, atomically: Bool = true, completion: (PurchaseResult) -> ())

If the user calls purchaseProduct() with atomically = false, she will get a non-nil transaction. After the content is delivered from the server, the transaction can be used like this:

SwiftyStoreKit.finishTransaction(transaction)

However, I'm not sure if the whole SKPaymentTransaction object should be exposed here as it has a lot of properties and would like to prevent the user from using this incorrectly.

After all, SwiftyStoreKit should take care of the low level details of working with transactions.

With this in mind, I was thinking to declare a phantom protocol and attach this to an extension of SKPaymentTransaction:

protocol PaymentTransaction { }
extension SKPaymentTransaction: PaymentTransaction { }

The PurchaseResult type would then look like this:

public enum PurchaseResult {
    case success(productId: String, transaction: PaymentTransaction?)
    case error(error: PurchaseError)
}

Finishing the transaction would then be delegated to SwiftyStoreKit:

func finishTransaction(transaction: PaymentTransaction) {
   guard let skTransaction = transaction as? SKPaymentTransaction else {
      print("Object is not a SKPaymentTransaction: \(transaction)
      return
   }
   SKPaymentQueue.default().finishTransaction(skTransaction)
}

I believe that a stricter API makes SwiftyStoreKit safer to use.

@tbaranes @Stealth2012 What are your thoughts?

@tbaranes
Copy link

tbaranes commented Nov 13, 2016

That's a good question. Both have pro and cons, but since SwiftyStoreKit is abstracting all the StoreKit symbols, I'll go for a stricter API.

Also, if necessary, PaymentTransaction can evolve in the future by exposing some of SKPaymentTransaction properties to fulfil the server requirements.

@artemshuba
Copy link

I agree, but would be nice if PaymentTransaction could expose transactionId from SKPaymentTransaction.

@bizz84
Copy link
Owner

bizz84 commented Nov 14, 2016

This feature is now dev complete: https://github.com/bizz84/SwiftyStoreKit/pull/107/files

The API has changed slightly from the initial proposal so that completeTransactions(), purchaseProduct() and restoreProducts() all return the same Product type on success:

// Purchased or restored product
public struct Product {
    public let productId: String
    public let transaction: PaymentTransaction
    public let needsFinishTransaction: Bool
}

@Stealth2012 do you want to expose SKPaymentTransaction.transactionIdentifer? What would be the use case for this?

@artemshuba
Copy link

@bizz84 we use server to validate purchase and we need transactionIdentifier to find correct transaction in app receipt

@bizz84
Copy link
Owner

bizz84 commented Nov 16, 2016

Non-atomic purchases are now implemented and available on pod version 0.6.0.

If there are any problems with the new API, feel free to re-open this issue.

@bizz84 bizz84 closed this as completed Nov 16, 2016
@artemshuba
Copy link

Hi, I found that there is still a case when finishTransaction will be called inside SwiftyStoreKit.
This is the code from paymentQueue:

if transactionState != .purchasing {

               let product = Product(productId: transaction.payment.productIdentifier, transaction: transaction, needsFinishTransaction: !atomically)

               completedTransactions.append(product)
               
               print("Finishing transaction for payment \"\(transaction.payment.productIdentifier)\" with state: \(transactionState.stringValue)")
               
               paymentQueue.finishTransaction(transaction)
           }

Looks like there is missing a check if atomically is true:

if atomically {
  paymentQueue.finishTransaction(transaction)
}

@bizz84
Copy link
Owner

bizz84 commented Nov 25, 2016

@Stealth2012 Thanks for spotting this!

I have fixed it in pod version 0.6.1.

Changeset: 0061034

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answered Questions which have accepted answers. type: enhancement type: question
Projects
None yet
Development

No branches or pull requests

4 participants