Add firebase auth destroy check before tenant operations.

[micahstairs]

This is part of the initiative to adding multi-tenancy support (see issue #332).

[hiranya911]

I think we should try to manage this within TenantManager it self. Here's what I'd suggest:

1. Add a destroy() method to the TenantManager class.
2. Call it from FirebaseAuth.destroy().
3. Handle the checkNotDestroyed() check entirely within the TenantManager class.

I think that results in a cleaner flow of control, without AbstractFirebaseAuth leaking into TenantManager. WDYT?

[micahstairs]

I think we should try to manage this within TenantManager it self. Here's what I'd suggest:

1. Add a destroy() method to the TenantManager class.
2. Call it from FirebaseAuth.destroy().
3. Handle the checkNotDestroyed() check entirely within the TenantManager class.

I think that results in a cleaner flow of control, without AbstractFirebaseAuth leaking into TenantManager. WDYT?

By #3, I assume you mean "Call it from FirebaseAuth.destroy()".

I like the idea of not leaking the firebase auth into TenantManager, however, it's not straightforward to call TenantManager.destroy() method from FirebaseAuth.destroy(), is it? I would need to override AbstractFirebaseAuth.destroy()'s method in FirebaseAuth. I believe overriding this would require me to expose the lock that it is used to synchronize on.

[micahstairs]

It's worth noting that I'm currently working on implementing TenantAwareFirebaseAuth, so perhaps that will shape our decision here.

[hiranya911]

You can do something like the following in AbstractFirebaseAuth:

protected abstract void doDestroy();

void destroy() {
   // Call doDestroy() while holding the lock
}

Now FirebaseAuth can implement doDestroy() to implement any child-specific cleanup logic.

Basically, pass control in one direction: AbstractFirebaseAuth --> FirebaseAuth --> TenantManager and never in both directions.

[micahstairs]

You can do something like the following in AbstractFirebaseAuth:

protected abstract void doDestroy();

void destroy() {
   // Call doDestroy() while holding the lock
}

Now FirebaseAuth can implement doDestroy() to implement any child-specific cleanup logic.

Basically, pass control in one direction: AbstractFirebaseAuth --> FirebaseAuth --> TenantManager and never in both directions.

Okay, this makes sense, but there's an issue. The TenantManager is lazily instantiated since it's a supplier. So if I call getTaskManager().destroy() and it hasn't been instantiated yet, then we end up invoking the destroyed app. The workaround (which I've pushed) isn't particularly elegant.

tenantManager = threadSafeMemoize(new Supplier<TenantManager>() {                               
      @Override                                                                                     
      public TenantManager get() {                                                                  
        return new TenantManager(builder.firebaseApp, getUserManager());                            
      }                                                                                             
});   

[hiranya911]

Thanks @micahstairs. Couple of suggestions on how to improve things a bit.

[hiranya911]

LGTM 👍