test: harden business boundary conditions

apps/server/src/notifications/dto/notification.dto.spec.ts

@@ -1,7 +1,7 @@
 import 'reflect-metadata';
 import { plainToInstance } from 'class-transformer';
 import { validate } from 'class-validator';
-import { NotificationQueryDto } from './notification.dto';
+import { CreateNotificationDto, NotificationQueryDto } from './notification.dto';
 
 describe('NotificationQueryDto', () => {
   it('converts numeric query-string values before integer validation', async () => {
@@ -15,3 +15,21 @@ describe('NotificationQueryDto', () => {
     expect(dto.limit).toBe(20);
   });
 });
+
+describe('notification DTO boundaries', () => {
+  it('rejects an empty recipient set', async () => {
+    const dto = plainToInstance(CreateNotificationDto, {
+      recipientIds: [],
+      type: 'test',
+      title: '标题',
+    });
+    await expect(validate(dto)).resolves.toEqual(expect.arrayContaining([expect.any(Object)]));
+  });
+
+  it('rejects non-positive cursors and page sizes outside 1-100', async () => {
+    for (const value of [{ after: '0' }, { limit: '0' }, { limit: '101' }]) {
+      const dto = plainToInstance(NotificationQueryDto, value);
+      expect(await validate(dto)).not.toEqual([]);
+    }
+  });
+});

apps/server/src/notifications/dto/notification.dto.ts

@@ -1,8 +1,18 @@
-import { IsString, IsNotEmpty, IsOptional, IsArray, IsInt } from 'class-validator';
+import {
+  ArrayNotEmpty,
+  IsString,
+  IsNotEmpty,
+  IsOptional,
+  IsArray,
+  IsInt,
+  Max,
+  Min,
+} from 'class-validator';
 import { Type } from 'class-transformer';
 
 export class CreateNotificationDto {
   @IsArray()
+  @ArrayNotEmpty()
   @IsInt({ each: true })
   recipientIds: number[];
 
@@ -27,10 +37,13 @@ export class NotificationQueryDto {
   @IsOptional()
   @Type(() => Number)
   @IsInt()
+  @Min(1)
   after?: number;
 
   @IsOptional()
   @Type(() => Number)
   @IsInt()
+  @Min(1)
+  @Max(100)
   limit?: number;
 }

apps/server/src/notifications/notifications.service.spec.ts

@@ -0,0 +1,61 @@
+import { BadRequestException, NotFoundException } from '@nestjs/common';
+import { EventEmitter2 } from '@nestjs/event-emitter';
+import { NotificationsService } from './notifications.service';
+
+function createQueryBuilder() {
+  return {
+    where: jest.fn().mockReturnThis(),
+    orderBy: jest.fn().mockReturnThis(),
+    take: jest.fn().mockReturnThis(),
+    andWhere: jest.fn().mockReturnThis(),
+    getMany: jest.fn().mockResolvedValue([]),
+  };
+}
+
+describe('NotificationsService boundaries', () => {
+  it('rejects empty recipients and de-duplicates repeated recipients', async () => {
+    const repo = {
+      save: jest.fn().mockImplementation(async (rows) => rows),
+    };
+    const service = new NotificationsService(repo as never, { emit: jest.fn() } as never);
+
+    await expect(
+      service.create({ recipientIds: [], type: 'test', title: '标题' }),
+    ).rejects.toBeInstanceOf(BadRequestException);
+
+    const saved = await service.create({ recipientIds: [1, 1, 2], type: 'test', title: '标题' });
+    expect(saved).toHaveLength(2);
+    expect(repo.save).toHaveBeenCalledWith([
+      expect.objectContaining({ recipientId: 1 }),
+      expect.objectContaining({ recipientId: 2 }),
+    ]);
+  });
+
+  it('clamps service-level page size to protect callers outside the controller', async () => {
+    const qb = createQueryBuilder();
+    const service = new NotificationsService(
+      { createQueryBuilder: jest.fn().mockReturnValue(qb) } as never,
+      new EventEmitter2(),
+    );
+
+    await service.findByUser(7, undefined, 1000);
+    expect(qb.take).toHaveBeenCalledWith(100);
+  });
+
+  it('does not allow marking another user notification as read', async () => {
+    const repo = { findOne: jest.fn().mockResolvedValue(null), update: jest.fn() };
+    const service = new NotificationsService(repo as never, new EventEmitter2());
+    await expect(service.markRead(3, 7)).rejects.toBeInstanceOf(NotFoundException);
+    expect(repo.update).not.toHaveBeenCalled();
+  });
+
+  it('treats marking an already-read notification as idempotent', async () => {
+    const repo = {
+      findOne: jest.fn().mockResolvedValue({ id: 3, recipientId: 7, isRead: true }),
+      update: jest.fn(),
+    };
+    const service = new NotificationsService(repo as never, new EventEmitter2());
+    await expect(service.markRead(3, 7)).resolves.toBeUndefined();
+    expect(repo.update).not.toHaveBeenCalled();
+  });
+});

apps/server/src/notifications/notifications.service.ts

@@ -1,4 +1,4 @@
-import { Injectable } from '@nestjs/common';
+import { BadRequestException, Injectable, NotFoundException } from '@nestjs/common';
 import { InjectRepository } from '@nestjs/typeorm';
 import { Repository } from 'typeorm';
 import { Subject, Observable } from 'rxjs';
@@ -18,7 +18,10 @@ export class NotificationsService {
   ) {}
 
   async create(dto: CreateNotificationDto): Promise<Notification[]> {
-    const notifications = dto.recipientIds.map((recipientId) => ({
+    const recipientIds = [...new Set(dto.recipientIds)];
+    if (recipientIds.length === 0) throw new BadRequestException('通知接收人不能为空');
+
+    const notifications = recipientIds.map((recipientId) => ({
       recipientId,
       type: dto.type,
       title: dto.title,
@@ -36,16 +39,13 @@ export class NotificationsService {
     return saved;
   }
 
-  async findByUser(
-    userId: number,
-    after?: number,
-    limit: number = 20,
-  ): Promise<Notification[]> {
+  async findByUser(userId: number, after?: number, limit: number = 20): Promise<Notification[]> {
+    const safeLimit = Math.min(Math.max(limit, 1), 100);
     const qb = this.repo
       .createQueryBuilder('n')
       .where('n.recipientId = :userId', { userId })
       .orderBy('n.createdAt', 'DESC')
-      .take(limit);
+      .take(safeLimit);
 
     if (after !== undefined) {
       qb.andWhere('n.id < :after', { after });
@@ -61,10 +61,10 @@ export class NotificationsService {
   }
 
   async markRead(id: number, userId: number): Promise<void> {
-    await this.repo.update(
-      { id, recipientId: userId },
-      { isRead: true, readAt: new Date() },
-    );
+    const notification = await this.repo.findOne({ where: { id, recipientId: userId } });
+    if (!notification) throw new NotFoundException('通知不存在');
+    if (notification.isRead) return;
+    await this.repo.update({ id, recipientId: userId }, { isRead: true, readAt: new Date() });
   }
 
   async markAllRead(userId: number): Promise<void> {