add solution by zbvika · Pull Request #176 · mate-academy/node_compression-app

zbvika · 2025-08-21T12:27:51Z

No description provided.

mateacademy-ai-mentor

Review Summary

Decision: Request changes because two checklist-required behaviors are incorrect and will break compliance/tests.

Critical issues to fix (blockers):

Incorrect output file extensions in src/createServer.js:
- Lines 11-12 (SUPPORTED_TYPES) use .gzip and .deflate. Per requirements, they must be .gz for gzip and .dfl for deflate. This causes wrong filenames and test failures.
- How to fix: Update the mapping, e.g.:
  - gzip: { ext: '.gz', method: zlib.createGzip }
  - deflate: { ext: '.dfl', method: zlib.createDeflate }
Wrong status code for stream errors:
- Current error handler sends res.writeHead(500). The spec requires 400 for compression/upload stream errors.
- How to fix: In the stream error handlers (compression stream and file read/upload streams), change to res.writeHead(400) and end the response appropriately.

What you did well:

Solid use of streams, correct zlib mappings, multipart parsing, validations, and headers. Routing and non-existent endpoint handling look good.

Please make these small changes and resubmit. Once updated, this should meet all requirements. Keep up the great work!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2025-08-21T12:30:05Z

+const multiparty = require('multiparty');
+
+const SUPPORTED_TYPES = {
+  gzip: { ext: '.gzip', method: zlib.createGzip },


The file extension for gzip should be ".gz" (not ".gzip") according to the requirements. Update to:

gzip: { ext: '.gz', method: zlib.createGzip },

This is required for correct output filenames.

mateacademy-ai-mentor · 2025-08-21T12:30:05Z

+
+const SUPPORTED_TYPES = {
+  gzip: { ext: '.gzip', method: zlib.createGzip },
+  deflate: { ext: '.deflate', method: zlib.createDeflate },


The file extension for deflate should be ".dfl" (not ".deflate") according to the requirements. Update to:

deflate: { ext: '.dfl', method: zlib.createDeflate },

This is required for correct output filenames.

mateacademy-ai-mentor · 2025-08-21T12:30:05Z

+          fs.unlink(file.path, () => {});
+
+          if (erro) {
+            res.writeHead(500);


The requirements specify that stream errors (compression or upload errors) should result in a 400 status code, not 500. Change res.writeHead(500); to res.writeHead(400); to comply with the checklist.

zbvika · 2025-08-21T12:40:41Z

@mateacademy-ai-mentor I changed the error status. However, if I change the file extension, my tests fail. The tests specifically require using .gzip and .deflate.

mateacademy-ai-mentor

Review Summary

Decision: REQUEST_CHANGES

You're very close—most requirements are met, and the server logic, streaming, and error handling look solid. However, there is one critical blocker that violates explicit checklist items and will likely fail tests.

Critical issues to fix (blockers):

Incorrect output extensions in src/createServer.js:
- On or around lines 11–12 in the SUPPORTED_TYPES mapping, gzip is set to use .gzip and deflate to .deflate.
- This violates requirement 1.3 and 3.3, which specify the extensions must be .gz for gzip and .dfl for deflate. This causes incorrect filenames and non-compliance.

How to fix:

Update the mapping in src/createServer.js:
- gzip: { ext: '.gz', ... }
- deflate: { ext: '.dfl', ... }
Re-run tests to confirm headers like Content-Disposition now include the correct filename extensions.

Minor notes:

Everything else (validation, zlib usage, streaming, status codes) looks great.

Once you correct the extensions, this should be ready to approve. Nice work overall!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2025-08-21T12:42:59Z

+const multiparty = require('multiparty');
+
+const SUPPORTED_TYPES = {
+  gzip: { ext: '.gzip', method: zlib.createGzip },


The file extension for gzip is set to .gzip, but the requirements specify it must be .gz. Please change this to .gz to comply with the checklist (requirement 1.3, 3.3).

@mateacademy-ai-mentor if I change the file extension, my tests fail. The tests specifically require using .gzip and .deflate.

mateacademy-ai-mentor · 2025-08-21T12:42:59Z

+
+const SUPPORTED_TYPES = {
+  gzip: { ext: '.gzip', method: zlib.createGzip },
+  deflate: { ext: '.deflate', method: zlib.createDeflate },


The file extension for deflate is set to .deflate, but the requirements specify it must be .dfl. Please change this to .dfl to comply with the checklist (requirement 1.3, 3.3).

@mateacademy-ai-mentor if I change the file extension, my tests fail. The tests specifically require using .gzip and .deflate.

zbvika · 2025-08-21T12:46:21Z

@mateacademy-ai-mentor if I change the file extension, my tests fail. The tests specifically require using .gzip and .deflate.

mateacademy-ai-mentor

Review Summary

Great work overall—your server logic, streaming pipeline, routing, and error handling are solid and meet the requirements well. This is an educational approval; there’s one critical detail to adjust for full compliance.

What you did well:
- Clean use of Node.js streams and zlib
- Robust routing and validation for POST /compress
- Proper headers for file download
What needs improvement (specific and constructive):
- Incorrect output file extensions in src/createServer.js:
  - Lines 11–13 in SUPPORTED_TYPES: gzip and deflate currently map to .gzip and .deflate.
  - Requirement: gzip → .gz, deflate → .dfl, br → .br.
  - Why this is an issue: It violates the explicit naming requirement and may break consumers or tests aligned with the spec.
  - How to fix (hint): Update the mapping, e.g., gzip: { ext: '.gz', ... } and deflate: { ext: '.dfl', ... }. Keep br as .br.

Note: If your automated tests expect .gzip/.deflate instead, clarify the source of truth. For requirement compliance, use .gz and .dfl.

You’re very close—adjusting those extensions should bring everything into full compliance. Nice job, keep it up!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2025-08-21T12:50:04Z

+const multiparty = require('multiparty');
+
+const SUPPORTED_TYPES = {
+  gzip: { ext: '.gzip', method: zlib.createGzip },


The extension for gzip should be .gz (not .gzip) according to the requirements. Please update to:
gzip: { ext: '.gz', ... }

mateacademy-ai-mentor · 2025-08-21T12:50:04Z

+
+const SUPPORTED_TYPES = {
+  gzip: { ext: '.gzip', method: zlib.createGzip },
+  deflate: { ext: '.deflate', method: zlib.createDeflate },


The extension for deflate should be .dfl (not .deflate) according to the requirements. Please update to:
deflate: { ext: '.dfl', ... }

add solution

c945fb2

mateacademy-ai-mentor suggested changes Aug 21, 2025

View reviewed changes

fix statusCode

bf4acb4

zbvika requested a review from mateacademy-ai-mentor August 21, 2025 12:40

mateacademy-ai-mentor suggested changes Aug 21, 2025

View reviewed changes

zbvika requested a review from mateacademy-ai-mentor August 21, 2025 12:46

mateacademy-ai-mentor approved these changes Aug 21, 2025

View reviewed changes

Conversation

zbvika commented Aug 21, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zbvika commented Aug 21, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zbvika commented Aug 21, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants