Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

장소 저장할 때 ES에도 저장 #164

Merged
merged 17 commits into from
Nov 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
b938e05
config: typeorm-transactional 모듈 설치 #163
koomchang Nov 21, 2024
108a13a
config: nestjs에 typeorm-transactional 이용을 위한 환경 설정 #163
koomchang Nov 21, 2024
5f56fe0
feat: es에 장소 저장할 수 있는 기능추가 #163
koomchang Nov 21, 2024
0d91f7d
feat: 장소 추가할 때 MySQL, ES에 모두 저장되도록 실패할 시 트랜잭션 적용까지 처리 #163
koomchang Nov 21, 2024
68f9947
config: NestJS EventEmitter 모듈 설치 #163
koomchang Nov 21, 2024
35079ed
feat: 장소 저장할 때 ES는 event 호출로 분리 #163
koomchang Nov 21, 2024
255aacc
fix: es 장소 저장 반환값 수정 #163
koomchang Nov 21, 2024
6d17f6a
test: 테스트 환경에서 DataSource 트랜잭션 설정 #163
koomchang Nov 21, 2024
c3c3c10
fix: 의존성 때문에 테스트 실패하는 버그 수정 #163
koomchang Nov 22, 2024
cee82da
fix: 테스트에서 로거를 확인 못하는 현상 수정 #163
koomchang Nov 23, 2024
2bd74aa
feat: nestjs schedule 모듈 설치 #163
koomchang Nov 23, 2024
adb4d87
feat: 생성, 수정, 삭제 2시간 이내에 변경된 데이터들을 1시간 마다 한번씩 ES에 동기화하는 배치 구성 #163
koomchang Nov 23, 2024
833cfec
feat: es 에 장소 저장 실패하는 경우 에러 로그 추가 #163
koomchang Nov 23, 2024
16dacf6
refactor: 불분명한 의미의 변수명 수정 #163
koomchang Nov 23, 2024
5613a4a
feat: 코스 사용자 확인 권한에서 admin은 모두 통과할 수 있도록 설정 #163
koomchang Nov 23, 2024
aee978c
refactor: lat, lon 네이밍을 줄이지 않고 사용하도록 변경 #163
koomchang Nov 23, 2024
33a1c63
config: yarn.lock 업데이트 #163
koomchang Nov 24, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"@nestjs/platform-express": "^10.0.0",
"@nestjs/typeorm": "^10.0.0",
"@nestjs/throttler": "^6.2.1",
"@nestjs/schedule": "^4.1.1",
"class-transformer": "^0.5.1",
"class-validator": "^0.14.1",
"google-auth-library": "^9.14.2",
Expand All @@ -34,6 +35,7 @@
"pino-socket": "^7.4.0",
"rxjs": "^7.8.1",
"typeorm": "^0.3.20",
"typeorm-transactional": "^0.5.0",
"@nestjs/elasticsearch": "^10.0.2",
"@elastic/elasticsearch": "^8.16.0"
},
Expand Down
2 changes: 2 additions & 0 deletions backend/src/admin/admin.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,7 @@ import { AuthModule } from '@src/auth/auth.module';
},
]),
],
providers: [AdminGuard],
exports: [AdminGuard],
})
export class AdminModule {}
13 changes: 8 additions & 5 deletions backend/src/admin/guard/AdminGuard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@ export class AdminGuard extends JwtAuthGuard {
if (!isAuthenticated) {
return false;
}

const request = context.switchToHttp().getRequest();
const user = request.user;

if (!user || user.role !== UserRole.ADMIN) {
if (!this.isAdmin(context)) {
throw new AuthorizationException('관리자 권한이 없습니다.');
}

return true;
}

isAdmin(context: ExecutionContext): boolean {
const request = context.switchToHttp().getRequest();
const user = request.user;

return user?.role === UserRole.ADMIN;
}
}
11 changes: 11 additions & 0 deletions backend/src/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,30 @@ import { StorageModule } from './storage/storage.module';
import { ThrottlerGuard, ThrottlerModule } from '@nestjs/throttler';
import { CustomLoggerModule } from './common/log/CustomLoggerModule';
import { SearchModule } from './search/search.module';
import { addTransactionalDataSource } from 'typeorm-transactional';
import { DataSource } from 'typeorm';
import { BatchModule } from './batch/batch.module';
import { ScheduleModule } from '@nestjs/schedule';

