Modularization prototype. #1

ElektrojungeAtWork · 2016-06-14T19:15:32Z

This PR is based on the work done by @ranterle in https://github.com/bitstadium/HockeySDK-Android-Private/pull/19 and I shamelessly copied Mat's explanation of how the SDK works

It's a base for our architecture discussions for the SDK.

Please have a look and discuss.

Disclaimer

Treat this as WIP and as a long running PR. This is just an idea that I wanted to get out there for us to discuss, we're by no means bound to any of this. I hope I ticked all the right label boxes to make this look like the crazy 🎄 that it is.

What do we have here?

This approach contains several concepts which are not easy to build without each other, that's why I included them in one massive PR.

Modularization of the SDK
Move away from the static class methods on previous manager feature classes
Completely new integration method through a shared Avalanche class

Modularization

First of all, this splits up the SDK into several sub-modules, basically one per feature.
It adds a [core-module]((https://github.com/Microsoft/AvalancheSDK-Android/blob/modular-prototype/avalanchesdk-core) which is meant to provide all kind of centralized services: Setup, module management & communication, logging, persistence, transmission pipeline, shared resources & strings, …
The rest of the features will be implemented through different modules, e.g. the hockeysdk-crash module.

Telling modules apart

Modules live in their own namespace. Where the core is in com.microsoft.android, the crash module for example lives in com.microsoft.android.crash. This is both reflected in the Java package structure, but also in the Android resource bundling structure. Otherwise the modules would conflict over shared and individual resources. See the relevant Java packages and AndroidManifest.xml files for example.

Module implementation - example: CrashManager

Next, each module means to be converted from the static class method access (which was horrible for testing/mocking and more) to singletons. As an example take the new CrashManager. It has been fully converted. It would even be possible to remove the need for the modules to be singletons - but that I left up for discussion on purpose (who doesn't love a fight over singletons) and it would have taken this one step too far.

Integration

Bringing it all together is a new approach in integration. Kudos to @troublemakerben for his work on the umbrella header on Android. This is somewhat similar.

Why not start with an example? Here you go:
For the brutally simplest way of integration you pull in your dependencies, and then at the earliest stage in your app call this:

Avalanche.configure(getApplication());

This will then detect all available features (for example if you have crash reporting and update distribution as dependencies, it will find both) and automatically instantiate and configure them.

A bit more flexible? How about:

Avalanche.configure(getApplication(), CrashManager.class);

This will only try to instantiate and configure the crash reporting feature, even if other modules would be available. It will still do auto-configuration for the modules you defined.

Want even more flexibility?

Avalanche.configure(getApplication(), CrashManager.getInstance().register(context, listener, …));

This will only configure the SDK with the features in the way you configured them. No auto-configuration will be performed, since you are doing this inline.

You can also do:

Avalanche.configure(getApplication(), false);

and then later

Avalanche.getInstance().addFeature(CrashManager.getInstance().register(…));

This will only set up the SDK in the first step and then allow you to add in the configured features where you need them.

Auto-Configuration?!

What does that mean? So as the integration, all modules will have an opinion on how they typically should integrate with the app. For example for the crash manager it makes sense to register and configure it in the onResume() callback of the app's main activity.
To achieve this, all features (or components) need to do two things:

They need to have a public static method getInstance() which returns an instance of the class - this is a convention defined by the Avalanche class.
Implement the AvalancheComponent interface or subclass the convenience class DefaultAvalancheComponent.

Because of this, they can hook into the application/activity lifecycle and perform their own registration when they see fit. They are responsible of respecting the users preference, e.g. when the user already configured the feature otherwise, they should reflect that. For an example see the registration code for the crash manager.

guperrot · 2016-06-14T19:29:19Z

.idea/compiler.xml

@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="UTF-8"?>


.idea files are generated by Android Studio and can be a pain to share between developers.
In azme we git ignored them, worked great.

IDE related files shouldn't be there. BTW Guillaume, it is not simply opening up project folder, need to be imported. There is a Gradle task that can generate IDE related stuffs so we can do this instead.

You're right. I just used Githubs default .gitignore instead of the one we already used. I removed the .idea and updated the gitignore.

guperrot · 2016-06-14T21:01:12Z

Shouldn't we rely only on the stable version of the android tools? I get Error:Failed to find Build Tools revision 24.0.0-rc4 when trying to open the project.

For an open source SDK we should have that design constraint: download latest stable android studio,next, next, checkout from sources: it just works with no warning and no error.

ElektrojungeAtWork · 2016-06-14T21:03:12Z

Sure.

From: Guillaume Perrot notifications@github.com
Reply-To: Microsoft/AvalancheSDK-Android reply@reply.github.com
Date: Tuesday, June 14, 2016 at 2:01 PM
To: Microsoft/AvalancheSDK-Android AvalancheSDK-Android@noreply.github.com
Cc: Benjamin Reimold bereimol@microsoft.com, Assign assign@noreply.github.com
Subject: Re: [Microsoft/AvalancheSDK-Android] Modularization prototype. (#1)

Shouldn't we rely only on the stable version of the android tools? I get Error:Failed to find Build Tools revision 24.0.0-rc4 when trying to open the project.

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/1#issuecomment-226014930, or mute the threadhttps://github.com/notifications/unsubscribe/AJsX6IPi-8OYXz1sEIL5gDwPEpjlzdw5ks5qLxaZgaJpZM4I1qde.

jaeklim · 2016-06-14T21:13:47Z

avalanchesdk-core/build.gradle

+            minifyEnabled false
+        }
+    }
+}


We have duplicated settings in build.gradle files. You can change the constants to variables and then set the variables in root build.gradle.

Yes.

Question is: how do you want/expect me/us to handle this sort of stuff? Should we discuss here or F2F?

No lintOptions and checkstyle (e.g. jacoco) here?

I think we should add lintoptions and use checkstyle.
I’m also a fan of Findbugs and was looking at infer (http://fbinfer.com/) the other day.

ElektrojungeAtWork · 2016-06-14T21:19:03Z

This is sort of a comment: I haven't said this excplicitly (at least I think) but I'd love to refactor as much of the stuff that's inside the CrashManager as possible. This is old old old old stuff.

ElektrojungeAtWork · 2016-06-14T21:39:05Z

For an open source SDK we should have that design constraint: download latest stable android studio,next, next, checkout from sources: it just works with no warning and no error.

That's how we worked so far, too.

guperrot · 2016-06-15T02:12:11Z

avalanchesdk-crash/build.gradle

+
+android {
+    compileSdkVersion 23
+    buildToolsVersion "24.0.0-rc4"


We should also use 23.0.3 like in other modules. We should not depend on rc/preview tools for open source product code.

All modules should use the same stable compileSdkVersion, buildToolsVersion. For bundling reasons those also have to target the same minSdkVersion and targetSdkVersion. It also makes sense for now to make them have all the same versionCode (which is ignored for libraries anyway) and versionName.

Adds a crash button which forces an app crash. Sets up the SDK in automatic-mode.

jaeklim · 2016-06-15T22:02:22Z

avalanchesdk-test/build.gradle

+dependencies {
+    compile fileTree(dir: 'libs', include: ['*.jar'])
+    compile 'com.android.support:appcompat-v7:23.4.0'
+    testCompile 'junit:junit:4.12'


Is this module for unit test or integration test?
Need to have unittest in each module?

No, this module is a “test” app that integrates the other two modules. Like your “puppet” app.

We can use a different approach, if you want. FYI: We’ve had a separate. app quick (manual) for integration tests for HockeyApp but I kinda like the approach of having a quick way of testing if my recent change works.

About unit tests: yes, I think each module should contain unit tests.

Then it needs to be renamed to avoid any confusions. :) I'd prefer an actual app name instead.

Okay. IMHO it’d be best if we name the app the same across all plattforms.

I hereby propose “Sasquatch” or “St.Bernard” to fit with the Avalanche theme ;)

ghost · 2016-06-16T22:25:00Z

Hi,

I like the structure of the SDK. Good starting point / great work! Also the iOS project structure aligns with that perfectly. Some things that we should discuss.

Naming of modules: Depending on the final product name I'd prefer something like AvalancheCrashes over CrashManager (yes, the Avalanche prefix is way to long, but maybe we can come up with something more exciting).
configure method name: Pretty much the same, I'd prefer something more self-explanatory (my proposal for iOS was useFeatures). I`m pretty sure we are going to have some meetings related to API surface.
Module instances: If we stick with the singleton approach, I'd prefer to mke the instance private and expose class methods rather than instance methods (it's so much shorter). Talking about the public API exposing the default instance only makes sense to me, if there's also a way to create custom instances.

Thank you!

iamclement · 2016-06-16T23:29:50Z

About the naming, I discussed about that with Matt on our channel and we can't use "Avalanche". He proposed the simple "Xamarin" for now as it needs to be really explicit because directly used by customers.

ElektrojungeAtWork · 2016-06-17T01:01:17Z

@chrwend

I like more simple names, too. I'd like really, really short ones. We don't have to prefix anything on Android as we have packages for that, so we could do Crashes, Metrics, Push and so on.
I like useFeatures, or use which would end up with something like Avalanche.use(Crash.class, ...) in Android.
I don't like class methods too much although they are very convenient...it's very easy to implement them but almost impossible to test them as we have seen with HockeyApp.

@clpolet
Just being curious: why can't we call it "Avalanche"? My 2 cents are: it doesn't make sense to name it "Xamarin" as there is already a "Xamarin SDK".

iamclement · 2016-06-17T01:12:03Z

Avalanche is an internal naming and can't be used for public facing code, at least that's what I've heard. This is definitively a PM discussion.

ghost · 2016-06-17T17:28:45Z

@TroubleMakerBen I'm also fine with module names like Crashes, Metrics, Push for Android. Regarding the class methods I'm pretty sure there would be a way to inject a mock which acts as the shared instance for your tests. But, yeah...

…ect’s build gradle

matthiaswenz · 2016-06-17T18:48:43Z

The main intention to go with a singleton approach with getInstance() rather than static class methods was that this makes it easier to change the API down the road, if we'd like to remove the singleton patterns and it makes the whole project and it's modules easier to test in isolation.

ElektrojungeAtWork · 2016-06-17T18:51:26Z

Update

I've added a checkstyle task with a basic checkstyle configuration. I haven't fixed any of the warnings as we have to agree on a style guide first.
I've bumped the support lib version to 24.0.0
I've removed the localization files for now.

…r for Channel

Also add missing abstract keyword for ingestion models.

Fix coverage for firebase/push

Feature/sasquatch breakpad

…onapp Customvision app + Assets module

Add modularized protoype for discussion

6e1b3ff

ElektrojungeAtWork added in progress question labels Jun 14, 2016

ElektrojungeAtWork assigned guperrot, matthiaswenz, ElektrojungeAtWork and jaeklim Jun 14, 2016

guperrot reviewed Jun 14, 2016
View reviewed changes

Benjamin Reimold added 2 commits June 14, 2016 13:44

remove .idea/

43b2b4a

Updated gitignore

9c2a4f1

jaeklim reviewed Jun 14, 2016
View reviewed changes

Benjamin Reimold added 2 commits June 14, 2016 14:34

Use stable build tools

67b3c05

Use build tools 2.1.2

3cd686c

guperrot reviewed Jun 15, 2016
View reviewed changes

matthiaswenz added 3 commits June 15, 2016 07:25

Remove constraint layout reference

a07f106

Added crash button to test functionality to test app

78de571

Adds a crash button which forces an app crash. Sets up the SDK in automatic-mode.

jaeklim reviewed Jun 15, 2016
View reviewed changes

Benjamin Reimold added 2 commits June 17, 2016 10:57

Use gradle daemon for faster builds

18efa0d

Bump supportLibs to 24.0.0 (is now stable) and define version in proj…

f26c7e5

…ect’s build gradle

Benjamin Reimold added 3 commits June 17, 2016 11:00

Add checkstyle to project

f72cb35

Remove localization (for now)

9ab1294

Renamed Test-App to “Sasquatch”

79e556a

Benjamin Reimold and others added 8 commits June 20, 2016 10:17

Remove “Tracking”-feature, rename some of the classes, add placeholde…

2dbce8b

…r for Channel

add isHighprio() to data interface

cc0291b

Renamed modules to base and crash

2fac4a8

Rename java packages

c634e63

Introduce analytics module and basic ingestion object models

b0bda4a

Fix package name in String that’s used to determine the classname during

1f4df7d

Add Model objects for crashes

f2a30da

Add osApiLevel to device log.

f023f88

Also add missing abstract keyword for ingestion models.

guperrot merged commit f7f7fdc into master Jun 23, 2016

guperrot deleted the modular-prototype branch June 23, 2016 00:40

Khoosham mentioned this pull request Dec 15, 2016

Some SSLException variants can be transient and should be retried #270

Closed

guperrot pushed a commit that referenced this pull request Apr 21, 2017

Merge pull request #1 from Microsoft/fix/push/firebase-coverage

035c397

Fix coverage for firebase/push

guperrot pushed a commit that referenced this pull request Dec 14, 2017

Merge pull request #1 from JessicaYeh/feature/sasquatch-breakpad

7e83187

Feature/sasquatch breakpad

annakocheshkova pushed a commit to akvelon/AppCenter-SDK-Android that referenced this pull request Jun 14, 2018

Merge pull request microsoft#1 from annakocheshkova/assets-customvisi…

3212ff6

…onapp Customvision app + Assets module

mohib-irshad-10p mentioned this pull request May 9, 2020

Inapp Update Fails #1418

Closed

ichenhe mentioned this pull request Nov 14, 2020

Failed to connect to in.appcenter.ms in some areas of China mainland #1478

Closed

yulin-li mentioned this pull request Dec 16, 2020

App ANR due to deadlock in appcenter SDK #1486

Closed

pasha-o mentioned this pull request Sep 6, 2023

W/AppCenter: Try #1 failed and will be retried in xxxx ms #1566

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modularization prototype. #1

Modularization prototype. #1

ElektrojungeAtWork commented Jun 14, 2016

guperrot Jun 14, 2016

jaeklim Jun 14, 2016

ElektrojungeAtWork Jun 14, 2016

guperrot commented Jun 14, 2016 •

edited

Loading

ElektrojungeAtWork commented Jun 14, 2016

jaeklim Jun 14, 2016

ElektrojungeAtWork Jun 14, 2016

jaeklim Jun 15, 2016

ElektrojungeAtWork Jun 15, 2016 •

edited

Loading

ElektrojungeAtWork commented Jun 14, 2016

ElektrojungeAtWork commented Jun 14, 2016

guperrot Jun 15, 2016

jaeklim Jun 15, 2016

ElektrojungeAtWork Jun 15, 2016 •

edited

Loading

jaeklim Jun 15, 2016

ElektrojungeAtWork Jun 15, 2016

ghost commented Jun 16, 2016 •

edited by ghost

Loading

iamclement commented Jun 16, 2016

ElektrojungeAtWork commented Jun 17, 2016

iamclement commented Jun 17, 2016

ghost commented Jun 17, 2016

matthiaswenz commented Jun 17, 2016

ElektrojungeAtWork commented Jun 17, 2016

Modularization prototype. #1

Modularization prototype. #1

Conversation

ElektrojungeAtWork commented Jun 14, 2016

Disclaimer

What do we have here?

Modularization

Telling modules apart

Module implementation - example: CrashManager

Integration

Auto-Configuration?!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guperrot commented Jun 14, 2016 • edited Loading

ElektrojungeAtWork commented Jun 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ElektrojungeAtWork Jun 15, 2016 • edited Loading

Choose a reason for hiding this comment

ElektrojungeAtWork commented Jun 14, 2016

ElektrojungeAtWork commented Jun 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ElektrojungeAtWork Jun 15, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Jun 16, 2016 • edited by ghost Loading

iamclement commented Jun 16, 2016

ElektrojungeAtWork commented Jun 17, 2016

iamclement commented Jun 17, 2016

ghost commented Jun 17, 2016

matthiaswenz commented Jun 17, 2016

ElektrojungeAtWork commented Jun 17, 2016

guperrot commented Jun 14, 2016 •

edited

Loading

ElektrojungeAtWork Jun 15, 2016 •

edited

Loading

ElektrojungeAtWork Jun 15, 2016 •

edited

Loading

ghost commented Jun 16, 2016 •

edited by ghost

Loading