Breaking OFN down into Domains

api
Tags: #<Tag:0x00007f431585db80>

#4

@oeoeaio @maikel @Matt-Yorkley @enricostn @Hugs @andy maybe you have some ideas to share on the topic as well?


#5

@Kirsten you might have ideas on this as well…


#6

Seems like a sensible suggestion. My only point would be that we shouldn’t try to specify the domains up-front.
In DDD, we talk about bounded contexts - these are areas of the business that may use the same word but have different meanings (e.g. a customer in the delivery context means something different to a customer in the invoicing context, even though they may be different views over the same entity)
When we try and specify things in advance, it usually leads to things being hard to change and probably not what we actually need. Eric Evans did a great talk at the DDD exchange in London, probably 10 years ago, where he talked about how, when the domains aren’t aligned properly, the important logic is in the spaghetti of connections and exceptions between the concepts.

Andy’s rule of thumb for domains and patterns: identify duplication, extract duplication, notice what you’ve extracted, name it.
The way that most people use domains and patterns: find a named solution, duplicate it, find it doesn’t quite fit, add exceptions, get slower and slower as the exceptions become more and more complex


#7

Nice, thanks Andy for your feedback!
I really like that pattern of extracting and then naming. I think that is very important specially at lower level, as we code business logic classes.
My suggestion of domains is just a way to separate the existing app in more or less similar sized mini-apps (it’s “heavily inspired” in other ecommerce systems I know). It works as a guide, we should re-evaluate as we go.


#8

duplication is far cheaper than the wrong abstraction

Sandi Metz dixit.

Quite aligned to her post https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstraction, I believe. So we are all on the same page.


#9

I have nothing to add, other than after working with @andy for awhile I am extremely happy to see this conversation / activity happening in the OFN community. From what I understand of it (I think a reasonable amount) it is going to make things way better, and help us to never fall this deep in tech debt again AND make it much easier for new people to join us and contribute to OFN more easily :slight_smile:


Document our API
#10

The separation of the admin interface jumps into my face when thinking about bounded context. We already have different namespaces in app/controllers, app/views and app/assets for all admin related stuff. Moving all this into its own module sounds like a good idea to me. I just don’t know yet how that works with shared code like the libs and models. Do we need to move all models into a separate module that is then available to the admin module and the main application? Sorry, I’m not familiar enough with Rails’ engines.


#11

:smiley:

@maikel we need to get the details sorted about the rails engine. I believe we should move the models together with controllers and views for specific parts/domains.
I have started the conversation around details with this PR.

imo, I think admin is too large already, I’d use the opportunity to make it a bit more specific (but not too specific and create too many parts), this is why I suggested above the separation of the backoffice/admin into 3 parts:

  • BackOffice - Catalog -> Product/Inventory management
  • BackOffice - Order Management -> Orders, packing and delivery management (to start with I’d include here Order Cycle management, Invoicing and Accounting)
  • BackOffice - Entity Management -> Enterprises and Customers management (CRM)

This is currently all under “Admin”.
So, this would be 3 engines: catalog, orders and entities.

On the frontoffice (website), I’d suggest we make it 3 domains/engines as well: web, shops and checkout (shops is what I called “frontoffice - catalog” on the original post)

Does this make sense?


#12

I commented on your pull request with a few questions. I think that there is a lot of potential in this, but I also see that it takes a lot of my time to think about it while there are so many things to do.

In regards to the back office, I don’t want to change too much at a time. Maybe it would be good to first experiment with the front end and see which lessons we learn from that.

There are a lot of things in transition. Code style, becoming independent from Spree, applying best practices in a lot of areas. And I think it’s bad for consistency if we have too many things in transition. It would help newcomers if everything was consistent.

I know you are really excited about this and I’m happy to see progress. Let’s do it, step by step. Don’t expect this to be merged in a few days.


#13

Hi @maikel,
I am sorry, I am aware I am taking quite a lot of your time for reviews lately! Thanks for that. I can wait.

I commented on your pull request with a few questions.

I think I got your feedback for the TemplateController on the main cookies PR, I dont think I got any feedback on the POC PR.

