bloom-housing / bloom

Bloom is Exygy’s affordable housing platform. Bloom's goal is to be a single entry point for affordable housing seekers and a hub for application and listing management for developers.
https://bloomhousing.com
Apache License 2.0
33 stars 26 forks source link

TypeORM discussion #1131

Closed pbn4 closed 3 years ago

pbn4 commented 3 years ago

The problem with our current setup (my subjective opinion):

  1. Excessive memory usage when querying listings and applications because of nested joins, see issues:

    The problem is two-fold: TypeORM might be better with memory usage when handling nested relations and our data model requires many JOIN operations to fetch all data requires for server response.

  2. Querybuilder ease of use, right now to join relations we have to write something like this:

    const qb = this.repository.createQueryBuilder("application")
    qb.leftJoinAndSelect("application.user", "user")
    qb.leftJoinAndSelect("application.listing", "listing")
    qb.leftJoinAndSelect("application.applicant", "applicant")
    qb.leftJoinAndSelect("applicant.address", "applicant_address")

    which I find problematic because this is not typed in any way and you have no guarantee that what's actually returned matches your runtime model, you have to check it yourself.

Potential solutions

Point 1 is crucial, point 2 is something we can live with eventually. I've been watching those linked issues for quite a bit now and there seems to be no progress in improving performance, neither do I feel competent to fix it myself and submit a PR to TypeORM repo, so we have two options:

  1. Optimize the model (less joins)
  2. Find an ORM which performs better.

Option 1. Query optimizations for Application and Listing models.

Applications

Currently we have a following query built on each application retrieval:

```
// --> Build main query
const qb = this.repository.createQueryBuilder("application")
qb.leftJoinAndSelect("application.user", "user")
qb.leftJoinAndSelect("application.listing", "listing")
qb.leftJoinAndSelect("application.applicant", "applicant")
qb.leftJoinAndSelect("applicant.address", "applicant_address")
qb.leftJoinAndSelect("applicant.workAddress", "applicant_workAddress")
qb.leftJoinAndSelect("application.alternateAddress", "alternateAddress")
qb.leftJoinAndSelect("application.mailingAddress", "mailingAddress")
qb.leftJoinAndSelect("application.alternateContact", "alternateContact")
qb.leftJoinAndSelect("alternateContact.mailingAddress", "alternateContact_mailingAddress")
qb.leftJoinAndSelect("application.accessibility", "accessibility")
qb.leftJoinAndSelect("application.demographics", "demographics")
qb.leftJoinAndSelect("application.householdMembers", "householdMembers")
qb.leftJoinAndSelect("householdMembers.address", "householdMembers_address")
qb.leftJoinAndSelect("householdMembers.workAddress", "householdMembers_workAddress")
```

I think we could optimize it slightly by removing:

Listings

We currently cannot optimize any joins:

    return Listing.createQueryBuilder("listings")
      .leftJoinAndSelect("listings.leasingAgents", "leasingAgents")
      .leftJoinAndSelect("listings.preferences", "preferences")
      .leftJoinAndSelect("listings.property", "property")
      .leftJoinAndSelect("property.buildingAddress", "buildingAddress")
      .leftJoinAndSelect("property.units", "units")
      .leftJoinAndSelect("units.amiChart", "amiChart"

so we solved long query time with http caching, which we will not be able to use when we introduce CMS (data changes should be reflected immediately in e.g. preview).

Option 2. Find TypeORM replacement:

This approach requires:

  1. Finding an ORM which:

    • has better performance in our use case scenario
    • automatically generates DB migrations
    • is typescript based
    • is actively maintained
    • supports transactions
  2. Replacing TypeORM with this new ORM:

    • writing model definition in this new ORM convention in a way that is compatible with our current DB schema
    • replacing TypeORM repositories injected in services with new db access classes

ORMs that I'm currently researching:

pbn4 commented 3 years ago

https://github.com/bloom-housing/bloom/issues/1070

pbn4 commented 3 years ago

Public app optimizations:

Backend:

pbn4 commented 3 years ago

@seanmalbert Together with @bpsushi we are having problems with exposing listing in application DTO as IdDto without joining it earlier using a query builder.

Expected model:

{
   ...
   "listing": {
     "id": "..."
   } (IdDto)
}

Current state is that we would like to use @RelationId decorator to expose foreign key column to the ORM model (without joining the actual table) and build an IdDto out of that, but DTO definition is ignoring our @Expose decorators for getter making it impossible to build listing IdDto out of listingId (relation key).

My suspicion is that since we are extending application.entity.ts entity class (with OmitType) we are inheriting decorators for keys that are omitted too and later those are passed to class-transformer (unexpectedly). What we want to try next is geting rid of OmitType extend/inheritance type of defining a DTO for an entity class and just put standard implements class there + redefine all properties in it. This way we will still retain strict type checking + it will be clear what decorators and what metadata is exactly part of our DTO definition. Hopefully that will also make exposing getters work. :)

kathyccheng commented 3 years ago

Is there more we want to do on this or can I close? @pbn4