JSONObject "payments" and "refunds"

[kkostov]

Hi @antweb,
After migrating to the sqldelight2 branch (thanks a lot for your insights for that step), I've encountered some errors/warnings when handling tickets. While the database migrations seem to be applied correctly and order data is looks OK, libpretixsync is reporting some missing json fields so I thought to report it here just in case it may be related to the migration.

e.g.

 val checkResult = checkProvider.check(
                conf.eventSelectionToMap(),
                ticketid = rawResult,
                source_type = sourceType,
                answers = answers,
                ignore_unpaid = ignoreUnpaid,
                with_badge_data = withBadgeData,
                scanType,
                allowQuestions = allowQuestions
            )

INFO: Handling ticket
...
org.json.JSONException: JSONObject["payments"] not found.
	at org.json.JSONObject.get(JSONObject.java:596)
	at org.json.JSONObject.getJSONArray(JSONObject.java:789)
	at eu.pretix.libpretixsync.models.db.OrderExensionsKt.parsePendingTotal(OrderExensions.kt:66)
	at eu.pretix.libpretixsync.models.db.OrderExensionsKt.toModel(OrderExensions.kt:25)
	at eu.pretix.libpretixsync.check.AsyncCheckProvider.checkOfflineWithData(AsyncCheckProvider.kt:616)
	at eu.pretix.libpretixsync.check.AsyncCheckProvider.check(AsyncCheckProvider.kt:609)
	at eu.pretix.libpretixsync.check.TicketCheckProvider.check$default(TicketCheckProvider.kt:160)
	at eu.pretix.scan.tickets.data.TicketCodeHandler.handleScan(TicketCodeHandler.kt:120)

org.json.JSONException: JSONObject["refunds"] not found.
	at org.json.JSONObject.get(JSONObject.java:596)
	at org.json.JSONObject.getJSONArray(JSONObject.java:789)
	at eu.pretix.libpretixsync.models.db.OrderExensionsKt.parseRefunds(OrderExensions.kt:100)
	at eu.pretix.libpretixsync.models.db.OrderExensionsKt.toModel(OrderExensions.kt:27)
	at eu.pretix.libpretixsync.check.AsyncCheckProvider.checkOfflineWithData(AsyncCheckProvider.kt:616)
	at eu.pretix.libpretixsync.check.AsyncCheckProvider.check(AsyncCheckProvider.kt:609)
	at eu.pretix.libpretixsync.check.TicketCheckProvider.check$default(TicketCheckProvider.kt:160)
	at eu.pretix.scan.tickets.data.TicketCodeHandler.handleScan(TicketCodeHandler.kt:120)

Example order data: https://pretix.eu/control/event/iosdemo/democon/orders/BFUBF/

[raphaelm]

These are intentionally not synced for non-pretixPOS:

libpretixsync/libpretixsync/src/main/java/eu/pretix/libpretixsync/sync/OrderSyncAdapter.kt

Line 375 in 13a605e

resUrl += "&exclude=payments&exclude=refunds"

So the code should be able to handle them missing

[kkostov]

Right! The errors are just printed and the rest works fine it seems.

try {
        return json.getJSONArray("refunds")
    } catch (e: JSONException) {
        e.printStackTrace()
        return JSONArray()
    }

[antweb]

Right! The errors are just printed and the rest works fine it seems.

TL;DR: Yes, those can be treated as warnings here.

A little bit of background: the SQLDelight migration introduced models that have a slightly different handling of some properties stored in json_data. Before, the JSON access happened whenever the property was read. The new models behave differently and parse many properties when they are created.

The idea is to add stronger type safety to those properties and use consistent parsing / fallback logic. But the side-effect of this eager parsing is that we might run into quirks like this where JSON keys are explicitly left out because they are not accessed in that code path anyway.

The (somewhat noisy) e.printStackTrace() was copied over from the original parsing code to easier spot when we see this new behaviour and should definitely be removed here if this is a valid variant of the model (thanks @raphaelm for pointing out the case when this happens).

[raphaelm]

I wonder if there's any mechanism to "mark" this model so it doesn't bite us when we ever actually try to work with this data and its not there?

[antweb]

If we want to keep the same model for both versions of an order (full and redacted), it would already help to add explicit null values for refunds and payments. That way null would represent "redacted", JSONArray() an actual empty value and the type system would force us to make that distinction every time we use an order (as compared to an additional isRedacted flag).

[raphaelm]

Yeah I think that makes the most sense.