In regards to the back office, I don’t want to change too much at a time. Maybe it would be good to first experiment with the front end and see which lessons we learn from that.

Yes, I totally agree. I suggested we split the BackOffice in 3 parts instead of keeping the existing Admin but I didn’t mean we should do it now.
After we get this 1st Web engine with the cookies merged, I’d create one more, for example, the Checkout one on the frontoffice. And then we evolve those two and see how it goes. We can then see when is the best time to continue with the approach.


#14

I have a question about models and common libs. Your POC pull request is a great example of encapsulating code that has no dependencies. But if we want to go forward with that approach, we will have engines that share the same models or libs. Spree came up with the core module. Should we do the same? “Core” is not really a domain. And what if we have five domains and only two of them want to share a certain lib file? Do we put it in core or create another shared lib? I’m wondering if this approach will make us discuss all the time where code belongs.


#15

Hey Maikel, your questions are awesome!!! I couldn’t avoid writing a long reply, sorry about that.

I am moving all conversations to this thread: the above, the one in the cookies POC #2521 and the one in the enterprises images PR #2512.

JS and CSS include in the layout

Maikel

“Is the long-term plan that everything is in the web engine and we don’t need darkswarm/all any more?”

Luis:

To start with, I think we should keep common look&feel stuff (darkswarm and admin) in the main app as it is. Because all frontoffice domains will need the common UI code of the website, just like in the backoffice all domains will need the common UI code of Admin.

At some point in the future, these two common UI modules (website and admin) can become independent modules that other domains fetch as a dependency. For now, I’d keep everything where it is in darkswarm and admin in the main app. First we need to extract what’s not common.

External Dependencies

Maikel

“I’m worried that dependencies could be a problem (will we have duplicate gem specs in the modules?).”

Luis:

In terms of external dependencies, each domain should have its dependencies and some dependencies will be repeated across domains, I don’t think that is a problem.

API structure related to domains

Luis

In terms of api endpoints and engines we can have two options:

  • option 1 - engine name on the api endpoint - /entities/api/enterprises or /web/api/content/ or /shop/api/product (entities, web and shop being the domains identifying it’s API).
  • option 2 - no engine name on the api endpoint - all engines make endpoints at /api like /api/enterprises /api/content /api/cart

imo, I think we should aim for the first option with domain name on api endpoints (see below)

Maikel

“Do you mean /admin/api or just /api? I never thought about the mix of admin and customer code in the API before.”

Luis

Yes, great question. The first thing is: do we want to have one API or do we want to separate front office from backoffice (or maybe one API per domain).

For example for product, we need a frontoffice API for rendering products lists, product detail pages and, at some point, some search capabilities. On the other side, we need a backoffice API for product inventory management, product import, etc. In lager scale systems this is a no-brainer, you need to split. In OFN, I am not sure. I’d go for the separate APIs per domain, for example, /shop/product (in the shop domain) for front office and /catalog/product (on the catalog domain) for backoffice management. The problem with not splitting is that the single API becomes quickly complicated with all mixed usages. And, imo, this is worse than the (not much) trouble of separating the APIs from the start. Does this question make sense to you? What do you think?

If we decide to have one API only, everything under /product (for the example above), I don’t think we should ever make the API a domain/engine. Domains should represent business domains and never technical parts of the application. I’d not follow Spree’s structure of having the api engine. Each of OFNs domains should implement the part of the API that belongs to it’s bounded context. Each engine will have it’s own models, controllers, views, api and angular UI, including some rails views and it’s html controllers as well as api controllers and angular code.

Dependencies

Maikel said: “My approach would probably be to first think of the best API. If it makes sense to have domains like shop and web in the API then it would make sense to structure our code accordingly. But in the end, the API should be something stable that is independent from our internal code organisation. So our code structure should not define the API.”

Luis

There is a major point to be made here. This is why I insist on calling them domains and not just engines (engines is what we use to implement our domains). Domains are not internal code organization. That’s why they are called business domains. In this specific case for example (enterprise images), it’s not just about the best API, it’s about where this API belongs: are images being managed from the backoffice and will only be something for the backoffice or are these images going to be served in a CDN and in the frontoffice. This is a factor that should be considered in every API endpoint.

