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

BC-8329 - fix meta tag extractor #5350

Merged
merged 35 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
a3f5358
only take max 50.000 characters of response
hoeppner-dataport Nov 1, 2024
e11ac1a
ensure html is never longer than 50.000 characters
hoeppner-dataport Nov 1, 2024
a81181d
accept only https
hoeppner-dataport Nov 1, 2024
fdcfeb2
only consider urls internal that have the correct hostname
hoeppner-dataport Nov 1, 2024
a0dfdb9
fix RegEx for mongoDb-Ids
hoeppner-dataport Nov 1, 2024
b525bb9
Merge branch 'main' into BC-8329-fix-meta-tag-extractor-service
Metauriel Nov 8, 2024
111744c
fix and extend tests
hoeppner-dataport Nov 8, 2024
028c969
small simplification of external url flow
hoeppner-dataport Nov 11, 2024
01fc383
add meta tag x-api-key authentication
hoeppner-dataport Nov 18, 2024
feae779
refactor internals of meta-tag-extractor
hoeppner-dataport Nov 18, 2024
1c8e587
add linkElement property originalImageUrl
hoeppner-dataport Nov 18, 2024
7fa7a96
other changes
hoeppner-dataport Nov 18, 2024
e435601
Merge branch 'main' into BC-8329-fix-meta-tag-extractor
hoeppner-dataport Nov 21, 2024
081006d
use configuration for PUBLIC_BACKEND_URL
hoeppner-dataport Nov 22, 2024
419795b
Merge branch 'BC-8329-fix-meta-tag-extractor' of https://github.com/h…
hoeppner-dataport Nov 22, 2024
27b6f86
Merge branch 'main' of https://github.com/hpi-schul-cloud/schulcloud-…
hoeppner-dataport Nov 22, 2024
d5ca845
chore: fix imports
hoeppner-dataport Nov 22, 2024
0e9daa9
chore: format code
hoeppner-dataport Nov 22, 2024
5230ab7
fix strategy-type-configuration
hoeppner-dataport Nov 22, 2024
019f966
chore: remove strategy type again
hoeppner-dataport Nov 22, 2024
873e0a4
add originalImageUrl to socket communication
hoeppner-dataport Nov 25, 2024
b838cf1
switch to XApiKeyAuthentication
hoeppner-dataport Nov 25, 2024
f49dcec
revert multiple changes
hoeppner-dataport Nov 26, 2024
de709b8
revert several changes
hoeppner-dataport Nov 26, 2024
ca3bbf8
fix tests
hoeppner-dataport Nov 26, 2024
4bae084
complete tests
hoeppner-dataport Nov 26, 2024
c2ded92
fix tests
hoeppner-dataport Nov 26, 2024
6109ed2
Merge branch 'main' of https://github.com/hpi-schul-cloud/schulcloud-…
hoeppner-dataport Nov 26, 2024
e5bfcd0
simplify url validation
hoeppner-dataport Nov 26, 2024
90486cb
fix typo in testfile
hoeppner-dataport Nov 26, 2024
70da3a5
chore: remove unused var
hoeppner-dataport Nov 26, 2024
39270a1
chore: improve coverage
hoeppner-dataport Nov 26, 2024
5cc46a1
add loggable test
hoeppner-dataport Nov 26, 2024
68c08d7
chore: fixes
hoeppner-dataport Nov 26, 2024
4fe90c2
Merge branch 'main' into BC-8329-fix-meta-tag-extractor
hoeppner-dataport Nov 27, 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: 1 addition & 1 deletion apps/server/src/modules/board/board-api.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ import { AuthorizationModule } from '@modules/authorization';
import { forwardRef, Module } from '@nestjs/common';
import { CourseRepo } from '@shared/repo';
import { LoggerModule } from '@src/core/logger';
import { BoardModule } from './board.module';
import {
BoardController,
BoardSubmissionController,
CardController,
ColumnController,
ElementController,
} from './controller';
import { BoardModule } from './board.module';
import { BoardNodePermissionService } from './service';
import { BoardUc, CardUc, ColumnUc, ElementUc, SubmissionItemUc } from './uc';
import { RoomMemberModule } from '../room-member';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe(`content element update content (api)`, () => {
};
};

it('should return status 201', async () => {
it('should return status 200', async () => {
const { loggedInClient, richTextElement } = await setup();

const response = await loggedInClient.patch(`${richTextElement.id}/content`, {
Expand All @@ -100,7 +100,7 @@ describe(`content element update content (api)`, () => {
},
});

expect(response.statusCode).toEqual(201);
expect(response.statusCode).toEqual(200);
});

it('should actually change content of the element', async () => {
Expand Down Expand Up @@ -159,7 +159,7 @@ describe(`content element update content (api)`, () => {
expect(result.alternativeText).toEqual('rich text 1 some more text');
});

it('should return status 201', async () => {
it('should return status 200', async () => {
const { loggedInClient, submissionContainerElement } = await setup();
const response = await loggedInClient.patch(`${submissionContainerElement.id}/content`, {
data: {
Expand All @@ -168,7 +168,7 @@ describe(`content element update content (api)`, () => {
},
});

expect(response.statusCode).toEqual(201);
expect(response.statusCode).toEqual(200);
});

it('should not change dueDate when not proviced in submission container element without dueDate', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ import { ContentElementType } from '../../../domain';
import { TimestampsResponse } from '../timestamps.response';

export class LinkElementContent {
constructor({ url, title, description, imageUrl }: LinkElementContent) {
constructor({ url, title, description, originalImageUrl, imageUrl }: LinkElementContent) {
this.url = url;
this.title = title;
this.description = description;
this.originalImageUrl = originalImageUrl;
this.imageUrl = imageUrl;
}

Expand All @@ -19,6 +20,9 @@ export class LinkElementContent {
@ApiPropertyOptional()
description?: string;

@ApiPropertyOptional()
originalImageUrl?: string;

@ApiPropertyOptional()
imageUrl?: string;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ export class LinkContentBody {
@IsOptional()
@ApiProperty({})
imageUrl?: string;

@IsString()
@IsOptional()
@ApiProperty({})
originalImageUrl?: string;
}

export class LinkElementContentBody extends ElementContentBody {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export class ElementController {
DrawingElementContentBody
)
@ApiResponse({
status: 201,
status: 200,
schema: {
oneOf: [
{ $ref: getSchemaPath(ExternalToolElementResponse) },
Expand All @@ -89,7 +89,7 @@ export class ElementController {
@ApiResponse({ status: 400, type: ApiValidationError })
@ApiResponse({ status: 403, type: ForbiddenException })
@ApiResponse({ status: 404, type: NotFoundException })
@HttpCode(201)
@HttpCode(200)
@Patch(':contentElementId/content')
async updateElement(
@Param() urlParams: ContentElementUrlParams,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export class LinkElementResponseMapper implements BaseResponseMapper {
url: element.url,
title: element.title,
description: element.description,
originalImageUrl: element.originalImageUrl,
imageUrl: element.imageUrl,
}),
});
Expand Down
10 changes: 10 additions & 0 deletions apps/server/src/modules/board/domain/link-element.do.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ describe('LinkElement', () => {
title: 'Example',
description: 'Example description',
imageUrl: 'https://example.com/image.jpg',
originalImageUrl: 'https://example.com/image.jpg',
});
});

Expand Down Expand Up @@ -68,6 +69,15 @@ describe('LinkElement', () => {
expect(linkElement.imageUrl).toBe('https://newurl.com/newimage.jpg');
});

it('should return originalImageUrl', () => {
expect(linkElement.originalImageUrl).toBe('https://example.com/image.jpg');
});

it('should set originalImageUrl', () => {
linkElement.originalImageUrl = 'https://newurl.com/newimage.jpg';
expect(linkElement.originalImageUrl).toBe('https://newurl.com/newimage.jpg');
});

it('should not have child', () => {
expect(linkElement.canHaveChild()).toBe(false);
});
Expand Down
8 changes: 8 additions & 0 deletions apps/server/src/modules/board/domain/link-element.do.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ export class LinkElement extends BoardNode<LinkElementProps> {
this.props.imageUrl = value;
}

get originalImageUrl(): string {
return this.props.originalImageUrl ?? '';
}

set originalImageUrl(value: string) {
this.props.originalImageUrl = value;
}

canHaveChild(): boolean {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export interface LinkElementProps extends BoardNodeProps {
title: string;
url: string;
description?: string;
originalImageUrl?: string;
imageUrl?: string;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ export class BoardNodeEntity extends BaseEntityWithTimestamps implements BoardNo
@Property({ type: 'string', nullable: true })
imageUrl: string | undefined;

@Property({ type: 'string', nullable: true })
originalImageUrl: string | undefined;

// FileElement
// --------------------------------------------------------------------------
@Property({ type: 'string', nullable: true })
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import { createMock, DeepMocked } from '@golevelup/ts-jest';
import { Test, TestingModule } from '@nestjs/testing';
import { InputFormat } from '@shared/domain/types';
import { ContentElementUpdateService } from './content-element-update.service';
import {
DrawingContentBody,
ExternalToolContentBody,
FileContentBody,
LinkContentBody,
RichTextContentBody,
DrawingContentBody,
SubmissionContainerContentBody,
ExternalToolContentBody,
} from '../../controller/dto';
import { BoardNodeRepo } from '../../repo';

import {
drawingElementFactory,
externalToolElementFactory,
Expand All @@ -20,6 +18,7 @@ import {
richTextElementFactory,
submissionContainerElementFactory,
} from '../../testing';
import { ContentElementUpdateService } from './content-element-update.service';

describe('ContentElementUpdateService', () => {
let module: TestingModule;
Expand Down
11 changes: 5 additions & 6 deletions apps/server/src/modules/board/uc/element.uc.spec.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,21 @@
import { createMock, DeepMocked } from '@golevelup/ts-jest';
import { Action } from '@modules/authorization';
import { HttpService } from '@nestjs/axios';
import { Test, TestingModule } from '@nestjs/testing';
import { InputFormat } from '@shared/domain/types';
import { setupEntities, userFactory } from '@shared/testing';
import { Logger } from '@src/core/logger';
import { Action } from '@modules/authorization';
import { ElementUc } from './element.uc';
import { BoardNodePermissionService, BoardNodeAuthorizableService, BoardNodeService } from '../service';
import { RichTextContentBody } from '../controller/dto';
import { BoardNodeFactory } from '../domain';

import { BoardNodeAuthorizableService, BoardNodePermissionService, BoardNodeService } from '../service';
import {
boardNodeAuthorizableFactory,
richTextElementFactory,
drawingElementFactory,
richTextElementFactory,
submissionContainerElementFactory,
submissionItemFactory,
} from '../testing';
import { RichTextContentBody } from '../controller/dto';
import { ElementUc } from './element.uc';

describe(ElementUc.name, () => {
let module: TestingModule;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,21 @@ import { IsString, IsUrl } from 'class-validator';
import { MetaDataEntityType } from '../../types';

export class MetaTagExtractorResponse {
constructor({ url, title, description, imageUrl, type, parentTitle, parentType }: MetaTagExtractorResponse) {
constructor({
url,
title,
description,
originalImageUrl,
imageUrl,
type,
parentTitle,
parentType,
}: MetaTagExtractorResponse) {
this.url = url;
this.title = title;
this.description = description;
this.imageUrl = imageUrl;
this.originalImageUrl = originalImageUrl;
this.type = type;
this.parentTitle = parentTitle;
this.parentType = parentType;
Expand All @@ -26,6 +36,10 @@ export class MetaTagExtractorResponse {
@DecodeHtmlEntities()
description?: string;

@ApiProperty()
@IsString()
originalImageUrl?: string;

@ApiProperty()
@IsString()
imageUrl?: string;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { CurrentUser, ICurrentUser, JwtAuthentication } from '@infra/auth-guard';
import { Body, Controller, InternalServerErrorException, Post, UnauthorizedException } from '@nestjs/common';
import { ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger';
import { CurrentUser, ICurrentUser, JwtAuthentication } from '@src/infra/auth-guard';
import { MetaTagExtractorUc } from '../uc';
import { MetaTagExtractorResponse } from './dto';
import { GetMetaTagDataBody } from './post-link-url.body.params';
Expand All @@ -21,8 +21,7 @@ export class MetaTagExtractorController {
@Body() bodyParams: GetMetaTagDataBody
): Promise<MetaTagExtractorResponse> {
const result = await this.metaTagExtractorUc.getMetaData(currentUser.userId, bodyParams.url);
const imageUrl = result.image?.url;
const response = new MetaTagExtractorResponse({ ...result, imageUrl });
const response = new MetaTagExtractorResponse({ ...result });
return response;
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { MetaData } from '../types';

export interface UrlHandler {
doesUrlMatch(url: string): boolean;
getMetaData(url: string): Promise<MetaData | undefined>;
doesUrlMatch(url: URL): boolean;
getMetaData(url: URL): Promise<MetaData | undefined>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { InvalidLinkUrlLoggableException } from './invalid-link-url.loggable';

describe('InvalidLinkUrlLoggableException', () => {
it('should implement Loggable interface', () => {
const exception = new InvalidLinkUrlLoggableException('http://invalid.url', 'Invalid URL');
expect(typeof exception.getLogMessage).toBe('function');
});

it('should return correct log message', () => {
const url = 'http://invalid.url';
const message = 'Invalid URL';
const exception = new InvalidLinkUrlLoggableException(url, message);

expect(exception.getLogMessage()).toEqual({
type: 'INVALID_LINK_URL',
message,
stack: exception.stack,
data: {
url,
},
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { BadRequestException } from '@nestjs/common';
import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger';

export class InvalidLinkUrlLoggableException extends BadRequestException implements Loggable {
constructor(private readonly url: string, readonly message: string) {
super();
}

getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage {
return {
type: 'INVALID_LINK_URL',
message: this.message,
stack: this.stack,
data: {
url: this.url,
},
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { createConfigModuleOptions } from '@src/config';
import { LoggerModule } from '@src/core/logger';
import metaTagExtractorConfig from './meta-tag-extractor.config';
import { MetaTagExtractorService } from './service';
import { MetaTagExternalUrlService } from './service/meta-tag-external-url.service';
import { MetaTagInternalUrlService } from './service/meta-tag-internal-url.service';
import { BoardUrlHandler, CourseUrlHandler, LessonUrlHandler, TaskUrlHandler } from './service/url-handler';

Expand All @@ -28,6 +29,7 @@ import { BoardUrlHandler, CourseUrlHandler, LessonUrlHandler, TaskUrlHandler } f
],
providers: [
MetaTagExtractorService,
MetaTagExternalUrlService,
MetaTagInternalUrlService,
TaskUrlHandler,
LessonUrlHandler,
Expand Down
Loading
Loading