Azure Table Storage: repository method findAll() add optional filters

[lufinima]

I'm submitting a...

[ ] Regression 
[ ] Bug report
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

When we try to use the findAll() method we can't use any filter

Expected behavior

We should be able to use at least the Azure Table Storage supported odata filters At least the filter could be optional and would be up to the user to use it or not.

Minimal reproduction of the problem with instructions

What is the motivation / use case for changing the behavior?

As an example without a filter for instance PartitionKey eq 'some-partition-key' we would need to retrieve all records from table then filter on the client side by the desired partition and this is really inefficient.

Environment

Nest version: X.Y.Z


For Tooling issues:
- Node version: XX  
- Platform:  

Others:

[nlaurie]

I just ran into this same issue , absolutely need to filter by partitionKey at a minimum

[nlaurie]

I added a PR to fix this , I been using filtering directly to azure for some time now

https://github.com/nestjs/azure-database/pull/1055

[lufinima]

I added a PR to fix this , I been using filtering directly to azure for some time now

1055

For backwards compatibility I would make the options parameter optional if and actual options is passed so then it would use it, if not would works as is right now. My take on it was like this as I don't need other thing than the filter to be different:

async findAll(filter?: string): Promise<T[]> {
    logger.debug(`Looking for entities in table: ${this.tableName}`);

    try {
      let options: ListTableEntitiesOptions = {};
      if (typeof filter !== 'undefined') {
        options = {
          queryOptions: { filter: `${filter}`, select: [] },
        };
      }

      const records = this.tableClientInstance.listEntities(options);
      ...

[nlaurie]

From what I saw in the code, there was no filter?:string param in the latest code.

async findAll(): Promise<T[]> {

The way I did it was backwards compatible and provides the full support to the listEntities() including the select which is important when needing to delete lots of large rows.

async findAll(options: { queryOptions?: TableEntityQueryOptions } = {}): Promise<T[]>

I use "select" to query and return just the rowKey using the select:['RowKey']. and iterate the results to do targeted deletes. Important when storing large blob data.

I can change it to the question mark notation as opposed to the default , if thats what you want. would be equally backward compatible. I like to use a generic options param because it gives you increased flexibility to adding additional backward compatible features in the future.

[nlaurie]

Here is a patch if they don't accept the PR anytime soon , just use findAll2

import { TableEntityQueryOptions } from '@azure/data-tables';
import { AzureTableStorageRepository } from '@nestjs/azure-database';
import { Logger } from '@nestjs/common';

export * from '@nestjs/azure-database';

const logger = new Logger('AzureStorageRepository');

// Remove the verbose output
// eslint-disable-next-line @typescript-eslint/no-empty-function
console.table = function () {};

declare module '@nestjs/azure-database' {
  interface AzureTableStorageRepository<T> {
    findAll2(options?: { queryOptions?: TableEntityQueryOptions }): Promise<T[]>;
  }
}

AzureTableStorageRepository.prototype.findAll2 = async function <T>(
  options: { queryOptions?: TableEntityQueryOptions } = {}
): Promise<T[]> {
  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
  // @ts-ignore
  logger.debug(`Looking for entities in table: ${this.tableName}`);

  try {
    const records = this.tableClientInstance.listEntities({
      queryOptions: options.queryOptions,
    });

    const entities = [];
    for await (const entity of records) {
      entities.push(entity);
    }

    logger.debug(`Entities fetched successfully`);
    return entities as T[];
  } catch (error) {
    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
    // @ts-ignore
    return this.handleRestErrors(error);
  }
};

[manekinekko]

Thank you @nlaurie for your PR. Changes are now merged in main branch https://github.com/nestjs/azure-database/commit/d0868a0836ad08db82796606403974f6042dd262