Still regarding dependencies, Maikel said : “You mentioned that engines would have their own models. That was one of my questions about this structure. Wouldn’t that lead to a lot of code duplication? I thought the idea of models is that they represent the data to manipulate which is independent of the perspective from which we look at them.”

Luis

No code duplication at all. The challenge is to find the right place for each model. That is what DDD is all about. After you find the best place for the model, all the other places in your application that need that data will have to depend on the domain that holds that data.

For example, in the backoffice, if we have entities domain (for the management of enterprises, customers, etc) and the catalog domain (for the management of product and inventory), when you build the product management screens, for example, the data related to the enterprise of the products you are managing must come from the enterprises domain. From the world of “everything in OFN can depend on everything else” we are growing to a more organised structure, but to make that step, we need to think better about our dependencies. Anyway, we are still in the theory stage, we need to actually do it, to find out the best approach.

Maikel said:

“we will have engines that share the same models… Spree came up with the core module. Should we do the same?”

Luis

I’d never create a module called Core like in Spree because it’s a meaningless name for a bucket where all the rest of the stuff ends up and no-one really knows what’s in there.

Maikel said: “And what if we have five domains and only two of them want to share a certain lib file? Do we put it in core or create another shared lib? I’m wondering if this approach will make us discuss all the time where code belongs.”

Luis

By definition, two domains should never own the same data. You need to decide in what domain each entity of your model belongs, in doubt, you put it in one of the domains and make the other depend on it. Only one owner, always. Imo, the discussion of where to put things is a good discussion and much better than having only one large bag of everything. The best part of it, there’s never a right place/domain to put stuff, we will see how our domains evolve.

In our specific case, to start with, I think the most important rule for dependencies is something like these Types of Dependencies between Domains:

RED - dependencies you don’t want to have, they mean your domains are entangled

YELLOW - they show that your domains are dependant but the dependency is contained

GREEN - they show that your domains are dependant but the dependency is very clear

And this is what I think falls into each of these 3 categories:

  • RED : dependency to Models of other domains
  • YELLOW : dependency to Services of other domains
  • GREEN : dependency to API endpoints of other domains with very clear contracts

If you need to use the DB or the model of another domain, you should try to use it’s API, if it’s too much trouble, you should use a service on that domain to fetch that data. If you can’t do that and you want to use the model of that other domain, you can still do it!, but you are keeping the two domains entangled and you are staying in the world of “everything depends on everything”.

This is for code related to business and data, for utilities (and the UI components for example) I think we should leave them in the main app for now (domains will depend on the main app for now), and at some point we can start to create shared libs outside the domains/engines.

Well, I hope this is useful. There’s no silver bullet and we will run into issues for sure. We need to go through the questions, go through the experiments and learn :slight_smile:


#16

Thank you @luisramos0. Great summary of our discussion.

I’m very happy about not replicating Spree’s engines and not having a core module.

I would like to add another question you asked on https://github.com/openfoodfoundation/openfoodnetwork/pull/2521#pullrequestreview-147395266:

how do we manage common css (as here) and common JS?
Is there an easy way (and not as ugly way as above) to make engines use css and js of the main app?
Or do we need to extract common css/js code to a new lib to be imported by the engines?
The css and JS are all available on the web page, the challenge is only for code dependencies like this scss @import or in js for code dependencies.

I think depending on the main application is like having that core bucket for everything unstructured. It also introduces circular dependencies. I would think that the best approach would be to apply the branding specific CSS (colours) within the main app. CSS within a module would be branding agnostic. But it would mean that there is CSS about the same page in two different places. :frowning:

Your description of keeping the models within a domain sounds interesting. I can imagine that it works and really makes our code cleaner. But I also think that there will be a bit of code duplication as a result. We will have an Enterprise class in the admin interface and the shop code, right? They both would need code like this:

  has_attached_file :logo,
    styles: { medium: "300x300>", small: "180x180>", thumb: "100x100>" },
    url:  '/images/enterprises/logos/:id/:style/:basename.:extension',
    path: 'public/images/enterprises/logos/:id/:style/:basename.:extension'

