loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.95k stars 1.06k forks source link

Read only properties #2454

Open yiev opened 5 years ago

yiev commented 5 years ago

Are there any plans of implementing this feature?

I saw a request for LB3: https://github.com/strongloop/loopback/issues/531 But there seems to be no information about this feature for LB4.

Also if anyone knows any workaround for this feature, please share it. 😃

I tried to delete the property that I want to be read only in my PATCH function with delete project.name but this lead to an error. My datasource is a MySQL database and obviously the SQL statement is created wrong, if you just delete the property.

zhaoqin-github commented 5 years ago

I attempted to define a 'readOnly' property using OpenAPI 3 schema, https://swagger.io/docs/specification/data-models/keywords/, and use that schema object to validate request body of POST method.

const abcSchema = {
  description: 'ABC',
  required: true,
  content: {
    'application/json': {
      schema: {
        type: 'object',
        required: [
          'name'
        ],
        properties: {
          id: {
            type: 'string',
            readOnly: true
          },
          name: {
            type: 'string',
          }
        }
      }
    }
  }
}

However, 'readOnly' does not work. Other OpenAPI keywords like enum can work fine. Is there any solution to implement a 'readOnly' property in LB4?

acrodrig commented 5 years ago

Having same issue. Please let me know if you find a workaround.

yiev commented 5 years ago

I came up with two possible workarounds. Both not very nice. The first one is to select the "old" element first from the repository before updating the element. Then overwriting the readonly value with the old one. So that if the user sends any value, it will just be overwritten. Something like this in the controller:

  @patch(`someurl/things/{id}`, {
    responses: {
      '200': {
        description: 'Thing PATCH success'
      }
    }
  })
  async updateById(@param.path.number('id') id: number, @requestBody() thing: Thing): Promise<Thing>   {
    const oldThing = await this.thingRepo.findById(id);
    thing.name = oldThing.name;  // This is the readonly property

    await this.thingRepo.updateById(id, thing);
    return await this.thingRepo.findById(id);
  }

But this way you are doing on every UPDATE also a SELECT... And you also can't update many items at once with an updateAll. You can only update single items.

Another possible, a bit better, but still not nice, workaround is to write your own update-function in the repository and do the update in this function via SQL-Query.

For example this function in the repository:

  public async updateByIdReadonly(thingId: number, thing: Thing) {
    return await this.dataSource.execute(
      `UPDATE
      ..... // Update all fields except the readonly ones
      FROM Thing WHERE Thing.id = ${thingId}`, []);
  }

I will use the second solution for my case. As I don't need an updateAll function, it won't get too complicated to put the SQL query together.

bseib commented 4 years ago

I wanted to share a workaround that I got working to implement read only properties as discussed here. It may address #3325 too. This workaround achieves the following:

1) it allows you to put readOnly in any @property() of your model 2) the schema generated from the controller for OpenAPI won't show any readOnly fields for "write" operations (POST, PUT, PATCH), but will show them for "read" operations ('GET'). 3) the schema generated for purposes of validating incoming request body (for POST, PUT, PATCH) will automatically exclude[] fields that are marked readOnly. (If someone does a POST and passes one of these fields, they will get a 422 validation failure.)

The way it works is by replacing the getModelSchemaRef function in your controller. That function will call the real getModelSchemaRef function, but then augment the resulting SchemaRef with the readOnly flags in the right places.

Likewise there is a second function getModelSchemaRefForRequestBody that you use whenever you are generating a schema inside a @requestBody, which ultimately is the schema used for validation of incoming JSON.

I believe these two schemas must be different.

Example controller snippet:

// [...]

import {
  getModelSchemaRef,
  getModelSchemaRefForRequestBody,
} from '../lb4-workarounds';

// [...]

@authorize({allowedRoles: [AUTHENTICATED]})
export class TimestampTestController {
  constructor(
    @repository(TimestampTestRepository)
    public timestampTestRepository: TimestampTestRepository,
  ) {}

  @post('/timestamp-test', {
    responses: {
      '200': {
        description: 'TimestampTest model instance',
        content: {
          'application/json': {
            schema: getModelSchemaRef(TimestampTest),
          },
        },
      },
    },
  })
  async create(
    @requestBody({
      content: {
        'application/json': {
          schema: getModelSchemaRefForRequestBody(TimestampTest, {
            title: 'NewTimestampTest',
          }),
        },
      },
    })
    timestampTest: TimestampTest,
  ): Promise<TimestampTest> {
    const now = new Date();
    timestampTest.sysTimestamp = now;
    timestampTest.sysTimestampTz = now;
    return this.timestampTestRepository.create(timestampTest);
  }

// [...]

And in the model, a few fields are marked read only. Take note that the testId field is marked as readOnly: true, but also required: false. In the database, this column has a default to get nextval() from a sequence. So we are correct to not require it as we don't want it supplied.

@model({
  settings: {idInjection: false, postgresql: {schema: 'public', table: 'timestamp_test'}}
})
export class TimestampTest extends Entity {
  @property({
    type: 'number',
    required: false,
    scale: 0,
    id: 1,
    readOnly: true,
    postgresql: {columnName: 'test_id', dataType: 'integer', dataLength: null, dataPrecision: null, dataScale: 0, nullable: 'NO'},
  })
  testId: number;

