Merge pull request #563 from Hexastack/562-issue-strict-null-check

feat: implement strict null checks
Hexastack · Jan 15, 2025 · 47e8056 · 47e8056
2 parents 0211153 + 7186d95
commit 47e8056
Show file tree

Hide file tree

Showing 22 changed files with 130 additions and 89 deletions.
diff --git a/api/src/attachment/controllers/attachment.controller.spec.ts b/api/src/attachment/controllers/attachment.controller.spec.ts
@@ -27,7 +27,7 @@ import {
 
 import { attachment, attachmentFile } from '../mocks/attachment.mock';
 import { AttachmentRepository } from '../repositories/attachment.repository';
-import { AttachmentModel, Attachment } from '../schemas/attachment.schema';
+import { Attachment, AttachmentModel } from '../schemas/attachment.schema';
 import { AttachmentService } from '../services/attachment.service';
 
 import { AttachmentController } from './attachment.controller';
@@ -55,14 +55,12 @@ describe('AttachmentController', () => {
     attachmentController =
       module.get<AttachmentController>(AttachmentController);
     attachmentService = module.get<AttachmentService>(AttachmentService);
-    attachmentToDelete = await attachmentService.findOne({
+    attachmentToDelete = (await attachmentService.findOne({
       name: 'store1.jpg',
-    });
+    }))!;
   });
 
-  afterAll(async () => {
-    await closeInMongodConnection();
-  });
+  afterAll(closeInMongodConnection);
 
   afterEach(jest.clearAllMocks);
 
@@ -79,7 +77,7 @@ describe('AttachmentController', () => {
   describe('Upload', () => {
     it('should throw BadRequestException if no file is selected to be uploaded', async () => {
       const promiseResult = attachmentController.uploadFile({
-        file: undefined,
+        file: [],
       });
       await expect(promiseResult).rejects.toThrow(
         new BadRequestException('No file was selected'),
@@ -117,17 +115,17 @@ describe('AttachmentController', () => {
 
     it('should download the attachment by id', async () => {
       jest.spyOn(attachmentService, 'findOne');
-      const storedAttachment = await attachmentService.findOne({
+      const storedAttachment = (await attachmentService.findOne({
         name: 'store1.jpg',
-      });
+      }))!;
       const result = await attachmentController.download({
         id: storedAttachment.id,
       });
 
       expect(attachmentService.findOne).toHaveBeenCalledWith(
         storedAttachment.id,
       );
-      expect(result.options).toEqual({
+      expect(result?.options).toEqual({
         type: storedAttachment.type,
         length: storedAttachment.size,
         disposition: `attachment; filename="${encodeURIComponent(

diff --git a/api/src/attachment/services/attachment.service.ts b/api/src/attachment/services/attachment.service.ts
@@ -256,9 +256,15 @@ export class AttachmentService extends BaseService<Attachment> {
   async download(
     attachment: Attachment,
     rootDir = config.parameters.uploadDir,
-  ) {
+  ): Promise<StreamableFile> {
     if (this.getStoragePlugin()) {
-      return await this.getStoragePlugin()?.download(attachment);
+      const streamableFile =
+        await this.getStoragePlugin()?.download(attachment);
+      if (!streamableFile) {
+        throw new NotFoundException('No file was found');
+      }
+
+      return streamableFile;
     } else {
       const path = resolve(join(rootDir, attachment.location));
 

diff --git a/api/src/extensions/helpers/llm-nlu/index.helper.ts b/api/src/extensions/helpers/llm-nlu/index.helper.ts
@@ -100,40 +100,39 @@ export default class LlmNluHelper
    *
    * @returns An array of objects representing the found entities, with their `value`, `start`, and `end` positions.
    */
-  private findKeywordEntities(
-    text: string,
-    entity: NlpEntityFull,
-  ): NLU.ParseEntity[] {
-    return entity.values
-      .flatMap(({ value, expressions }) => {
-        const allValues = [value, ...expressions];
-
-        // Filter the terms that are found in the text
-        return allValues
-          .flatMap((term) => {
-            const regex = new RegExp(`\\b${term}\\b`, 'g');
-            const matches = [...text.matchAll(regex)];
-
-            // Map matches to FoundEntity format
-            return matches.map((match) => ({
-              entity: entity.name,
-              value: term,
-              start: match.index!,
-              end: match.index! + term.length,
-              confidence: 1,
-            }));
-          })
-          .shift();
-      })
-      .filter((v) => !!v);
+  private findKeywordEntities(text: string, entity: NlpEntityFull) {
+    return (
+      entity.values
+        .flatMap(({ value, expressions }) => {
+          const allValues = [value, ...expressions];
+
+          // Filter the terms that are found in the text
+          return allValues
+            .flatMap((term) => {
+              const regex = new RegExp(`\\b${term}\\b`, 'g');
+              const matches = [...text.matchAll(regex)];
+
+              // Map matches to FoundEntity format
+              return matches.map((match) => ({
+                entity: entity.name,
+                value: term,
+                start: match.index!,
+                end: match.index! + term.length,
+                confidence: 1,
+              }));
+            })
+            .shift();
+        })
+        .filter((v) => !!v) || []
+    );
   }
 
   async predict(text: string): Promise<NLU.ParseEntities> {
     const settings = await this.getSettings();
     const helper = await this.helperService.getDefaultLlmHelper();
     const defaultLanguage = await this.languageService.getDefaultLanguage();
     // Detect language
-    const language = await helper.generateStructuredResponse<string>(
+    const language = await helper.generateStructuredResponse<string>?.(
       `input text: ${text}`,
       settings.model,
       this.languageClassifierPrompt,
@@ -147,13 +146,13 @@ export default class LlmNluHelper
       {
         entity: 'language',
         value: language || defaultLanguage.code,
-        confidence: undefined,
+        confidence: 100,
       },
     ];
     for await (const { name, doc, prompt, values } of this
       .traitClassifierPrompts) {
       const allowedValues = values.map(({ value }) => value);
-      const result = await helper.generateStructuredResponse<string>(
+      const result = await helper.generateStructuredResponse<string>?.(
         `input text: ${text}`,
         settings.model,
         prompt,
@@ -163,12 +162,13 @@ export default class LlmNluHelper
           enum: allowedValues.concat('unknown'),
         },
       );
-      const safeValue = result.toLowerCase().trim();
-      const value = allowedValues.includes(safeValue) ? safeValue : '';
+      const safeValue = result?.toLowerCase().trim();
+      const value =
+        safeValue && allowedValues.includes(safeValue) ? safeValue : '';
       traits.push({
         entity: name,
         value,
-        confidence: undefined,
+        confidence: 100,
       });
     }
 
@@ -179,7 +179,7 @@ export default class LlmNluHelper
     });
     const entities = keywordEntities.flatMap((keywordEntity) =>
       this.findKeywordEntities(text, keywordEntity),
-    );
+    ) as NLU.ParseEntity[];
 
     return { entities: traits.concat(entities) };
   }

diff --git a/api/src/migration/migration.service.spec.ts b/api/src/migration/migration.service.spec.ts
@@ -191,7 +191,7 @@ describe('MigrationService', () => {
 
       await service.run({
         action: MigrationAction.UP,
-        version: null,
+        version: undefined,
         isAutoMigrate: false,
       });
 

diff --git a/api/src/migration/migration.service.ts b/api/src/migration/migration.service.ts
@@ -29,6 +29,7 @@ import { Migration, MigrationDocument } from './migration.schema';
 import {
   MigrationAction,
   MigrationName,
+  MigrationRunOneParams,
   MigrationRunParams,
   MigrationSuccessCallback,
   MigrationVersion,
@@ -239,7 +240,7 @@ module.exports = {
    *
    * @returns Resolves when the migration action is successfully executed or stops if the migration already exists.
    */
-  private async runOne({ version, action }: MigrationRunParams) {
+  private async runOne({ version, action }: MigrationRunOneParams) {
     // Verify DB status
     const { exist, migrationDocument } = await this.verifyStatus({
       version,
@@ -258,7 +259,7 @@ module.exports = {
         attachmentService: this.attachmentService,
       });
 
-      if (result) {
+      if (result && migrationDocument) {
         await this.successCallback({
           version,
           action,

diff --git a/api/src/migration/migrations/1735836154221-v-2-2-0.migration.ts b/api/src/migration/migrations/1735836154221-v-2-2-0.migration.ts
@@ -442,7 +442,10 @@ const migrateAttachmentMessages = async ({
               type: response.headers['content-type'],
               channel: {},
             });
-            await updateAttachmentId(msg._id, attachment.id);
+
+            if (attachment) {
+              await updateAttachmentId(msg._id, attachment.id);
+            }
           }
         } else {
           logger.warn(

diff --git a/api/src/migration/types.ts b/api/src/migration/types.ts
@@ -28,6 +28,10 @@ export interface MigrationRunParams {
   isAutoMigrate?: boolean;
 }
 
+export interface MigrationRunOneParams extends MigrationRunParams {
+  version: MigrationVersion;
+}
+
 export interface MigrationSuccessCallback extends MigrationRunParams {
   migrationDocument: MigrationDocument;
 }

diff --git a/api/src/nlp/controllers/nlp-entity.controller.spec.ts b/api/src/nlp/controllers/nlp-entity.controller.spec.ts
@@ -109,7 +109,7 @@ describe('NlpEntityController', () => {
           acc.push({
             ...curr,
             values: nlpValueFixtures.filter(
-              ({ entity }) => parseInt(entity) === index,
+              ({ entity }) => parseInt(entity!) === index,
             ) as NlpEntityFull['values'],
             lookups: curr.lookups!,
             builtin: curr.builtin!,

diff --git a/api/src/nlp/controllers/nlp-value.controller.spec.ts b/api/src/nlp/controllers/nlp-value.controller.spec.ts
@@ -98,7 +98,7 @@ describe('NlpValueController', () => {
           acc.push({
             ...curr,
             entity: nlpEntityFixtures[
-              parseInt(curr.entity)
+              parseInt(curr.entity!)
             ] as NlpValueFull['entity'],
             builtin: curr.builtin!,
             expressions: curr.expressions!,
@@ -125,15 +125,15 @@ describe('NlpValueController', () => {
         (acc, curr) => {
           const ValueWithEntities = {
             ...curr,
-            entity: nlpEntities[parseInt(curr.entity)].id,
+            entity: curr.entity ? nlpEntities[parseInt(curr.entity!)].id : null,
             expressions: curr.expressions!,
             metadata: curr.metadata!,
             builtin: curr.builtin!,
           };
           acc.push(ValueWithEntities);
           return acc;
         },
-        [] as TFixtures<NlpValue>[],
+        [] as TFixtures<NlpValueCreateDto>[],
       );
       expect(result).toEqualPayload(nlpValueFixturesWithEntities);
     });

diff --git a/api/src/nlp/controllers/nlp-value.controller.ts b/api/src/nlp/controllers/nlp-value.controller.ts
@@ -75,8 +75,9 @@ export class NlpValueController extends BaseController<
     this.validate({
       dto: createNlpValueDto,
       allowedIds: {
-        entity: (await this.nlpEntityService.findOne(createNlpValueDto.entity))
-          ?.id,
+        entity: createNlpValueDto.entity
+          ? (await this.nlpEntityService.findOne(createNlpValueDto.entity))?.id
+          : null,
       },
     });
     return await this.nlpValueService.create(createNlpValueDto);

diff --git a/api/src/nlp/dto/nlp-value.dto.ts b/api/src/nlp/dto/nlp-value.dto.ts
@@ -6,7 +6,6 @@
  * 2. All derivative works must include clear attribution to the original creator and software, Hexastack and Hexabot, in a prominent location (e.g., in the software's "About" section, documentation, and README file).
  */
 
-import { PartialType } from '@nestjs/mapped-types';
 import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
 import {
   IsArray,
@@ -49,11 +48,42 @@ export class NlpValueCreateDto {
   @IsString()
   @IsNotEmpty()
   @IsObjectId({ message: 'Entity must be a valid ObjectId' })
-  entity: string;
+  entity: string | null;
 }
 
-export class NlpValueUpdateDto extends PartialType(NlpValueCreateDto) {}
+export class NlpValueUpdateDto {
+  @ApiPropertyOptional({ description: 'Foreign ID', type: String })
+  @IsOptional()
+  @IsString()
+  foreign_id?: string;
+
+  @ApiPropertyOptional({ description: 'Nlp value', type: String })
+  @IsOptional()
+  @IsString()
+  value?: string;
+
+  @ApiPropertyOptional({
+    description: 'Nlp value expressions',
+    isArray: true,
+    type: Array,
+  })
+  @IsOptional()
+  @IsArray()
+  expressions?: string[];
+
+  @ApiPropertyOptional({ description: 'Nlp value entity', type: String })
+  @IsOptional()
+  @IsString()
+  @IsObjectId({ message: 'Entity must be a valid ObjectId' })
+  entity?: string | null;
+
+  @ApiPropertyOptional({ description: 'Nlp value is builtin', type: Boolean })
+  @IsOptional()
+  @IsBoolean()
+  builtin?: boolean;
+}
 
 export type NlpValueDto = DtoConfig<{
   create: NlpValueCreateDto;
+  update: NlpValueUpdateDto;
 }>;
diff --git a/api/src/nlp/repositories/nlp-value.repository.spec.ts b/api/src/nlp/repositories/nlp-value.repository.spec.ts
@@ -87,7 +87,7 @@ describe('NlpValueRepository', () => {
           const ValueWithEntities = {
             ...curr,
             entity: nlpEntityFixtures[
-              parseInt(curr.entity)
+              parseInt(curr.entity!)
             ] as NlpValueFull['entity'],
             builtin: curr.builtin!,
             expressions: curr.expressions!,

diff --git a/api/src/nlp/seeds/nlp-value.seed.ts b/api/src/nlp/seeds/nlp-value.seed.ts
@@ -38,7 +38,7 @@ export class NlpValueSeeder extends BaseSeeder<
       const entities = await this.nlpEntityRepository.findAll();
       const modelDtos = models.map((v) => ({
         ...v,
-        entity: entities.find(({ name }) => name === v.entity)?.id,
+        entity: entities.find(({ name }) => name === v.entity)?.id || null,
       }));
       await this.repository.createMany(modelDtos);
       return true;

diff --git a/api/src/nlp/services/nlp-value.service.spec.ts b/api/src/nlp/services/nlp-value.service.spec.ts
@@ -69,9 +69,7 @@ describe('NlpValueService', () => {
     nlpValues = await nlpValueRepository.findAll();
   });
 
-  afterAll(async () => {
-    await closeInMongodConnection();
-  });
+  afterAll(closeInMongodConnection);
 
   afterEach(jest.clearAllMocks);
 
@@ -94,9 +92,7 @@ describe('NlpValueService', () => {
         (acc, curr) => {
           const ValueWithEntities = {
             ...curr,
-            entity: nlpEntityFixtures[
-              parseInt(curr.entity)
-            ] as NlpValueFull['entity'],
+            entity: nlpEntityFixtures[parseInt(curr.entity!)] as NlpEntity,
             expressions: curr.expressions!,
             metadata: curr.metadata!,
             builtin: curr.builtin!,