Is that right?


#17

re the CSS topic, I think the best approach is to keep it in the main app (which is not perfect as you mention), but only until we extract it to another common module. In this case, something like ofn-frontoffice-common-assets. There may be other things we are not seeing now but the future of the main app could well be to banish into the void as each domain becomes autonomous :slight_smile:
The split between branding and nonbranding is interesting and it could be done even between the extracted common module and the engine but do you mean to split the css files in 2 keeping the same selectors in both files? I have never seen that. Would that not be painful to maintain afterwards?

Re Enterpise class, it’s really good to go through the example.
“We will have an Enterprise class in the admin interface and the shop code, right?” I dont think so.
First, we need to decide where this entity called Enterprise belongs. Is it frontoffice/shop or is it backoffice/entities? I think this object belongs to the backoffice/entities domain where all the management will be done. The frontoffice/shop will basically grab this info and use it to build pages. SO, this model will only be in the backoffice/entities domain.
Lets suppose the domain frontoffice/shop needs Enterprise to build the Enterprise description section of the shop page. Here we have 3 types of dependencies we can use:

  • Model - RED - the frontoffice/shop code simply requires Entities::Enterprise model and does what it needs
  • Service - YELLOW - the frontoffice/shop code requires a Entities::EnterpriseService that offers methods to access and do/execute only what’s required (not the full model as in previous option, specially not a Entities::Enterprise.class_eval :-1: )
  • API - GREEN - the frontoffice/shop code uses a api endpoint on the backoffice/entities domain to fetch the data required to build the page

I think this is the way to go. And we can start having direct dependencies to models (RED), which basically means you can copy the code as is (adding the namespaces) to the engines…
In the PR, this dependency from app/helpers/footer_links_helper.rb to engines/web/lib/web/cookies_consent.rb is an example of a service dependency (it’s an easy one with no model but still a good example).


#18

But that’s not a domain. It’s like core, just a bit more specific.

In the frontoffice, we have shops which are enterprises. And they have order cycles and are connected to producers. There is a lot of dependency. Using the model seems to be the only efficient way here. We even have custom SQL. Maybe we will have a Network domain at some point that could hold these things.


#19

Yes, these would not be domains. Like core but with a specific name and specific content. A library that the domains can depend on. Engines can also be used for this as far as I understand.

Using the model seems to be the only efficient way here

Complexity and custom SQL are fine and they can be extracted from the Model to Services. Everything can be extracted to services out of the models (a good model is a very very stupid model with data coming in and out of the DB). This makes the code easier to read and maintain. But in OFN it’s quite obvious we need to start with dependencies to models, i.e., we put the existing model in one domain (backoffice in this case) and make the other domain depend on that model (just with a require). In this case, we will have a Entities::Enterprise (in the Entities domain) and both the Shop domain and the Catalog (backoffice inventory management) will depend on it. I think this is ok to start with.

Later on we can extract from this Entities::Enterprise what is not core to it, for example, code that is only used in the frontoffice could be taken from the model to a service, or, also possible, break the model in two and create a Shop::EnterpriseShop for example where you put all the enterprise frontoffice specific stuff.


#20

I am quite happy about where PR2521 is going.

Circular dependencies

With circular dependencies in the Ruby code, I couldn’t see the value of using engines versus namespaces.

I wasn’t convinced we would be able to get rid of circular dependencies, until I re-read this discussion and stopped thinking about the engines as being able to directly access each other’s code but as micro-services that will (the goal) only talk to each other through their APIs.

So, from what I understand, we would only be dealing with Ruby circular dependencies while we are transitioning the code.

API wrapper for APIs of domain-engines

For example, “FrontOffice - Catalog -> Product Search and Display” and “FrontOffice - Checkout -> Cart” will need API wrappers and resource models for Product, although simple and read-only.

I think it makes sense to have these wrappers in a separate gem (Ruby) / package (JS), and domain-engines could just use what they need.

Do you think this is okay? Maybe this is also what you already had in mind.

General utilities and behaviour