  @property({
    type: 'number',
    required: true,
    scale: 0,
    postgresql: {columnName: 'as_integer', dataType: 'integer', dataLength: null, dataPrecision: null, dataScale: 0, nullable: 'NO'},
  })
  asInteger: number;

// [...]

  @property({
    type: 'string',
    length: 64,
    postgresql: {columnName: 'comment', dataType: 'character varying', dataLength: 64, dataPrecision: null, dataScale: null, nullable: 'YES'},
  })
  comment?: string;

  @property({
    type: 'date',
    readOnly: true,
    postgresql: {columnName: 'sys_timestamp', dataType: 'timestamp without time zone', dataLength: null, dataPrecision: null, dataScale: null, nullable: 'NO'},
  })
  sysTimestamp: Date;

  @property({
    type: 'date',
    readOnly: true,
    postgresql: {columnName: 'sys_timestamp_tz', dataType: 'timestamp with time zone', dataLength: null, dataPrecision: null, dataScale: null, nullable: 'NO'},
  })
  sysTimestampTz: Date;

// [...]
}

The workaround code lb4-workarounds.ts in its entirety:

import {
  SchemaRef,
  SchemasObject,
  SchemaObject,
  getModelSchemaRef as originalGetModelSchemaRef,
  JsonSchemaOptions,
} from '@loopback/rest';
import {ModelDefinition} from '@loopback/repository';
//import {Entity} from '@loopback/repository';

export function getModelSchemaRef<T extends object>(
  modelCtor: Function & {
    prototype: T;
  },
  options?: JsonSchemaOptions<T>,
): SchemaRef {
  const schema: SchemaRef = originalGetModelSchemaRef(modelCtor, options);

  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  const modelDefinition = getModelDefinition(modelCtor);
  const schemaName = options?.title ?? modelDefinition.name;
  const schemaDefinitions: SchemasObject = schema.definitions;
  const schemaObject: SchemaObject = schemaDefinitions[schemaName];
  if (schemaObject) {
    const modelProperties = modelDefinition.properties;
    const schemaProperties = schemaObject.properties;
    if (schemaProperties) {
      Object.keys(modelProperties).forEach(propName => {
        if (
          modelProperties[propName]['readOnly'] &&
          schemaProperties[propName]
        ) {
          console.log(`setting property ${schemaName}.${propName} to readOnly`);
          (schemaProperties[propName] as SchemaObject).readOnly = true;
        }
      });
    }
  }
  //  console.log(JSON.stringify(schema, null, 2));
  return schema;
}

export function getModelSchemaRefForRequestBody<T extends object>(
  modelCtor: Function & {
    prototype: T;
  },
  options?: JsonSchemaOptions<T>,
): SchemaRef {
  const excludes: (keyof T)[] = [];
  const modelDefinition = getModelDefinition(modelCtor);
  const modelProperties = modelDefinition.properties;
  if (modelProperties) {
    Object.keys(modelProperties).forEach(propName => {
      if (modelProperties[propName]['readOnly']) {
        excludes.push(propName as keyof T);
      }
    });
  }
  const mergedOptions = mergeExcludesOptions(excludes, options);
  return getModelSchemaRef(modelCtor, mergedOptions);
}

function mergeExcludesOptions<T extends Object>(
  excludes: (keyof T)[],
  options?: JsonSchemaOptions<T>,
): JsonSchemaOptions<T> {
  const mergedOptions: JsonSchemaOptions<T> = {...options};
  const existingExcludes: (keyof T)[] = mergedOptions.exclude ?? [];
  const mergedExcludes: (keyof T)[] = [
    ...new Set([...existingExcludes, ...excludes]),
  ];
  mergedOptions.exclude = mergedExcludes;
  return mergedOptions;
}

function getModelDefinition<T extends object>(
  modelCtor: Function & {
    prototype: T;
  },
): ModelDefinition {
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  return (modelCtor as any)?.definition;
}

I'm sure this code could be better -- I got lazy and used any in there a few places to get things going. I did try to dig into the schema generation code to figure out how to do all of this properly and then open a PR, but then had to get something working more quickly for the short term. Any pointers on the correct place this would go in the code would be helpful.

The other conclusion I came to is that the there are two consumers of the generated schema: AJV validators, and OpenAPI. But I'm not sure they can consume the same schema and accomplish the (my) desired outcome. The only way to get AJV to fail when attempting to write a property marked readOnly is to not have that property exist in the schema, thus exclude[] it is the answer. But for OpenAPI, we want all readOnly fields to show up for all the GET operations. I could be wrong in my assessment of this situation.

Anyhow, I thought I'd share my current approach to this issue.

lajosf commented 4 years ago

Dear @deepakrkris, Are you working on the issue, or is it planned to be started near in the future? Thanks, Lajos

lajosf commented 4 years ago

Dear @raymondfeng, @bajtos , can you please help on this ticket to be solved some time? It would be nice.

stale[bot] commented 3 years ago

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

aaqilniz commented 12 months ago

I have opened up a PR here for the feature.