@Module({
imports: [
ConfigModule.forRoot({ isGlobal: true }),
CustomLoggerModule,
TypeOrmModule.forRootAsync({
useClass: TypeOrmConfigService,
async dataSourceFactory(options) {
const dataSource = new DataSource(options);
await dataSource.initialize();
return addTransactionalDataSource(dataSource);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q. 테스트에서 datasource 를 오버라이드해서 사용하는데,
이 설정도 따로 적용해야 할까요?

Copy link
Collaborator Author

@koomchang koomchang Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정했습니다!

},
}),
ThrottlerModule.forRoot([
{
ttl: 60000,
limit: 500,
},
]),
ScheduleModule.forRoot(),
AuthModule,
UserModule,
PlaceModule,
Expand All @@ -41,6 +51,7 @@ import { SearchModule } from './search/search.module';
AdminModule,
StorageModule,
SearchModule,
BatchModule,
],
controllers: [AppController],
providers: [
Expand Down
11 changes: 11 additions & 0 deletions backend/src/batch/batch.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { Module } from '@nestjs/common';
import { BatchService } from '@src/batch/batch.service';
import { PinoLogger } from 'nestjs-pino';
import { PlaceRepository } from '@src/place/place.repository';
import { SearchModule } from '@src/search/search.module';

@Module({
imports: [SearchModule],
providers: [BatchService, PlaceRepository, PinoLogger],
})
export class BatchModule {}
31 changes: 31 additions & 0 deletions backend/src/batch/batch.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { Injectable } from '@nestjs/common';
import { PinoLogger } from 'nestjs-pino';
import { Cron, CronExpression } from '@nestjs/schedule';
import { PlaceRepository } from '@src/place/place.repository';
import { SearchService } from '@src/search/search.service';

@Injectable()
export class BatchService {
constructor(
private readonly searchService: SearchService,
private readonly placeRepository: PlaceRepository,
private readonly logger: PinoLogger,
) {}

@Cron(CronExpression.EVERY_HOUR)
async syncPlacesToElastic() {
this.logger.debug(`syncPlacesToElastic() 배치 시작`);
try {
this.logger.debug(`동기화 작업을 시작합니다.`);
const places = await this.placeRepository.findUpdatedPlacesForTwoHours();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q. 수동으로 동기화 한 것 포함해서 모두 동기화하나요?

많지 않아서 괜찮을 것 같습니다~

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

만약 수동으로 잘 저장되는 데이터 수가 아주 많아서,
중복으로 동기화하는 비용이 유의미하게 커지는 지점이 어디쯤일지 계산해보면 좋을 것 같아요.
-> 그렇다면 실패했다는 표시를 어떻게 해서 주기적 동기화에 포함할 것인지?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q. 수동으로 동기화 한 것 포함해서 모두 동기화하나요?

많지 않아서 괜찮을 것 같습니다~

넵! 현재는 그런 상황이고 만약 syncedAt 같은 칼럼을 추가하면 최근에 싱크된 데이터는 빼고 동기화할 수 있을 것 같네요!

만약 수동으로 잘 저장되는 데이터 수가 아주 많아서, 중복으로 동기화하는 비용이 유의미하게 커지는 지점이 어디쯤일지 계산해보면 좋을 것 같아요. -> 그렇다면 실패했다는 표시를 어떻게 해서 주기적 동기화에 포함할 것인지?

맞습니다! 같이 고민해봐야 될 부분인 것 같네요

this.logger.debug(`동기화 할 장소의 갯수: ${places.length}`);

await this.searchService.syncPlaceToElasticSearch(places);
this.logger.debug(`동기화 작업이 완료되었습니다.`);
} catch (error) {
this.logger.error(`동기화 작업 중 에러가 발생했습니다: ${error}`);
} finally {
this.logger.debug(`syncPlacesToElastic() 배치 종료`);
}
}
}
18 changes: 16 additions & 2 deletions backend/src/course/course.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,24 @@ import { CourseRepository } from './course.repository';
import { PlaceModule } from '../place/place.module';
import { TypeOrmModule } from '@nestjs/typeorm';
import { CoursePlace } from '@src/course/entity/course-place.entity';
import { AdminGuard } from '@src/admin/guard/AdminGuard';
import { CoursePermissionGuard } from '@src/course/guards/CoursePermissionGuard';
import { AdminModule } from '@src/admin/admin.module';

@Module({
imports: [UserModule, PlaceModule, TypeOrmModule.forFeature([CoursePlace])],
imports: [
UserModule,
PlaceModule,
AdminModule,
TypeOrmModule.forFeature([CoursePlace]),
],
controllers: [CourseController],
providers: [CourseService, CourseRepository],
providers: [
CourseService,
CourseRepository,
CoursePermissionGuard,
AdminGuard,
],
exports: [CoursePermissionGuard],
})
export class CourseModule {}
16 changes: 12 additions & 4 deletions backend/src/course/guards/CoursePermissionGuard.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,26 @@
import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
import { CourseService } from '../course.service';
import { CoursePermissionException } from '../exception/CoursePermissionException';
import { AdminGuard } from '@src/admin/guard/AdminGuard';

@Injectable()
export class CoursePermissionGuard implements CanActivate {
constructor(private readonly courseService: CourseService) {}
constructor(
private readonly adminGuard: AdminGuard,
private readonly courseService: CourseService,
) {}

async canActivate(context: ExecutionContext): Promise<boolean> {
const isAdmin = this.adminGuard.isAdmin(context);
if (isAdmin) {
return true;
}

const request = context.switchToHttp().getRequest();
const courseId = Number(request.params.id);
const userId = Number(request.user.userId);

const course = await this.courseService.getCourseOwnerId(courseId);
if (course !== userId) {
const courseOwnerId = await this.courseService.getCourseOwnerId(courseId);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p2. 항상 함께 사용될 것 같은데 인증 가드를 상속해 사용하면 어떨까요?
불필요한 컨트롤러 인자와 가드 사용 코드를 줄일 수 있을 것 같아요.

참고 : AdminGuard

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q. 밑에서 다시 사용하나요?
변수로 분리하신 이유가 있나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q. 밑에서 다시 사용하나요? 변수로 분리하신 이유가 있나요?

가독성 고려했습니다!

if (courseOwnerId !== userId) {
throw new CoursePermissionException(courseId);
}
return true;
Expand Down
3 changes: 3 additions & 0 deletions backend/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@
HttpExceptionFilter,
BaseExceptionFilter,
} from './common/exception/filter/GlobalExceptionFilter';
import { initializeTransactionalContext } from 'typeorm-transactional';

process.env.NODE_ENV =
process.env.NODE_ENV && process.env.NODE_ENV.trim().toLowerCase() === 'prod'
? 'prod'
: 'develop';

async function bootstrap() {
initializeTransactionalContext();

const app = await NestFactory.create(AppModule, { bufferLogs: true });
const logger = app.get(Logger);

Expand All @@ -30,12 +33,12 @@

process.on('unhandledRejection', (reason, promise) => {
// 버그 원인 파악시까지 컨테이너 로그에도 남기기 위해 console.error 사용
console.error('Unhandled Rejection at:', { promise, reason });

Check warning on line 36 in backend/src/main.ts

View workflow job for this annotation

GitHub Actions / Lint, Build and Test (backend)

Unexpected console statement
logger.error('Unhandled Rejection at:', { promise, reason });
});

process.on('uncaughtException', (err) => {
console.error('Uncaught Exception:', err);

Check warning on line 41 in backend/src/main.ts

View workflow job for this annotation

GitHub Actions / Lint, Build and Test (backend)

Unexpected console statement
logger.error('Uncaught Exception:', err);
});

Expand Down
9 changes: 6 additions & 3 deletions backend/src/place/place.module.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import { Module } from '@nestjs/common';
import { forwardRef, Module } from '@nestjs/common';
import { PlaceController } from './place.controller';
import { PlaceService } from './place.service';
import { PlaceRepository } from './place.repository';
import { ConfigModule } from '@nestjs/config';
import { SearchModule } from '@src/search/search.module';
import { SearchService } from '@src/search/search.service';
import { PinoLogger } from 'nestjs-pino';

@Module({
imports: [ConfigModule],
imports: [ConfigModule, forwardRef(() => SearchModule)],
controllers: [PlaceController],
providers: [PlaceService, PlaceRepository],
providers: [PlaceService, PlaceRepository, SearchService, PinoLogger],
exports: [PlaceRepository],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q. 이거 테스트 때문에 로거 추가하신건가요?

저도 비슷하게 글로벌 모듈로 이미 등록된 ConfigModule 을 다시 import 했는데요,
혹시 다른 방법 없을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q. 이거 테스트 때문에 로거 추가하신건가요?

저도 비슷하게 글로벌 모듈로 이미 등록된 ConfigModule 을 다시 import 했는데요, 혹시 다른 방법 없을까요?

테스트 때문에 추가했습니다.
저도 비슷한 고민을 했는데 테스트뿐만 아니라, 일반 로직이 들어있는 service들에서도 DIlogger를 받는게 좀 어색한 것 같더라구요. 로거를 싱글톤으로 이용할 수 있으면 좋지 않을까 고민했습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기존의 nest 에서 제공하는 LoggerModule 구조를 따라간건데,
그것도 고민해봐야겠네용

})
export class PlaceModule {}
15 changes: 14 additions & 1 deletion backend/src/place/place.repository.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Injectable, BadRequestException } from '@nestjs/common';
import { DataSource, ILike } from 'typeorm';
import { DataSource, ILike, MoreThan } from 'typeorm';
import { Place } from './entity/place.entity';
import { SoftDeleteRepository } from '../common/SoftDeleteRepository';

Expand Down Expand Up @@ -39,6 +39,19 @@ export class PlaceRepository extends SoftDeleteRepository<Place, number> {
});
}

public async findUpdatedPlacesForTwoHours(): Promise<Place[]> {
const twoHoursAgo = new Date();
twoHoursAgo.setHours(twoHoursAgo.getHours() - 2);

return this.find({
where: [
{ createdAt: MoreThan(twoHoursAgo) },
{ updatedAt: MoreThan(twoHoursAgo) },
{ deletedAt: MoreThan(twoHoursAgo) },
],
});
}

private validatePageAndPageSize(page: number, pageSize: number) {
if (page <= 0) {
throw new BadRequestException('페이지는 1 이상이어야 합니다.');
Expand Down
27 changes: 19 additions & 8 deletions backend/src/place/place.service.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import { Injectable, BadGatewayException } from '@nestjs/common';
import { PlaceRepository } from './place.repository';
import { CreatePlaceRequest } from './dto/CreatePlaceRequest';
import { PlaceNotFoundException } from './exception/PlaceNotFoundException';
import { PlaceAlreadyExistsException } from './exception/PlaceAlreadyExistsException';
import { PlaceSearchResponse } from './dto/PlaceSearchResponse';
import { BadGatewayException, Injectable } from '@nestjs/common';
import { PlaceRepository } from '@src/place/place.repository';
import { CreatePlaceRequest } from '@src/place/dto/CreatePlaceRequest';
import { PlaceNotFoundException } from '@src/place/exception/PlaceNotFoundException';
import { PlaceAlreadyExistsException } from '@src/place/exception/PlaceAlreadyExistsException';
import { PlaceSearchResponse } from '@src/place/dto/PlaceSearchResponse';
import { ConfigService } from '@nestjs/config';
import { Transactional } from 'typeorm-transactional';
import { SearchService } from '@src/search/search.service';
import { PinoLogger } from 'nestjs-pino';

@Injectable()
export class PlaceService {
Expand All @@ -17,10 +20,13 @@ export class PlaceService {
constructor(
private readonly placeRepository: PlaceRepository,
private readonly configService: ConfigService,
private readonly searchService: SearchService,
private readonly logger: PinoLogger,
) {
this.GOOGLE_API_KEY = this.configService.get(<string>'GOOGLE_MAPS_API_KEY');
}

@Transactional()
async addPlace(createPlaceRequest: CreatePlaceRequest) {
const { googlePlaceId } = createPlaceRequest;
if (await this.placeRepository.findByGooglePlaceId(googlePlaceId)) {
Expand All @@ -33,6 +39,11 @@ export class PlaceService {
);

const savedPlace = await this.placeRepository.save(place);
this.searchService.savePlace(place).catch((error) => {
this.logger.error(
`ElasticSearch에 장소 정보를 저장하는 중 오류가 발생했습니다. ${error}`,
);
});
return { id: savedPlace.id };
}

Expand Down Expand Up @@ -85,8 +96,8 @@ export class PlaceService {
name: name,
rating: rating || null,
location: {
longitude: geometry.location.lng,
latitude: geometry.location.lat,
longitude: geometry.location.longitude,
latitude: geometry.location.latitude,
},
formattedAddress: formatted_address || null,
category: types?.[0] || null,
Expand Down
43 changes: 43 additions & 0 deletions backend/src/search/dto/ESPlaceSaveDTO.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { Place } from '@src/place/entity/place.entity';

export class ESPlaceSaveDTO {
constructor(
readonly id: number,
readonly googlePlaceId: string,
readonly name: string,
readonly thumbnailUrl: string,
readonly rating: number,
readonly location: {
latitude: number;
longitude: number;
},
readonly formattedAddress: string,
readonly category: string,
readonly description: string,
readonly detailPageUrl: string,
readonly createdAt: Date,
readonly updatedAt: Date,
readonly deletedAt: Date,
) {}

static from(place: Place) {
return new ESPlaceSaveDTO(
place.id,
place.googlePlaceId,
place.name,
place.thumbnailUrl,
place.rating,
{
latitude: place.latitude,
longitude: place.longitude,
},
place.formattedAddress,
place.category,
place.description,
place.detailPageUrl,
place.createdAt,
place.updatedAt,
place.deletedAt,
);
}
}
15 changes: 15 additions & 0 deletions backend/src/search/exception/ElasticSearchException.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { BaseException } from '@src/common/exception/BaseException';
import { HttpStatus } from '@nestjs/common';

export class ElasticSearchException extends BaseException {
constructor(id?: number) {
const message = id
? `id:${id} ElasticSearch에 데이터를 저장하는데 실패했습니다.`
: 'ElasticSearch에 데이터를 저장하는데 실패했습니다.';
super({
code: 1003,
message,
status: HttpStatus.INTERNAL_SERVER_ERROR,
});
}
}
Loading
Loading