silverstripe / silverstripe-graphql

Serves Silverstripe data as GraphQL representations
BSD 3-Clause "New" or "Revised" License
52 stars 61 forks source link

RFC: Model bulk loading #367

Open unclecheese opened 3 years ago

unclecheese commented 3 years ago

Summary

In the interest of the 80% case that just want "a content API", we should make exposing a group of models with common configurations as simple as possible.

schemas:
  default:
    moduleLoader:
       namespace: 'MyProject\Models\*'
       fields: '*'
       operations: '*'

Proposed implementation

This would hinge on two new abstraction layers:

Implementations for collections might include:

Implementations for filters might include:

Example

schemas:
  default:
    modelCollectors:
      namespaceLoader:
        include: [ 'MyProject\Models\*' ]
        filters:
          matchFilter: [ 'My\Poject\Models\MySpecialModel' ]
      filesystemLoader:
        include:
          -  'app/src/**/Models' 
          -  'somevendor/somemodule: src/Models/*'
        filters:
          subclassFilter: [ 'MyApp\Test\TestObject' ]
      extensionLoader:
        include: ['SilverStripe\Versioned\Versioned']

Granted, most of the time this would be pretty straightforward -- subclass of DataObject collector, filter a few exact matches, and you're good to go. But we should make this pretty flexible for the edge cases.

Contract would look something like:

public function collect(string[] $keys): void;
public function filter(string $className, SplFileInfo $file): bool;
michalkleiner commented 2 years ago

Yep, looks sensible to me. I'd add an exclude mechanism as well so that the include part would create the set and then exclude would remove from it. Or add a not: true negation. Would also want to allow multiple instances of the same loader, so they wouldn't be able to be the keys of the list of collectors.

LiamKearn commented 2 years ago

+1 for this. Could be really really nice QoL.

unclecheese commented 2 years ago

@michalkleiner Working on this now and trying to find a reason why you'd need multiple instances of the same loader if they allow include and exclude as arrays.

michalkleiner commented 2 years ago

@unclecheese didn't have a particular use case in mind but everytime I see a class being the key of an array, it triggers a warning light for me because it means it can only be used once.

The difference would only be this (note the extra dashes):

schemas:
  default:
    modelCollectors:
      - namespaceLoader:
          include: [ 'MyProject\Models\*' ]
          filters:
            matchFilter: [ 'My\Poject\Models\MySpecialModel' ]
      - filesystemLoader:
          include:
            - 'app/src/**/Models' 
            - 'somevendor/somemodule: src/Models/*'
          filters:
            subclassFilter: [ 'MyApp\Test\TestObject' ]
      - extensionLoader:
          include: ['SilverStripe\Versioned\Versioned']

With this, the modelCollectors becomes an array of objects/arrays where the classname is still a key, but there's no collision on the model collectors level.

michalkleiner commented 2 years ago

I see it as a simpler/safer way of defining multiple, possibly overlapping rules. At the same time might be just overkill, though it's how I'd do it.

unclecheese commented 2 years ago

This is how I've got it right now.. let me know what you think:

_graphql/bulkLoad.yml

elemental: # arbitrary key describing the section of the bulk load
  load:
    subclassLoader:
      include:
        - DNADesign\Elemental\Models\BaseElement
  apply:
    fields:
      '*': true
    operations:
      read: true
      readOne: true
app:
  load:
    namespaceLoader:
      include:
        - 'App\Models\*'
  apply:      
    fields:
      '*': true
    operations:
      '*': true
michalkleiner commented 2 years ago

That would work as well since you can create any number of arbitrary top-level keys.

unclecheese commented 2 years ago

Draft PR is up https://github.com/silverstripe/silverstripe-graphql/pull/432