Skip to content

Commit

Permalink
android notif: Parse things at a "crunchy shell", step 1 of N.
Browse files Browse the repository at this point in the history
We're going to refactor this code into the "crunchy shell,
soft center" pattern, where

 * All parsing of messy data-from-the-network happens at the edge,
   as soon as possible.

 * The parsing logic at the edge constructs new, inward-facing data
   structures from the messy data it gets.  It meticulously ensures
   that the data it produces has the expected format.  Where the
   messy input does something unexpected that we aren't prepared
   to cleanly handle, the input is rejected at the edge.

 * All the application code on the inside deals only with the nice,
   well-formatted data produced by the parsing logic at the edge.
   As a result it needs many fewer boring checks for null, for
   strings that should parse as integers but don't, etc., etc.
   The real application logic it's expressing is easier to read,
   and it's less prone to bugs.

 * The metaphor: the edge is "crunchy" with parsing and validation,
   protecting the inside so it can safely be "soft".

 * In particular, when data comes in that doesn't match expectations,
   a program without a "crunchy shell" will run into that fact
   somewhere deep inside application code.  It's likely to be missing
   any logic there to deal with the situation, and then to crash; if
   it does handle it, it may have already half-processed the data,
   making it too late to cleanly ignore.  When bad data is caught by
   a "crunchy shell", it gets cleanly ignored or rejected.

We'll go about this in incremental, individually-understandable
steps over a series of commits.

This commit gives the "shell" its first small bit of "crunch": we
parse out one required field, `recipient_type`.  Most of the commit
is in adding a few lines of shared infrastructure that will be
re-used as we move more parsing to the "shell".
  • Loading branch information
gnprice committed Mar 7, 2019
1 parent 12c4219 commit f85d325
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,13 @@ static void onReceived(Context context, ConversationMap conversations, Map<Strin
final String eventType = mapData.get("event");
switch (eventType) {
case "message":
final MessageFcmMessage fcmMessage = MessageFcmMessage.Companion.fromBundle(data);
MessageFcmMessage fcmMessage;
try {
fcmMessage = MessageFcmMessage.Companion.fromBundle(data);
} catch (FcmMessageParseException e) {
Log.w(TAG, "Ignoring malformed FCM message of type `message`: " + e.getMessage());
return;
}
addConversationToMap(fcmMessage, conversations);
updateNotification(context, conversations, fcmMessage);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ import android.os.Bundle
* In our notification code we often say "FCM message" or "Zulip message"
* to disambiguate between these two.
*/
internal class MessageFcmMessage private constructor(val bundle: Bundle) {
internal class MessageFcmMessage private constructor(
val bundle: Bundle,
val recipientType: String
) {

/** Really "event type": one of a small fixed set of identifiers. */
val event: String?
get() = bundle.getString("event")

val recipientType: String?
get() = bundle.getString("recipient_type")

val content: String?
get() = bundle.getString("content")

Expand Down Expand Up @@ -69,7 +69,16 @@ internal class MessageFcmMessage private constructor(val bundle: Bundle) {

companion object {
fun fromBundle(bundle: Bundle): MessageFcmMessage {
return MessageFcmMessage(bundle.clone() as Bundle)
return MessageFcmMessage(
bundle = bundle.clone() as Bundle,
recipientType = bundle.requireString("recipient_type")
)
}
}
}

private fun Bundle.requireString(key: String): String {
return getString(key) ?: throw FcmMessageParseException("missing expected field: $key")
}

class FcmMessageParseException(errorMessage: String): RuntimeException(errorMessage)

0 comments on commit f85d325

Please sign in to comment.