DeepLinkDelegate

[ZacSweers]

This implements what was discussed in #86 by using a generate DeepLinkDelegate to handle dispatches instead of maintaining the logic solely in a generated DeepLinkActivity.

Three parts:

• Generate DeepLinkDelegate class with its single static dispatchFrom(Activity) method.
• Slim down the generated DeepLinkActivity to just use this delegate
• Generate a DeepLinkResult class to use on the fly to return results from dispatchFrom()
  • This is a little wonky because the current API is just a Java library and not an AAR. Generating it is the only current way to make this class available in generated code.

Now you can simply call this from your own activity to handle a dispatch:

DeepLinkResult result = DeepLinkDelegate.dispatchFrom(this);

if (!result.isSuccessful()) {
  // Do whatever
  // result.uri();
  // result.error();
} else {
  // Yay. The activity started, probably don't actually want to do anything in this case
}

Open questions:

• API/naming
• Should DeepLinkDelegate be an instance or just have its one static method?
  • Is there any benefit to having an instance? Testing, helpful with DI?
• Should DeepLinkActivity generation be disable-able? If so, how?
  • Even though not adding it to the manifest closes it off, some might still not like having it at all.
  • Alternative is to remove it entirely.
• Where does DeepLinkLoader fit in? Should it remain separate?

Resolves #86

[ZacSweers]

My personal thoughts on the open questions:

• I like this naming.
• API seems a little weird in that it can return a result even if it was successful, so the calling activity could do something after the new one was launched. I suppose there's nothing wrong with that if the user is sensible about it.
• I prefer static right now, and maybe considering other options later if the need arises for it.
• Would be nice to have it disable-able, but don't know how that could be controlled dynamically. Would personally be fine with removing it entirely, though that would be a breaking API change.
• Fine with the loader being separate for testing and separation of concerns, but it does seem like something that could be as easily pulled into the delegate class or even inlined if we wanted.

[felipecsl]

looking pretty good so far. just need to make checkstyle happy.
@cdeonier might want to take a look as well

[ZacSweers]

Finally fixed all the checkstyle issues and CI's happy

[ZacSweers]

New thought: should DeepLinkResult#errorMessage() maybe just be shortened to error()?

Also, regarding error handling, there's a couple other approaches we could take.

• dispatch could just call through to the broadcast receiver like the existing implementation and not have the result class
• dispatchFrom could have an overload that takes a DispatchResultCallback that has some onResult(boolean,Uri,String) method, potentially even only calling back on errors.

[felipecsl]

• errorMessage -> error 👍
• Sounds like it could be a good simplification for the DeepLinkDelegate to also notify the broadcast receiver.
• Re. taking a DispatchResultCallback I'm not that sure. Maybe for a follow-up PR

[ZacSweers]

Restructured more, sorry it ended up being a decent amount of change

• error() instead of errorMessage()
• dispatch now returns a result and also handles notifying listeners
• refactor constants and create DeepLinkActivity annotation
  • This holds those constants in a non-generated place and also makes it easy to indicate to the processor that it doesn't need to generate an activity when it's used. This prevents extra entry points or unnecessarily generated code.

It's a breaking change, but I think one worth making anyway. This change in general would probably be worth a (semantically) major version bump anyway

[felipecsl]

Alright this is looking pretty good to me. Do you mind squashing the commits before we merge?

[ZacSweers]

Done

[felipecsl]

Thanks! I'll bump the pre-release to 2.0.0-SNAPSHOT

[ZacSweers]

awesome, thanks!