[Proposal] (Server) Decouble DB from routes (from domain logic)

[jazcarate]

trafficstars

I actually wanted to propose a slightly different change, and decouple the controller with the database. Right now tests are very slow because they spool a whole DB, and any new changes will be very sequilize bound. Maybe it's because I don't really like sequalize, but I would like to have the domain logic (e.g.: hashing the password, or failing if there are no file) to live outside the class Post extends Model and leave the current Post class as a mere DTO to the database

Current implementation

Lets explore how one route would change. /posts/create is a big ofender:

// server/src/routes/posts.ts
posts.post(
	"/create",
	jwt,
	celebrate({                                 // ---- All good. This is a route responsability 🆗 
		body: {
			title: Joi.string().required(),
			files: Joi.any().required(),
			visibility: Joi.string()
				.custom(postVisibilitySchema, "valid visibility")
				.required(),
			userId: Joi.string().required(),
			password: Joi.string().optional(),
			//  expiresAt, allow to be null
			expiresAt: Joi.date().optional().allow(null, ""),
			parentId: Joi.string().optional().allow(null, "")
		}
	}),
	async (req, res, next) => {
		try {
			// check if all files have titles
			const files = req.body.files as File[]
			const fileTitles = files.map((file) => file.title)
			const missingTitles = fileTitles.filter((title) => title === "")
			if (missingTitles.length > 0) {    // ---- This is an invariant on all post->files. There should never be a post without titles 🙅 
				throw new Error("All files must have a title")
			}

			if (files.length === 0) {    // ---- This again is an invariant on all post->files. 🙅 
				throw new Error("You must submit at least one file")
			}

			let hashedPassword: string = ""
			if (req.body.visibility === "protected") {    // ---- This again is transformation of the Post creation; not the router🙅 
				hashedPassword = crypto
					.createHash("sha256")
					.update(req.body.password)
					.digest("hex")
			}

			const newPost = new Post({
				title: req.body.title,
				visibility: req.body.visibility,
				password: hashedPassword,
				expiresAt: req.body.expiresAt
			})

			await newPost.save()  // ---- Now we are crosing a lot of boundries, directly accesing the DB🙅 
			await newPost.$add("users", req.body.userId)
			const newFiles = await Promise.all(
				files.map(async (file) => {
					const html = getHtmlFromFile(file)
					const newFile = new File({
						title: file.title || "",
						content: file.content,
						sha: crypto
							.createHash("sha256")
							.update(file.content)
							.digest("hex")
							.toString(),
						html: html || "",
						userId: req.body.userId,
						postId: newPost.id
					})
					await newFile.save()
					return newFile
				})
			)

			await Promise.all(
				newFiles.map(async (file) => {
					await newPost.$add("files", file.id)  // ---- We have to manage timings on how sequalize like things to be stored🙅 
					await newPost.save()
				})
			)
			if (req.body.parentId) {
				// const parentPost = await Post.findOne({
				// 	where: { id: req.body.parentId }
				// })
				// if (parentPost) {
				// 	await parentPost.$add("children", newPost.id)
				// 	await parentPost.save()
				// }
				const parentPost = await Post.findByPk(req.body.parentId)
				if (parentPost) {
					newPost.$set("parent", req.body.parentId)
					await newPost.save()
				} else {
					throw new Error("Parent post not found")
				}
			}

			res.json(newPost)
		} catch (e) {
			res.status(400).json(e)
		}
	}
)

Proposed implementation

// server/src/routes/posts.ts
function render({ title, files, visibility, userId, password: hashedPassword, expiresAt }): PostResponse { 
	return { title, files, visibility, userId, password: hashedPassword, expiresAt }
}

posts.post(
	"/create",
	jwt,
	celebrate(...),
	hashPassword,
	async (req, res, next) => {
		try {
                        const { title, files, visibility, userId, hashedPassword, expiresAt, parentId } = req.body

                        const newPost = await postService.create({ title, files, visibility, userId, password: hashedPassword, expiresAt, parentId })
			res.json(render(newPost))
		} catch (e) {
			res.status(400).json(e)
		}
	}
)

We can then discuss how to implement the dependency injection mechanism of the postService.

[MaxLeiter]

Thanks for the thorough issue, this is a great idea. I just overlooked it while hacking together the initial version /:

Do you want to start an initial conversion starting in #76? Or maybe we should do it before more work is done there?

P.S. If you want to stay connected in IRC I can give you an account on my thelounge.chat instance. Send me an email at [email protected] if that's something you'd like.