feat: Upgrade to @google-cloud/storage 7.19.0#57
feat: Upgrade to @google-cloud/storage 7.19.0#57mtrezza merged 3 commits intoparse-community:mainfrom
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #57 +/- ##
==========================================
+ Coverage 49.25% 58.20% +8.95%
==========================================
Files 1 1
Lines 67 67
==========================================
+ Hits 33 39 +6
+ Misses 34 28 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
mtrezza
changed the title
|
@coderabbitai does this dependency upgrade cause any breaking changes? Does it require any code changes in the storage adapter? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🌐 Web query: 💡 Result:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 8 🌐 Web query: 💡 Result:
|
|
This is quite a risky repo lacking integration tests. I'd strongly recommend to clean this up in the future (in separate PRs): |
|
@coderabbitai suggest an integration test that can be added regarding GCSAdapter.prototype.deleteFile |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 797 🏁 Script executed: Length of output: 2509
// Add to spec/test.spec.js
describe('GCSAdapter.deleteFile integration tests', () => {
// Only run integration tests if GCS credentials are provided
if (process.env.GCP_PROJECT_ID && process.env.GCP_KEYFILE_PATH && process.env.GCS_BUCKET) {
let gcsAdapter;
const testFileName = 'test-delete-file.txt';
const testFileContent = Buffer.from('test content for deletion');
beforeEach(() => {
gcsAdapter = new GCSAdapter({
projectId: process.env.GCP_PROJECT_ID,
keyFilename: process.env.GCP_KEYFILE_PATH,
bucket: process.env.GCS_BUCKET,
bucketPrefix: 'test-delete-'
});
});
afterEach(async () => {
// Cleanup: ensure test file is removed even if test fails
try {
await gcsAdapter.deleteFile(testFileName);
} catch (e) {
// Ignore errors during cleanup
}
});
it('should successfully delete an existing file', async () => {
// Arrange: Create a file first
await gcsAdapter.createFile(testFileName, testFileContent);
// Verify file exists before deletion
const fileExistsBefore = await gcsAdapter.getFileData(testFileName);
expect(fileExistsBefore).toBeDefined();
// Act: Delete the file
const deleteResult = await gcsAdapter.deleteFile(testFileName);
// Assert: Verify deletion
expect(deleteResult).toBeDefined(); // v7 returns raw response
// Verify file no longer exists
try {
await gcsAdapter.getFileData(testFileName);
fail('Expected getFileData to throw after file deletion');
} catch (error) {
expect(error.code).toBe(404); // GCS returns 404 for missing files
}
});
it('should handle deleting a non-existent file gracefully', async () => {
// Act & Assert: Try to delete a file that doesn't exist
const nonExistentFileName = 'non-existent-file-' + Date.now() + '.txt';
try {
await gcsAdapter.deleteFile(nonExistentFileName);
// GCS delete is idempotent - deleting non-existent file may succeed
// This is expected behavior
} catch (error) {
// If it throws, verify it's a reasonable error
expect(error.code).toBeDefined();
expect([404, 'ENOENT']).toContain(error.code);
}
});
it('should delete file with special characters in name', async () => {
// Arrange: File with special characters
const specialFileName = 'test file with spaces & special-chars_123.txt';
await gcsAdapter.createFile(specialFileName, testFileContent);
// Act: Delete the file
const deleteResult = await gcsAdapter.deleteFile(specialFileName);
// Assert: Verify deletion
expect(deleteResult).toBeDefined();
try {
await gcsAdapter.getFileData(specialFileName);
fail('Expected file to be deleted');
} catch (error) {
expect(error.code).toBe(404);
}
});
it('should respect bucketPrefix when deleting files', async () => {
// This ensures files are deleted from the correct path
// Arrange: Create file (will be created with prefix)
await gcsAdapter.createFile(testFileName, testFileContent);
// Act: Delete should use the same prefix
await gcsAdapter.deleteFile(testFileName);
// Assert: File should be gone
try {
await gcsAdapter.getFileData(testFileName);
fail('Expected file to be deleted with correct prefix');
} catch (error) {
expect(error.code).toBe(404);
}
});
} else {
it('skipping integration tests - GCS credentials not provided', () => {
console.log('Set GCP_PROJECT_ID, GCP_KEYFILE_PATH, and GCS_BUCKET to run integration tests');
expect(true).toBe(true);
});
}
});Key features of this integration test:
To run these tests: export GCP_PROJECT_ID="your-project-id"
export GCP_KEYFILE_PATH="/path/to/keyfile.json"
export GCS_BUCKET="your-test-bucket"
npm testAlternative: Using GCS Emulator for CI For automated testing without real GCS credentials, you could also use the Google Cloud Storage Emulator in CI pipelines, which would make these tests run faster and without costs. Would you like me to also suggest a GitHub Actions workflow that sets up the GCS emulator for automated integration testing? |
|
@coderabbitai provide a test regarding GCSAdapter.prototype.deleteFile that doesn't requires GCS credentials or parse-server, but ensures the interface contract is honored. |
|
Tip For best results, initiate chat on the files or code changes.
// Add to spec/test.spec.js
describe('GCSAdapter.deleteFile unit tests (mocked)', () => {
let gcsAdapter;
let mockStorage;
let mockBucket;
let mockFile;
let mockDeleteFn;
beforeEach(() => {
// Create mock chain: Storage -> bucket() -> file() -> delete()
mockDeleteFn = jasmine.createSpy('delete');
mockFile = {
delete: mockDeleteFn
};
mockBucket = jasmine.createSpyObj('bucket', ['file']);
mockBucket.file.and.returnValue(mockFile);
mockStorage = jasmine.createSpyObj('Storage', ['bucket']);
mockStorage.bucket.and.returnValue(mockBucket);
// Create adapter with mocked storage client
gcsAdapter = new GCSAdapter({
projectId: 'test-project',
keyFilename: 'test-key.json',
bucket: 'test-bucket',
bucketPrefix: 'test-prefix/'
});
// Replace the internal GCS client with our mock
gcsAdapter._gcsClient = mockStorage;
});
it('should call GCS delete with correct bucket and file path', (done) => {
const testFileName = 'myfile.txt';
// Mock successful deletion
mockDeleteFn.and.callFake((callback) => {
callback(null, { statusCode: 204 });
});
gcsAdapter.deleteFile(testFileName)
.then((result) => {
// Verify bucket() was called with correct bucket name
expect(mockStorage.bucket).toHaveBeenCalledWith('test-bucket');
// Verify file() was called with bucketPrefix + filename
expect(mockBucket.file).toHaveBeenCalledWith('test-prefix/myfile.txt');
// Verify delete() was called
expect(mockDeleteFn).toHaveBeenCalled();
// Verify result is returned (interface contract: deleteFile returns a promise that resolves with response)
expect(result).toBeDefined();
expect(result.statusCode).toBe(204);
done();
})
.catch((err) => {
fail('Promise should not reject: ' + err);
done();
});
});
it('should reject promise when GCS delete fails', (done) => {
const testFileName = 'error-file.txt';
const mockError = new Error('GCS delete failed');
mockError.code = 403;
// Mock failed deletion
mockDeleteFn.and.callFake((callback) => {
callback(mockError, null);
});
gcsAdapter.deleteFile(testFileName)
.then(() => {
fail('Promise should have rejected');
done();
})
.catch((err) => {
// Verify the error is properly propagated
expect(err).toBe(mockError);
expect(err.message).toBe('GCS delete failed');
expect(err.code).toBe(403);
// Verify methods were still called
expect(mockStorage.bucket).toHaveBeenCalledWith('test-bucket');
expect(mockBucket.file).toHaveBeenCalledWith('test-prefix/error-file.txt');
expect(mockDeleteFn).toHaveBeenCalled();
done();
});
});
it('should handle empty bucketPrefix correctly', (done) => {
// Create adapter without bucketPrefix
const adapterNoPrefixgcsAdapter = new GCSAdapter({
projectId: 'test-project',
keyFilename: 'test-key.json',
bucket: 'test-bucket'
// No bucketPrefix
});
adapterNoPrefixgcsAdapter._gcsClient = mockStorage;
const testFileName = 'no-prefix-file.txt';
mockDeleteFn.and.callFake((callback) => {
callback(null, { statusCode: 204 });
});
adapterNoPrefixgcsAdapter.deleteFile(testFileName)
.then(() => {
// Verify file() was called with just filename (no prefix)
expect(mockBucket.file).toHaveBeenCalledWith('no-prefix-file.txt');
done();
})
.catch((err) => {
fail('Promise should not reject: ' + err);
done();
});
});
it('should return a Promise (interface contract)', () => {
mockDeleteFn.and.callFake((callback) => {
callback(null, {});
});
const result = gcsAdapter.deleteFile('test.txt');
// Verify deleteFile returns a Promise
expect(result).toEqual(jasmine.any(Promise));
expect(typeof result.then).toBe('function');
expect(typeof result.catch).toBe('function');
});
it('should handle special characters in filename', (done) => {
const specialFileName = 'file with spaces & special-chars_123.txt';
mockDeleteFn.and.callFake((callback) => {
callback(null, {});
});
gcsAdapter.deleteFile(specialFileName)
.then(() => {
// Verify the filename is passed as-is (not encoded or modified)
expect(mockBucket.file).toHaveBeenCalledWith('test-prefix/file with spaces & special-chars_123.txt');
done();
})
.catch((err) => {
fail('Promise should not reject: ' + err);
done();
});
});
});Key features of this unit test:
This test ensures the adapter correctly implements its contract without requiring real GCS credentials or making actual API calls. |
|
@Moumouls could you review and add the suggested tests? the upgrade may include breaking changes. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
spec/test.spec.js (1)
40-62: Consider adding semicolons for style consistency.The file starts with
'use strict'but this new test block omits semicolons on most lines. Consider adding them for consistency with the rest of the codebase's style.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/test.spec.js` around lines 40 - 62, The new test block under describe('deleteFile') uses inconsistent missing semicolons; update the beforeEach block and variable declarations (gcsAdapter, mockStorage, mockBucket, mockFile, mockDelete) to add semicolons at the end of statements to match the project's `'use strict'` style—ensure each let/assignment and spy setup lines (e.g., mockDelete = jasmine.createSpy('delete'), mockFile = { delete: mockDelete }, mockBucket = jasmine.createSpyObj('bucket', ['file']), mockBucket.file.and.returnValue(mockFile), mockStorage = jasmine.createSpyObj('storage', ['bucket']), mockStorage.bucket.and.returnValue(mockBucket), gcsAdapter = new GCSAdapter(...), gcsAdapter._gcsClient = mockStorage) end with semicolons.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@spec/test.spec.js`:
- Around line 40-62: The new test block under describe('deleteFile') uses
inconsistent missing semicolons; update the beforeEach block and variable
declarations (gcsAdapter, mockStorage, mockBucket, mockFile, mockDelete) to add
semicolons at the end of statements to match the project's `'use strict'`
style—ensure each let/assignment and spy setup lines (e.g., mockDelete =
jasmine.createSpy('delete'), mockFile = { delete: mockDelete }, mockBucket =
jasmine.createSpyObj('bucket', ['file']),
mockBucket.file.and.returnValue(mockFile), mockStorage =
jasmine.createSpyObj('storage', ['bucket']),
mockStorage.bucket.and.returnValue(mockBucket), gcsAdapter = new
GCSAdapter(...), gcsAdapter._gcsClient = mockStorage) end with semicolons.
|
@coderabbitai does new pushed changes fix concerns ? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 5046
What was addedYour latest commit ✅ Mock the GCS SDK chain - No credentials required Concerns addressed1. v7 Breaking Changes: The tests verify that the response object is returned from 2. Interface Contract: The tests properly verify the method contract without requiring actual GCS credentials or Parse Server, exactly as requested. Minor noteThe only test case from the suggestion that wasn't included is special characters in filenames (e.g., Overall, excellent work! The tests significantly reduce the risk of the v7 upgrade. 🎉 |
Add semicolons in the deleteFile unit test block to match repository style and address review feedback without changing behavior. Made-with: Cursor
|
@Moumouls ready for merge? |
|
yes @mtrezza ready for merge ! |
# [2.1.0](2.0.0...2.1.0) (2026-02-27) ### Features * Upgrade to @google-cloud/storage 7.19.0 ([#57](#57)) ([b1446ed](b1446ed))
|
🎉 This change has been released in version 2.1.0 |
3 participants
Minimum supported node version is 18, this is a breaking change and should be released as a major
Summary by CodeRabbit
Chores
Tests