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

[New arch] App modularization and decoupling #2523

Closed
davigonz opened this issue Apr 24, 2019 · 3 comments
Closed

[New arch] App modularization and decoupling #2523

davigonz opened this issue Apr 24, 2019 · 3 comments

Comments

@davigonz
Copy link
Contributor

davigonz commented Apr 24, 2019

The status of the app and its code is a recurrent topic that's been discussed several times. A couple of months ago we started to work on a new architecture that is transforming the app to be more testable, robust and future proof.

Thanks to this new architecture, we should manage to have an app with much less dependencies but sometimes it's not easy to know and control, even less in an app as huge as ownCloud for Android.

So let's go through several solutions to modularize and decouple the app

Modularization with layers approach

  • Diagram
    2 3  oC app modules diagram_resized

  • Advantages:

    • Scalability: several developers working independently in different modules.
    • Maintainability: gradle can build modules faster since the classes and resources are separated in different modules. CI builds will be accelerated too.
    • Less tests to run when there's a new change: we need to run the tests in the module affected by that change and anything else that depends on it. We do not need to run all the tests if other modules are not impacted by the changes.
    • Reduce dependencies => less coupled code.
    • SWITCH MODULES: if in the future we want an app with a different UI and a different way to handle the information to show, we would just need to replace the :ownCloudApp module. And the same for the rest of modules, if one day we want to handle data differently to the current implementation, replacing the :ownCloudData module should be enough.
  • Video: https://youtu.be/PZBg5DIzNww


OTHER TOOLS

I've found an open source tool called Apk dependency graph that could help us a lot on this matter.

This is how the dependency graph for the ownCloud Android app looks like, based on New arch - capabilities branch (I put it live in https://davigonz.github.io/oCAndroidDependencyGraph/ so everyone can play with it) :

Screenshot 2019-04-24 at 18 31 14

Above we can notice some parts of new architecture (OCShareRepository, OCShareViewModel, OCCapabilityRepository, OCCapabilityViewModel...) along with old code.

And below there's an example of a good architecture with low class coupling:

Screenshot 2019-04-24 at 18 33 25

Taking into account this diagram while working on the new architecture would be great and will allow us to have a higher level vision and complete the whole app transformation process.

So we could use this issue as an epic and create possible changes that come to our mind after having a look at the diagram.

Note: the diagram is currently hosted as a webpage in https://davigonz.github.io/oCAndroidDependencyGraph/ and it uses this repo but we could move it with the rest of ownCloud repositories and even update the diagram via some kind of script included in bitrise (not sure if is possible, need to be checked) and have it updated with new arch changes.

CC / @michaelstingl @jesmrec @abelgardep @hosy

@owncloud/ios-core-developers you have something similar for iOS as well => https://github.com/PaulTaykalo/objc-dependency-visualizer

@davigonz davigonz changed the title App modularization App modularization and decoupling Apr 24, 2019
@michaelstingl
Copy link
Contributor

@davigonz branch with first steps in new architecture isn't merged yet. What does the top image show? Last stable release build? Current master branch? Or the branch with the new architecture?

@davigonz
Copy link
Contributor Author

davigonz commented Apr 29, 2019

branch with first steps in new architecture isn't merged yet. What does the top image show? Last stable release build? Current master branch? Or the branch with the new architecture?

I generated the graph from new arch - capabilities branch to start seeing how the dependencies of the new architecture look like there, along with old code.

Let me update the first comment with this info

@jesmrec jesmrec added this to the 2.12.0 milestone Jul 2, 2019
@jesmrec jesmrec added the Sprint label Jul 2, 2019
@davigonz davigonz self-assigned this Jul 2, 2019
@davigonz davigonz changed the title App modularization and decoupling [New arch] App modularization and decoupling Jul 8, 2019
@davigonz davigonz added the Epic label Jul 29, 2019
@davigonz
Copy link
Contributor Author

davigonz commented Jul 29, 2019

In order to not duplicate information and have the generic architecture details in only one issue, I've just moved what I wrote here to the epic architecture issue. I will close this, please follow up the epic in #2351

The modularization related to shares will be addressed via #2607

@davigonz davigonz removed this from the 2.12.0 milestone Jul 29, 2019
@jesmrec jesmrec removed the Sprint label Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants