troyanskiy/ngx-resource

Reactive resources

smnbbrv opened this issue · 8 comments

Hi @troyanskiy

I was never really a friend of an automatic resource retrieving and object mutations, so I always used observables; I love Rx and working with static objects is just going against it. So, I was always in the middle of the river: on one hand I had a neat and good library where I can use nice interfaces to work with REST, on another hand I was really suffering from calling .$observable all the time.

Since 3 months I tried a bit different approach to use the library. I did not want to rewrite / fork the whole library but just to slightly change the things to make my life a bit easier. This is what I slowly came to:

import { Resource, ResourceAction, ResourceActionBase } from 'ngx-resource';

export function ReactiveResourceAction(methodOptions?: ResourceActionBase) {
  const resourceAction = ResourceAction(methodOptions);

  return function(target: Resource, propertyKey: string) {
    resourceAction(target, propertyKey);
    const resourceMethod = target[propertyKey];

    target[propertyKey] = function(...args) {
      return resourceMethod
        .apply(this, args)
        .$observable
        .map(res => Resource.$cleanData(res));
    };
  };
}

This might seem dirty and it is. But this works perfectly as a standalone feature without changing the original library.

Below you can see e.g. the abstract CRUDLResource implementation which uses this approach.

import { RequestMethod } from '@angular/http';
import { ProtectedResource } from './protected.resource';
import { ReactiveResourceAction } from './reactive-resource-action';
import { Observable } from 'rxjs/Observable';
import { BaseListResponseDTO } from '../../dto/base/base-list-response.dto';
import { BaseDTO } from '../../dto/base/base.dto';

export class CRUDLResource<IDENTITY, OBJECT extends BaseDTO, LIST_REQUEST, LIST_OBJECT> extends ProtectedResource {

  @ReactiveResourceAction({ method: RequestMethod.Post })
  public create: (data: OBJECT) => Observable<OBJECT>;

  @ReactiveResourceAction({ method: RequestMethod.Get, path: '/{!id}' })
  public read: (data: IDENTITY) => Observable<OBJECT>;

  @ReactiveResourceAction({ method: RequestMethod.Put, path: '/{!id}' })
  public update: (data: OBJECT) => Observable<OBJECT>;

  @ReactiveResourceAction({ method: RequestMethod.Delete, path: '/{!id}' })
  public delete: (data: IDENTITY) => Observable<void>;

  @ReactiveResourceAction({ method: RequestMethod.Get })
  public list: (data: LIST_REQUEST) => Observable<BaseListResponseDTO<LIST_OBJECT>>;

  public save(data: OBJECT) {
    if (data.id) {
      return this.update(data);
    } else {
      return this.create(data);
    }
  }

}

The only thing that is missing is lazy calling HTTP methods (the original ResourceAction is of course forced to call it directly), but this, I guess, could definitely be solved even within ReactiveResourceAction by wrapping the thing with another Observable.

This works perfectly for me. What I wanted to ask here: do you see any future with having two approaches like having this one (properly implemented or even like it is posted here)? I understand that the library's purpose is probably slightly different from what I propose, this is more like a REST HTTP client. But still, if the library allows both without real effort (and it looks like it does) why can't we cover all the use cases?

If this is a wrong direction I would also understand that.

Hi @smnbbrv
As I understand your approach returns directly observable.
Currently, I'm using the library with toPromise flag which switches resource methods to return a Promise.
Here is one example of one of my resources:

@Injectable()
@ResourceParams({
  add2Provides: false,
  pathPrefix: '/user',
  toPromise: true
})
export class UserResource extends BaseResource {

  /**
   * Create new User
   */
  @ResourceAction({
    method: RequestMethod.Post,
    initResultObject: () => ({})
  })
  create: ResourceMethodPromise<IUserCreationData, any>;

  /**
   * List users
   */
  @ResourceAction({
    isArray: true
  })
  query: ResourceMethodPromise<IUserQueryParams, User[]>;


  /**
   * Get one user
   */
  @ResourceAction({
    path: '/{!_id}'
  })
  get: ResourceMethodPromise<{ _id: string }, User>;
}

So on the app I'm doing something like that

async methodToLoadDoSomethingWithUser() {
  const user = await this.userResource.get({_id: '1'});
  // here you have loaded user
  console.log(user)
}

So as I understand correctly if your approach returns (and you need) direct observable, we can imagine having toObservable flag.

Let me know what do you think?

Thank you.

This actually sounds very good especially in combination with ResourceGlobalConfig. The only thing to keep in mind here is that for the sake of Observable's idea the call should be performed lazily / only after subscribe happened (which is naturally not true for both standard / toPromise ways). Other than that - would be super!

Would a lean option value be true by default for this option? (sorry if it is already like that for toPromise I did not try it yet)

and thank you for being so available all the time :) Cannot imagine how you can do that :D

I can't imagine also ;)
So, I think lean should be false (as is not even with toPromise) by default because it will be confusing that for some flags it's true and for some false.
And yes, I agree with you about Observable which should be lazy loaded.

I will try to implement it now.

@smnbbrv Here you are
Ver 3.4.0 has been published

@Injectable()
@ResourceParams({
  add2Provides: false,
  pathPrefix: '/user',
  toObservable: true // or ResourceGlobalConfig.toObservable = true
})
export class UserResource extends BaseResource {

  /**
   * Create new User
   */
  @ResourceAction({
    method: RequestMethod.Post,
    initResultObject: () => ({})
  })
  create: ResourceMethodObservable<IUserCreationData, any>;

  /**
   * List users
   */
  @ResourceAction({
    isArray: true
  })
  query: ResourceMethodObservable<IUserQueryParams, User[]>;


  /**
   * Get one user
   */
  @ResourceAction({
    path: '/{!_id}'
  })
  get: ResourceMethodObservable<{ _id: string }, User>;
}

And yes... It's better to add lean also to the ResourceGlobalConfig. I will do that in v 3.4.1 in 5 min.

@smnbbrv voilà.
Published v 3.4.1
May I ask you to test that new toObservale for me, please?

Thanks a lot!

@troyanskiy super, danke dir!

I will let you know the result tomorrow ;)

works perfectly from what I could check. Thank you very much! Lazy load works as well