We should extract these into private gems early on, as we add code to the domain-engines, so we will be able to run tests for each engine separately from the start. Some might be able to stand on their own, and more specific code like certain methods in ApplicationController could be part of a collection named … :laughing: ofn_core.

Sorry about the name, @luisramos0! :laughing: But it wouldn’t be the same as spree_core - it would just embody OFN utilities and behaviour (and conventions) that the domain-engines could use. Importantly, it wouldn’t have any domain-engine as a dependency. We could also make it a rule that each group of behaviours in this ofn_core would be opt-in (they should not be enabled unless a domain-engine explicitly enables them through include).

Performance overhead

Related to:

I think part of the concern of @maikel here is that there would be a lot of necessary communication among domain-engines through their APIs, which would have some performance overhead.

For example, if I understand correctly, the cart page in “FrontOffice - Checkout -> Cart and Checkout” would have to fetch product names in a separate API query to “BackOffice - Catalog -> Product/Inventory management” API, a separate API query to “BackOffice - Entity Management -> Enterprises and Customers management” for customer data, and also “BackOffice - Order Management -> Orders, packing and delivery management” for OC data.

Is this correct, @luisramos0?

And we wouldn’t be able take advantage of DB joins through Rails associations if the entities belong in different engines?

On the other hand, with these micro-services and clear contracts, I think it will be easier to find places where we could improve performance. For example, because a domain-engine owns a model and there are fewer places where its data could be changed, we might be more confident when doing a code audit for caching.


#21

Great, thanks for your feedback @kristinalim!

Circular dependencies
Yes, that’s the point and the mindset. Also, even if we have dependencies between main app and domains or the other way around, most of them will not be circular, unless you count main_app/routes redirect to domain/routes to be a dependency.

API wrapper for APIs of domain-engines
I am so happy to read what you just wrote here. This is the whole point of this thread actually! Do you know why? Because these gems you talk about here, they are for clients of our API, that means, anyone can use them to interact with the OFN API.

But… I think some people will feel this may be too forward looking for OFN (mainly because it is quite a big step), so we may just accept that, for now, we can make domains depend on each others models or services or other things like AMS serializers…

General utilities and behaviour
We can call it ofn-commons :slight_smile: Or we can create many smaller/specific gems. Also, we can keep all this in the main app’s /lib folder, from what I understand this is what /lib is for.

Actually, I want to see what exact code we are talking about here, and then we can decide where to put it :slight_smile:

Performance overhead
Yes, we need to move away from the style of app where all places in the app have access to all other places. We need to create indirection layers (to reduce complexity/chaos), and that is not as fast/simple as accessing the data we need in a given place directly.

For all these topics, I think we need to create PRs, go through the changes and choose/find the best solutions. The branding CSS work you are doing in #2521 is an excellent example of that.


#22

I’m not sure I follow the topic about API wrapper for APIs of domain-engines. Do you mean @kristinalim to have API calls to read from a domain to the other? I’m assuming that we still have a single app but discrete components that have clear boundaries and contract and not separate services in separate processes.

I share @luisramos0’s view

Actually, I want to see what exact code we are talking about here, and then we can decide where to put it

For all these topics, I think we need to create PRs, go through the changes and choose/find the best solutions. The branding CSS work you are doing in #2521 is an excellent example of that.

Let’s move one and see how can make things adapt to our needs, which will change over time.


#23

@sauloperez :+1:

API wrapper for APIs of domain-engines . Do you mean @kristinalim to have API calls to read from a domain to the other?

Yes, we will have domains depending on each others models but also we will have domains communicating through each others APIs. It’s easy to imagine on the frontend if you think that a angular page on the Web domain makes an ajax call to some backoffice api endpoint to get some data. For listing product on the Web this will be a reality very easily, right? Having backend code communicating through APIs, that’s a different thing to be thought/evaluated…

I’m assuming that we still have a single app but discrete components that have clear boundaries and contract and not separate services in separate processes.

Yes, but long term, as you start to have these components independent and communicating through APIs we can decouple even the deployment. Not something for now, but something to keep in mind because the need to decouple the deployment of web frontoffice from lets say backoffice catalog will come fast… this move is really strategic/long term, think 2025 :smile: