feat: add complete OSS client methods by killagu · Pull Request #43 · layotto/js-sdk

killagu · 2025-11-06T08:48:50Z

Add 20 missing OSS methods to complete the gRPC client implementation:

Batch operations: deleteObjects, isObjectExist, listObjectVersions
Tagging: putObjectTagging, deleteObjectTagging, getObjectTagging
ACL: getObjectCannedAcl, putObjectCannedAcl
Multipart upload: createMultipartUpload, uploadPart, uploadPartCopy, completeMultipartUpload, abortMultipartUpload, listMultipartUploads, listParts
Advanced: appendObject, restoreObject, updateDownloadBandwidthRateLimit, updateUploadBandwidthRateLimit

All methods follow the existing code patterns with:

Type definitions in src/types/Oss.ts
Implementation in src/client/Oss.ts
Comprehensive test coverage in test/unit/client/Oss.test.ts

🤖 Generated with Claude Code

What this PR does:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

fengmk2 · 2025-11-06T09:51:49Z

@wenxuwan go 侧也实现了吗

killagu · 2025-11-10T02:47:33Z

@wenxuwan go 侧也实现了吗

实现了，我在本地测试过 oss 的能力了。

Add 20 missing OSS methods to complete the gRPC client implementation: - Batch operations: deleteObjects, isObjectExist, listObjectVersions - Tagging: putObjectTagging, deleteObjectTagging, getObjectTagging - ACL: getObjectCannedAcl, putObjectCannedAcl - Multipart upload: createMultipartUpload, uploadPart, uploadPartCopy, completeMultipartUpload, abortMultipartUpload, listMultipartUploads, listParts - Advanced: appendObject, restoreObject, updateDownloadBandwidthRateLimit, updateUploadBandwidthRateLimit All methods follow the existing code patterns with: - Type definitions in src/types/Oss.ts - Implementation in src/client/Oss.ts - Comprehensive test coverage in test/unit/client/Oss.test.ts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

akitaSummer

1

fengmk2 · 2025-11-10T23:39:58Z

我先将自动发布配置好，然后合并

Copilot

Pull Request Overview

This PR adds 20 missing OSS (Object Storage Service) methods to complete the gRPC client implementation, including batch operations, tagging, ACL management, multipart uploads, and advanced features. The changes also include an unrelated refactoring in State.ts to improve empty message detection and a Docker image update for the CI workflow.

Adds comprehensive OSS client methods for batch operations, tagging, ACL, multipart uploads, and bandwidth control
Includes type definitions, implementation following existing patterns, and comprehensive test coverage
Refactors State.ts to replace isEmptyPBMessage utility with inline empty check logic

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/types/Oss.ts	Adds type definitions for 20 new OSS operations including request types for batch operations, tagging, ACL, and multipart uploads
src/client/Oss.ts	Implements all new OSS methods following existing patterns with proper error handling and metadata management
test/unit/client/Oss.test.ts	Adds comprehensive test coverage for all new OSS methods
src/client/State.ts	Refactors empty message detection to use inline checks instead of `isEmptyPBMessage` utility
.github/workflows/nodejs.yml	Updates etcd Docker image from `bitnami/etcd` to `bitnamilegacy/etcd`

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-10T23:41:34Z

test/unit/client/Oss.test.ts

 import path from 'node:path';
 import crypto from 'node:crypto';
 import { Client } from '../../../src';
+import { randomUUID } from 'node:crypto';


The randomUUID import is duplicated. It's imported from 'node:crypto' on line 7, but crypto is already imported on line 5. Consider using crypto.randomUUID() instead of a separate import.

Suggested change

import { randomUUID } from 'node:crypto';

Copilot · 2025-11-10T23:41:34Z

test/unit/client/Oss.test.ts


 describe.skip('client/Oss.test.ts', () => {
-  const client = new Client('34901', '127.0.0.1', { ossEnable: true });
+  const client = new Client('34904', '127.0.0.1', { ossEnable: true });


[nitpick] The port number changed from '34901' to '34904'. If this is an intentional configuration change, consider documenting why this specific port is needed, or use an environment variable for easier test configuration.

Suggested change

const client = new Client('34904', '127.0.0.1', { ossEnable: true });

// Use OSS_TEST_PORT environment variable to configure the port for easier test configuration.

// Defaults to '34904' if not set. Previously, port '34901' was used.

const client = new Client(process.env.OSS_TEST_PORT || '34904', '127.0.0.1', { ossEnable: true });

Copilot · 2025-11-10T23:41:35Z

test/unit/client/Oss.test.ts

+    console.log(headRes);
+    // assert(headRes.contentLength);
+    // assert.equal(headRes.contentLength, 9);


Debug console.log statement should be removed before merging to production.

Suggested change

console.log(headRes);

// assert(headRes.contentLength);

// assert.equal(headRes.contentLength, 9);

assert(headRes.contentLength);

assert.equal(headRes.contentLength, 9);

Copilot · 2025-11-10T23:41:35Z

test/unit/client/Oss.test.ts

+      bucket: 'antsys-tnpmbuild',
+      key,
+      body: Readable.from(Buffer.from('hello ')),
+      // position: 0,


Commented-out code should either be implemented or removed. If this parameter is optional and intentionally omitted for the first append, consider adding a comment explaining why.

Suggested change

// position: 0,

// position is optional for the first append; defaults to 0

Copilot · 2025-11-10T23:41:35Z

test/unit/client/Oss.test.ts

+  it('test uploadPartCopy', async () => {
+    const key = `test_part_copy_source${randomUUID()}.txt`;
+    // Create source object
+    const sourceData = Buffer.alloc(2 * 1024 * 1024).fill('s'); // 5MB


The comment says '5MB' but the buffer size is 2 * 1024 * 1024 bytes (2MB). The comment should be corrected to match the actual size.

Suggested change

const sourceData = Buffer.alloc(2 * 1024 * 1024).fill('s'); // 5MB

const sourceData = Buffer.alloc(2 * 1024 * 1024).fill('s'); // 2MB

Copilot · 2025-11-10T23:41:36Z

src/client/Oss.ts

+      const req = new UploadPartInput();
+      req.setStoreName(request.storeName);
+      req.setBucket(request.bucket);
+      req.setKey(key);
+      req.setContentLength(request.contentLength);
+      req.setPartNumber(request.partNumber);
+      req.setUploadId(request.uploadId);
+      if (request.contentMd5) {
+        req.setContentMd5(request.contentMd5);
+      }
+      if (request.expectedBucketOwner) {
+        req.setExpectedBucketOwner(request.expectedBucketOwner);
+      }
+      if (request.requestPayer) {
+        req.setRequestPayer(request.requestPayer);
+      }
+      if (request.sseCustomerAlgorithm) {
+        req.setSseCustomerAlgorithm(request.sseCustomerAlgorithm);
+      }
+      if (request.sseCustomerKey) {
+        req.setSseCustomerKey(request.sseCustomerKey);
+      }
+      if (request.sseCustomerKeyMd5) {
+        req.setSseCustomerKeyMd5(request.sseCustomerKeyMd5);
+      }
+      req.setBody(chunk);
+      yield req;
+    }
+
+    if (!hasChunk) {
+      // upload empty part
+      const req = new UploadPartInput();
+      req.setStoreName(request.storeName);
+      req.setBucket(request.bucket);
+      req.setKey(key);
+      req.setContentLength(request.contentLength);
+      req.setPartNumber(request.partNumber);
+      req.setUploadId(request.uploadId);
+      if (request.contentMd5) {
+        req.setContentMd5(request.contentMd5);
+      }
+      if (request.expectedBucketOwner) {
+        req.setExpectedBucketOwner(request.expectedBucketOwner);
+      }
+      if (request.requestPayer) {
+        req.setRequestPayer(request.requestPayer);
+      }
+      if (request.sseCustomerAlgorithm) {
+        req.setSseCustomerAlgorithm(request.sseCustomerAlgorithm);
+      }
+      if (request.sseCustomerKey) {
+        req.setSseCustomerKey(request.sseCustomerKey);
+      }
+      if (request.sseCustomerKeyMd5) {
+        req.setSseCustomerKeyMd5(request.sseCustomerKeyMd5);
+      }
+      yield req;
+    }
+  }
+


Significant code duplication: the request building logic (lines 738-764) is duplicated in lines 769-794. Consider extracting a helper method to build the UploadPartInput object to reduce duplication and improve maintainability. The same pattern appears in appendObjectIterator (lines 982-1075).

Suggested change

const req = new UploadPartInput();

req.setStoreName(request.storeName);

req.setBucket(request.bucket);

req.setKey(key);

req.setContentLength(request.contentLength);

req.setPartNumber(request.partNumber);

req.setUploadId(request.uploadId);

if (request.contentMd5) {

req.setContentMd5(request.contentMd5);

}

if (request.expectedBucketOwner) {

req.setExpectedBucketOwner(request.expectedBucketOwner);

}

if (request.requestPayer) {

req.setRequestPayer(request.requestPayer);

}

if (request.sseCustomerAlgorithm) {

req.setSseCustomerAlgorithm(request.sseCustomerAlgorithm);

}

if (request.sseCustomerKey) {

req.setSseCustomerKey(request.sseCustomerKey);

}

if (request.sseCustomerKeyMd5) {

req.setSseCustomerKeyMd5(request.sseCustomerKeyMd5);

}

req.setBody(chunk);

yield req;

}

if (!hasChunk) {

// upload empty part

const req = new UploadPartInput();

req.setStoreName(request.storeName);

req.setBucket(request.bucket);

req.setKey(key);

req.setContentLength(request.contentLength);

req.setPartNumber(request.partNumber);

req.setUploadId(request.uploadId);

if (request.contentMd5) {

req.setContentMd5(request.contentMd5);

}

if (request.expectedBucketOwner) {

req.setExpectedBucketOwner(request.expectedBucketOwner);

}

if (request.requestPayer) {

req.setRequestPayer(request.requestPayer);

}

if (request.sseCustomerAlgorithm) {

req.setSseCustomerAlgorithm(request.sseCustomerAlgorithm);

}

if (request.sseCustomerKey) {

req.setSseCustomerKey(request.sseCustomerKey);

}

if (request.sseCustomerKeyMd5) {

req.setSseCustomerKeyMd5(request.sseCustomerKeyMd5);

}

yield req;

}

}

const req = this.buildUploadPartInput(request, key, chunk);

yield req;

}

if (!hasChunk) {

// upload empty part

const req = this.buildUploadPartInput(request, key);

yield req;

}

}

/**

* Helper to build UploadPartInput from request, key, and optional chunk.

*/

private buildUploadPartInput(

request: UploadPartRequest,

key: string,

chunk?: any

): UploadPartInput {

const req = new UploadPartInput();

req.setStoreName(request.storeName);

req.setBucket(request.bucket);

req.setKey(key);

req.setContentLength(request.contentLength);

req.setPartNumber(request.partNumber);

req.setUploadId(request.uploadId);

if (request.contentMd5) {

req.setContentMd5(request.contentMd5);

}

if (request.expectedBucketOwner) {

req.setExpectedBucketOwner(request.expectedBucketOwner);

}

if (request.requestPayer) {

req.setRequestPayer(request.requestPayer);

}

if (request.sseCustomerAlgorithm) {

req.setSseCustomerAlgorithm(request.sseCustomerAlgorithm);

}

if (request.sseCustomerKey) {

req.setSseCustomerKey(request.sseCustomerKey);

}

if (request.sseCustomerKeyMd5) {

req.setSseCustomerKeyMd5(request.sseCustomerKeyMd5);

}

if (chunk !== undefined) {

req.setBody(chunk);

}

return req;

}

Copilot · 2025-11-10T23:41:36Z

src/client/Oss.ts

+    let resolve;
+    let reject;
+    const promise = new Promise<UploadPartOutput.AsObject>((res, rej) => {
+      resolve = res;
+      reject = rej;
+    });


Non-idiomatic Promise usage with untyped let variables. The resolve and reject variables lack type annotations. Consider declaring them with proper types: let resolve: (value: UploadPartOutput.AsObject) => void; and let reject: (reason?: any) => void; or restructure to avoid this pattern. This pattern also appears in appendObject (lines 1078-1083).

Copilot · 2025-11-10T23:41:36Z

.github/workflows/nodejs.yml

      # https://github.com/mosn/layotto/blob/main/docker/layotto-etcd/docker-compose.yml
      etcd:
-        image: bitnami/etcd
+        image: bitnamilegacy/etcd


[nitpick] The Docker image was changed from 'bitnami/etcd' to 'bitnamilegacy/etcd'. The 'legacy' naming suggests this may be an outdated or deprecated image. Consider documenting why this change was necessary or verify if a more recent stable image is available.

Suggested change

image: bitnamilegacy/etcd

image: bitnami/etcd

fengmk2 · 2025-11-11T13:45:44Z

+1

fengmk2 · 2025-11-11T13:56:54Z

https://github.com/layotto/js-sdk/releases/tag/v1.18.0

killagu force-pushed the feat/oss_features branch from cb2282b to d483934 Compare November 10, 2025 03:19

killagu requested a review from akitaSummer November 10, 2025 15:32

akitaSummer approved these changes Nov 10, 2025

View reviewed changes

fengmk2 approved these changes Nov 10, 2025

View reviewed changes

fengmk2 requested a review from Copilot November 10, 2025 23:38

Copilot started reviewing on behalf of fengmk2 November 10, 2025 23:38 View session

Copilot finished reviewing on behalf of fengmk2 November 10, 2025 23:40

Copilot AI reviewed Nov 10, 2025

View reviewed changes

fengmk2 merged commit 2b154b7 into main Nov 11, 2025
14 checks passed

fengmk2 deleted the feat/oss_features branch November 11, 2025 13:46

-  const client = new Client('34904', '127.0.0.1', { ossEnable: true });
+  // Use OSS_TEST_PORT environment variable to configure the port for easier test configuration.
+  // Defaults to '34904' if not set. Previously, port '34901' was used.
+  const client = new Client(process.env.OSS_TEST_PORT || '34904', '127.0.0.1', { ossEnable: true });

	// position: 0,
	// position is optional for the first append; defaults to 0

	const sourceData = Buffer.alloc(2 * 1024 * 1024).fill('s'); // 5MB
	const sourceData = Buffer.alloc(2 * 1024 * 1024).fill('s'); // 2MB

Comments

Conversation

killagu commented Nov 6, 2025

Uh oh!

fengmk2 commented Nov 6, 2025

Uh oh!

killagu commented Nov 10, 2025

Uh oh!

akitaSummer left a comment

Choose a reason for hiding this comment

Uh oh!

fengmk2 commented Nov 10, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

fengmk2 commented Nov 11, 2025

Uh oh!

Uh oh!

fengmk2 commented Nov 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants