Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
71 changes: 71 additions & 0 deletions src/controllers/message.controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { EventEmitter } from 'events';
import { messageService } from '../services/message.service.js';
import { User } from '../models/user.model.js';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Import path is wrong. The code imports User from ../models/user.model.js, but the actual model file is src/models/user.js (exported as User). This will throw a module-not-found error at runtime; change to ../models/user.js or the correct relative path.


export const messageEmitter = new EventEmitter();

const create = async (req, res) => {
const { roomId } = req.params;
const { text, userId } = req.body;

if (!roomId || !userId || !text || typeof text !== 'string') {
return res.status(400).json({ message: 'Missing or invalid parameters' });
}

try {
const newMessage = await messageService.createMessageInRoom(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Be careful: messageService.createMessageInRoom (service) returns a mapped DTO (see message.service.js) with fields like author, time, text — not a Sequelize instance. The code below treats newMessage as an instance (newMessage.createdAt, newMessage.text) which will be undefined. Either make the service return the instance or use the DTO fields the service provides.

text,
userId,
roomId,
);

const user = await User.findByPk(userId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You're fetching the user from DB after creating the message. If you need to verify the user exists, check before creating the message to avoid creating messages for non-existent users. Alternatively, rely on the service to include the author's name so you don't need an additional DB lookup.


if (!user) {
return res.status(404).json({ message: 'User not found' });
}

const formattedMessage = {
author: user.name,
time: newMessage.createdAt,
text: newMessage.text,
};
Comment on lines +28 to +32

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The formattedMessage does not include roomId. The WebSocket broadcaster in index.js expects message.roomId to route messages to the right room — omitting it prevents broadcasting to the correct room. Also, time: newMessage.createdAt and text: newMessage.text may be undefined due to the service/controller shape mismatch (see earlier comment).


messageEmitter.emit('message', formattedMessage);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When emitting the message, ensure you emit a canonical DTO like { author, time, text, roomId } so the emitter and WebSocket layer can broadcast correctly. Right now messageEmitter.emit('message', formattedMessage) emits an object missing roomId and with possibly-missing fields.


res.status(201).json(formattedMessage);
} catch (err) {
// eslint-disable-next-line no-console
console.error('Error creating message:', err);
res.status(500).json({ message: 'Internal server error' });
}
};

const getMessages = async (req, res) => {
const { roomId } = req.params;

if (!roomId) {
return res.status(400).json({ message: 'Missing roomId' });
}

try {
const messages = await messageService.getAllMessagesInRoom(roomId);

const formattedMessages = messages.map((m) => ({
author: m.User?.name || 'Unknown',
time: m.createdAt,
text: m.text,
}));
Comment on lines +54 to +58

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

messageService.getAllMessagesInRoom already returns messages mapped to { author, time, text, roomId }. This mapping here uses m.User?.name and m.createdAt, which don't exist on the mapped objects — resulting in author and time being undefined. Either return raw DB objects from the service and map here, or (simpler) send the service's DTOs directly (res.json(messages)) or map using m.author, m.time, m.text.


res.status(200).json(formattedMessages);
} catch (err) {
// eslint-disable-next-line no-console
console.error('Error getting messages:', err);
res.status(500).json({ message: 'Internal server error' });
}
};

export const messageController = {
create,
getMessages,
};
51 changes: 51 additions & 0 deletions src/controllers/rooms.controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { roomsService } from '../services/rooms.service.js';

const getAllRooms = async (req, res) => {
const rooms = await roomsService.getAllRooms();

res.status(200).send(rooms);
};

const createRoom = async (req, res) => {
const { title, userId, description } = req.body;

if (!title || !userId) {
return res.sendStatus(404);
Comment on lines +12 to +13

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Returning 404 for missing title or userId is misleading. Use 400 (Bad Request) for missing/invalid input and include a JSON error message to help clients (this endpoint is responsible for creating rooms per task requirements).

}

await roomsService.createRoom(title, userId, description);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Calls to the service (create/update/delete) are not wrapped in try/catch. If the service throws (for example, room not found or DB error), the error will bubble and result in an uncontrolled 500. Wrap these awaits in try/catch and return meaningful statuses (e.g., 201/204 on success, 404 if service reports not found, 500 for unexpected errors).


res.sendStatus(201);
};

const updateRoom = async (req, res) => {
const { roomId } = req.params;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

roomId is taken from params but not validated. If your Room model uses UUIDs (as in the repo), validate that roomId is a valid UUID string before calling the service to avoid type/lookup issues and improve error messages.

const { title, description } = req.body;

if ((!title && !description) || !roomId) {
return res.sendStatus(404);
Comment on lines +25 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This validation returns 404 when no title/description or roomId is provided. Use 400 for missing/invalid request data. 404 should be returned only when the target resource is not found after attempting the update. Also separate the checks so you can return clearer errors (e.g., missing params vs not found).

}

await roomsService.updateRoom(roomId, title, description);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same as above: await roomsService.updateRoom(...) can throw (service validates id/existence). Add try/catch around this call and translate service errors into appropriate HTTP responses so clients get clear errors.


res.sendStatus(204);
};

const deleteRoom = async (req, res) => {
const { roomId } = req.params;

if (!roomId) {
return res.sendStatus(400);
}

await roomsService.deleteRoom(roomId);

res.sendStatus(204);
};

export const roomController = {
getAllRooms,
createRoom,
updateRoom,
deleteRoom,
};
28 changes: 28 additions & 0 deletions src/controllers/user.controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { User } from '../models/user.js';
import { userService } from '../services/user.service.js';

const create = async (req, res) => {
try {
const { name } = req.body;

if (!name) {
return res.status(400).json({ message: 'Name is required' });
}

const isNameExist = await User.findOne({ where: { name } });

if (isNameExist) {
return res.status(409).json({ message: 'User already exists' });
}

await userService.createUser(name);

return res.status(201).json({ message: 'User was created!' });
} catch {
return res.status(500).json({ message: 'Internal server error' });
}
};

export const userController = {
create,
};
10 changes: 10 additions & 0 deletions src/db.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import 'dotenv/config';
import { Sequelize } from 'sequelize';

export const client = new Sequelize({
host: process.env.DB_HOST,
username: process.env.DB_USERNAME,
password: process.env.DB_PASS,
database: process.env.DB_DATABASE,
dialect: 'postgres',
});
82 changes: 82 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -1 +1,83 @@
'use strict';
import 'dotenv/config';
import express from 'express';
import { WebSocketServer } from 'ws';
import cors from 'cors';
import { userRoute } from './routes/user.route.js';
import { roomsRoute } from './routes/rooms.route.js';
import { messageRoute } from './routes/message.route.js';
import { messageEmitter } from './controllers/message.controller.js';
import { messageService } from './services/message.service.js';

const PORT = process.env.PORT || 5000;
const app = express();

app.use(cors());
app.use(express.json());

app.use('/user', userRoute);
app.use('/rooms', roomsRoute);
app.use(messageRoute);

const server = app.listen(PORT);

const wss = new WebSocketServer({ server });
const rooms = {};

wss.on('connection', (ws) => {
ws.on('message', async (rawMessage) => {
let parsed;

try {
parsed = JSON.parse(rawMessage);
} catch {
return;
}

const { roomId } = parsed;

if (!roomId || typeof roomId !== 'number') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This rejects non-numeric room IDs. The Room model uses UUID strings, so this check prevents valid room IDs (UUIDs) from being accepted. Remove the numeric-type requirement or accept string UUIDs instead so clients can join rooms correctly.

return;
}

if (!rooms[roomId]) {
rooms[roomId] = [];
}

if (!rooms[roomId].includes(ws)) {
rooms[roomId].push(ws);
}

const messages = await messageService.getAllMessagesInRoom(roomId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When sending previous messages to the newly-joined socket, ensure messageService.getAllMessagesInRoom(roomId) is called with the correct roomId type (string UUID) and handle potential errors (wrap in try/catch) so a thrown error doesn't silently break the connection.


messages.forEach((msg) => {
ws.send(JSON.stringify(msg));
});
});

ws.on('close', () => {
for (const roomId in rooms) {
rooms[roomId] = rooms[roomId].filter((client) => client !== ws);
}
});
});

messageEmitter.on('message', async (message) => {
const { roomId } = message;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You destructure roomId from the emitted message. Ensure the emitter always receives a roomId (e.g., emit { author, time, text, roomId } from the message creation flow), otherwise this check will be falsy and the message will be dropped instead of broadcast to the room.


if (!roomId || !rooms[roomId]) {
return;
}

const formatted = {
author: message.author || message.User?.name || 'Unknown',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The formatted message probes message.User?.name and message.createdAt. This is inconsistent with the service/controller mapping which returns author and time. Standardize on a single DTO (for example message.author and message.time) and use those fields here to avoid undefined author/time values in broadcasts.

time: message.time || message.createdAt,
text: message.text,
};

rooms[roomId].forEach((client) => {
if (client.readyState === 1) {
client.send(JSON.stringify(formatted));
}
});
});
35 changes: 35 additions & 0 deletions src/models/message.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { DataTypes } from 'sequelize';
import { client } from '../db.js';
import { User } from './user.js';
import { Room } from './room.js';

export const Message = client.define(
'message',
{
id: {
type: DataTypes.UUID,
defaultValue: DataTypes.UUIDV4,
allowNull: false,
primaryKey: true,
},
text: {
type: DataTypes.STRING,
allowNull: false,
},
createdAt: {
type: DataTypes.DATE,
defaultValue: DataTypes.NOW,
allowNull: false,
},
},
{
updatedAt: false,
tableName: 'messages',
},
);

Message.belongsTo(User, { foreignKey: 'userId' });
User.hasMany(Message, { foreignKey: 'userId' });

Message.belongsTo(Room, { foreignKey: 'roomId' });
Room.hasMany(Message, { foreignKey: 'roomId' });
23 changes: 23 additions & 0 deletions src/models/room.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { DataTypes } from 'sequelize';
import { client } from '../db.js';
import { User } from './user.js';

export const Room = client.define('room', {
id: {
type: DataTypes.UUID,
defaultValue: DataTypes.UUIDV4,
allowNull: false,
primaryKey: true,
},
title: {
type: DataTypes.STRING,
allowNull: false,
},
description: {
type: DataTypes.STRING,
allowNull: false,
Comment on lines +16 to +18

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider adding a defaultValue: '' (or allowNull: true) to the description field so the database won't reject inserts created without a description. The service currently supplies description = '', but making the DB column have a default makes the model more robust if other code creates rooms directly.

},
});

Room.belongsTo(User);
User.hasMany(Room);
Comment on lines +22 to +23

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Be explicit about the foreign key to ensure consistent column naming (userId) across associations and queries — for example Room.belongsTo(User, { foreignKey: 'userId' }) and User.hasMany(Room, { foreignKey: 'userId' }). This removes ambiguity and aligns the model with other code that references userId.

16 changes: 16 additions & 0 deletions src/models/user.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { DataTypes } from 'sequelize';
import { client } from '../db.js';

export const User = client.define('user', {
id: {
type: DataTypes.UUID,
defaultValue: DataTypes.UUIDV4,
allowNull: false,
primaryKey: true,
},
name: {
type: DataTypes.STRING,
unique: true,
allowNull: false,
},
});
7 changes: 7 additions & 0 deletions src/routes/message.route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import express from 'express';
import { messageController } from '../controllers/message.controller.js';

export const messageRoute = new express.Router();

messageRoute.get('/rooms/:roomId/messages', messageController.getMessages);
messageRoute.post('/rooms/:roomId/messages', messageController.create);
9 changes: 9 additions & 0 deletions src/routes/rooms.route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import express from 'express';
import { roomController } from '../controllers/rooms.controller.js';

export const roomsRoute = new express.Router();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

express.Router is a factory function, not a constructor. Using new express.Router() will throw at runtime. Change this to export const roomsRoute = express.Router(); so the router is created correctly.


roomsRoute.get('/', roomController.getAllRooms);
roomsRoute.post('/', roomController.createRoom);
roomsRoute.patch('/:roomId', roomController.updateRoom);
roomsRoute.delete('/:roomId', roomController.deleteRoom);
6 changes: 6 additions & 0 deletions src/routes/user.route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import express from 'express';
import { userController } from '../controllers/user.controller.js';

export const userRoute = new express.Router();

userRoute.post('/', userController.create);
45 changes: 45 additions & 0 deletions src/services/message.service.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { Message } from '../models/message.js';
import { User } from '../models/user.js';

const getAllMessagesInRoom = async (roomId) => {
if (!roomId) {
throw new Error('roomId is required');
}

const messages = await Message.findAll({
where: { roomId },
include: [{ model: User, attributes: ['name'] }],
order: [['createdAt', 'ASC']],
});

return messages.map((msg) => ({
author: msg.user?.name || 'Unknown',
Comment on lines +15 to +16

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When mapping messages you access the included user as msg.user?.name. With Sequelize the included model is usually attached as msg.User (capitalized) when the model is named User. This likely results in author being 'Unknown'. Use msg.User?.name (or confirm the include alias) and keep the shape consistent with other layers.

time: msg.createdAt,
text: msg.text,
roomId: msg.roomId,
}));
};

const createMessageInRoom = async (text, userId, roomId) => {
if (!text || !userId || !roomId) {
throw new Error('Missing required fields');
}

const newMessage = await Message.create({ text, userId, roomId });

const messageWithUser = await Message.findByPk(newMessage.id, {
include: [{ model: User, attributes: ['name'] }],
});

return {
author: messageWithUser.user?.name || 'Unknown',
Comment on lines +34 to +35

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

After creating a message you fetch messageWithUser and access messageWithUser.user?.name. For the same reason as above this should likely be messageWithUser.User?.name (or use the actual alias from your model associations). Ensure the service returns a canonical DTO (e.g. { author, time, text, roomId }) that other modules consume without remapping.

time: messageWithUser.createdAt,
text: messageWithUser.text,
roomId: messageWithUser.roomId,
};
};

export const messageService = {
getAllMessagesInRoom,
createMessageInRoom,
};
Loading