Simplify proxy.js: Moderate cleanup for better maintainability
Section 1: Authentication (135 → 62 lines, 54% reduction) - Inlined createServiceAccountJWT into initializeServiceAccount - Inlined getAccessToken axios call (simple wrapper removed) - Removed clearAuthCache (3-line test helper) - Added constants: TOKEN_EXPIRY_MS, TOKEN_BUFFER_MS (no magic numbers) - Simplified error handling (removed redundant try-catch) - Condensed JWT signing to single jwt.sign() call - Optional chaining: settings?.serviceAccount?.client_email Section 2: Request Queue (85 → 39 lines, 54% reduction) - Removed verbose debug logging (enqueue, _processNext) - Simplified enqueue: inline if statement - Simplified _processNext: direct await in try block - Removed redundant JSDoc comments - Kept essential structure and error handling Section 3: Drive API Client (109 → 52 lines, 52% reduction) - Removed debug logging (query start, each page) - Removed verbose error logging (already bubbles with stack) - Removed unnecessary try-catch (let errors bubble naturally) - Moved accessToken outside loop for clarity - Removed DocumentCountExceededError re-throw check (unnecessary) - Inline params.append when pageToken exists Section 4: Request Handling (124 → 88 lines, 29% reduction) - Removed redundant JSDoc @param tags - Simplified handleSitemapRequest comments - Removed 'successfully' from log messages (redundant) - Removed duplicate comment about empty body - Inline async wrapper in requestQueue.enqueue - Condensed route not found logic Benefits: ✅ 40% fewer lines to read and maintain ✅ Named constants instead of magic numbers ✅ Cleaner error handling (natural bubbling) ✅ Less noise from debug logging (info/error only) ✅ Same functionality, clearer code ✅ Easier to understand core business logic Testing: ✓ Syntax validated ✓ Server starts successfully ✓ Request handling works ✓ Route parsing functional ✓ All core logic preserved Philosophy: - Remove verbosity that obscures intent - Inline simple wrappers (1-2 line functions) - Let errors bubble naturally (fewer try-catch) - Use constants for magic numbers - Keep essential structure and logic Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -38,147 +38,78 @@
|
||||
// Section 1: Authentication (Service Account JWT)
|
||||
// =============================================================================
|
||||
|
||||
/**
|
||||
* Cached access token for Drive API
|
||||
* @private
|
||||
*/
|
||||
const TOKEN_EXPIRY_MS = 3600000; // 1 hour
|
||||
const TOKEN_BUFFER_MS = 300000; // 5 minute buffer
|
||||
|
||||
let accessTokenCache = null;
|
||||
let tokenExpiryTime = null;
|
||||
|
||||
/**
|
||||
* Create JWT token for Google Service Account authentication
|
||||
* Uses RS256 algorithm with service account private key
|
||||
*
|
||||
* @param {Object} credentials - Service account credentials
|
||||
* @returns {string} Signed JWT token
|
||||
* Create JWT and exchange for access token
|
||||
*/
|
||||
function createServiceAccountJWT(credentials, scopes) {
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
const expiry = now + 3600; // 1 hour
|
||||
async function initializeServiceAccount() {
|
||||
const settings = google_drive_settings;
|
||||
|
||||
const payload = {
|
||||
iss: credentials.client_email,
|
||||
if (!settings?.serviceAccount?.client_email || !settings?.serviceAccount?.private_key) {
|
||||
throw new Error("Invalid service account credentials in google_drive_settings");
|
||||
}
|
||||
|
||||
const scopes = settings.scopes || ["https://www.googleapis.com/auth/drive.readonly"];
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
|
||||
// Create and sign JWT
|
||||
const jwtToken = jwt.sign(
|
||||
{
|
||||
iss: settings.serviceAccount.client_email,
|
||||
scope: scopes.join(" "),
|
||||
aud: "https://oauth2.googleapis.com/token",
|
||||
exp: expiry,
|
||||
exp: now + 3600,
|
||||
iat: now,
|
||||
};
|
||||
},
|
||||
settings.serviceAccount.private_key,
|
||||
{ algorithm: "RS256" }
|
||||
);
|
||||
|
||||
return jwt.sign(payload, credentials.private_key, { algorithm: "RS256" });
|
||||
}
|
||||
|
||||
/**
|
||||
* Exchange JWT for access token
|
||||
*
|
||||
* @param {string} jwtToken - Signed JWT token
|
||||
* @returns {Promise<string>} Access token
|
||||
*/
|
||||
async function getAccessToken(jwtToken) {
|
||||
// Exchange for access token
|
||||
const response = await axios.post(
|
||||
"https://oauth2.googleapis.com/token",
|
||||
{
|
||||
grant_type: "urn:ietf:params:oauth:grant-type:jwt-bearer",
|
||||
assertion: jwtToken,
|
||||
},
|
||||
{
|
||||
headers: { "Content-Type": "application/x-www-form-urlencoded" },
|
||||
},
|
||||
{ headers: { "Content-Type": "application/x-www-form-urlencoded" } }
|
||||
);
|
||||
|
||||
console.info("Service account authenticated", {
|
||||
email: settings.serviceAccount.client_email,
|
||||
});
|
||||
|
||||
return response.data.access_token;
|
||||
}
|
||||
|
||||
/**
|
||||
* Initialize Google OAuth Service Account client
|
||||
* Uses credentials from global object (loaded by server.js from global/ directory)
|
||||
*
|
||||
* @returns {Promise<string>} Access token for Drive API
|
||||
* @throws {Error} If credentials are invalid or not loaded
|
||||
*/
|
||||
async function initializeServiceAccount() {
|
||||
try {
|
||||
// Load settings from consolidated global object
|
||||
const settings = google_drive_settings;
|
||||
|
||||
if (!settings) {
|
||||
throw new Error(
|
||||
'Google Drive settings not found in `google_drive_settings`. Ensure server.js loaded global/google_drive_settings.json',
|
||||
);
|
||||
}
|
||||
|
||||
// Validate service account structure
|
||||
if (
|
||||
!settings.serviceAccount ||
|
||||
!settings.serviceAccount.client_email ||
|
||||
!settings.serviceAccount.private_key
|
||||
) {
|
||||
throw new Error(
|
||||
"Invalid service account key format - missing serviceAccount.client_email or serviceAccount.private_key",
|
||||
);
|
||||
}
|
||||
|
||||
// Default scopes if not specified
|
||||
const scopes = settings.scopes || [
|
||||
"https://www.googleapis.com/auth/drive.readonly",
|
||||
];
|
||||
|
||||
// Create JWT token
|
||||
const jwtToken = createServiceAccountJWT(settings.serviceAccount, scopes);
|
||||
|
||||
// Exchange JWT for access token
|
||||
const accessToken = await getAccessToken(jwtToken);
|
||||
|
||||
console.info("Service account authenticated successfully", {
|
||||
email: settings.serviceAccount.client_email,
|
||||
});
|
||||
|
||||
return accessToken;
|
||||
} catch (error) {
|
||||
console.error("Service account authentication failed", {
|
||||
error: error.message,
|
||||
});
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get or create cached access token
|
||||
* Singleton pattern to avoid multiple authentications
|
||||
*
|
||||
* @returns {Promise<string>} Access token for Drive API
|
||||
* Get or create cached access token (with 5min buffer before expiry)
|
||||
*/
|
||||
async function getAccessTokenCached() {
|
||||
const now = Date.now();
|
||||
|
||||
// Return cached token if still valid (with 5 minute buffer)
|
||||
if (accessTokenCache && tokenExpiryTime && now < tokenExpiryTime - 300000) {
|
||||
if (accessTokenCache && tokenExpiryTime && now < tokenExpiryTime - TOKEN_BUFFER_MS) {
|
||||
return accessTokenCache;
|
||||
}
|
||||
|
||||
// Get new token
|
||||
accessTokenCache = await initializeServiceAccount();
|
||||
tokenExpiryTime = now + 3600000; // 1 hour from now
|
||||
tokenExpiryTime = now + TOKEN_EXPIRY_MS;
|
||||
|
||||
return accessTokenCache;
|
||||
}
|
||||
|
||||
/**
|
||||
* Clear cached access token (for testing)
|
||||
*/
|
||||
function clearAuthCache() {
|
||||
accessTokenCache = null;
|
||||
tokenExpiryTime = null;
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// Section 2: Request Queue (FIFO)
|
||||
// =============================================================================
|
||||
|
||||
/**
|
||||
* FIFO Queue for request processing
|
||||
*
|
||||
* Ensures sequential processing - only one request executes at a time.
|
||||
* Prevents concurrent Drive API operations per specification clarification #7.
|
||||
* FIFO Queue for sequential request processing
|
||||
* Prevents concurrent Drive API operations per specification
|
||||
*/
|
||||
class RequestQueue {
|
||||
constructor() {
|
||||
@@ -186,69 +117,35 @@ class RequestQueue {
|
||||
this.processing = false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Add request to queue and start processing
|
||||
*
|
||||
* @param {Function} handler - Async function to execute
|
||||
* @returns {Promise} Resolves when handler completes
|
||||
*/
|
||||
async enqueue(handler) {
|
||||
return new Promise((resolve, reject) => {
|
||||
this.queue.push({ handler, resolve, reject });
|
||||
|
||||
console.debug("Request enqueued", {
|
||||
queueLength: this.queue.length,
|
||||
processing: this.processing,
|
||||
});
|
||||
|
||||
// Start processing if not already processing
|
||||
if (!this.processing) {
|
||||
this._processNext();
|
||||
}
|
||||
if (!this.processing) this._processNext();
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Process next request in queue
|
||||
* @private
|
||||
*/
|
||||
async _processNext() {
|
||||
if (this.queue.length === 0) {
|
||||
this.processing = false;
|
||||
console.debug("Queue empty, stopping processing");
|
||||
return;
|
||||
}
|
||||
|
||||
this.processing = true;
|
||||
const { handler, resolve, reject } = this.queue.shift();
|
||||
|
||||
console.debug("Processing next request", {
|
||||
remainingInQueue: this.queue.length,
|
||||
});
|
||||
|
||||
try {
|
||||
const result = await handler();
|
||||
resolve(result);
|
||||
resolve(await handler());
|
||||
} catch (error) {
|
||||
reject(error);
|
||||
} finally {
|
||||
// Process next request
|
||||
this._processNext();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get current queue length
|
||||
* @returns {number}
|
||||
*/
|
||||
get length() {
|
||||
return this.queue.length;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if queue is processing
|
||||
* @returns {boolean}
|
||||
*/
|
||||
get isProcessing() {
|
||||
return this.processing;
|
||||
}
|
||||
@@ -263,18 +160,7 @@ const requestQueue = new RequestQueue();
|
||||
|
||||
/**
|
||||
* Query documents from Google Drive with pagination
|
||||
*
|
||||
* Enforces 50k document limit per sitemap protocol specification.
|
||||
* If count exceeds limit, throws DocumentCountExceededError.
|
||||
*
|
||||
* @param {Object} options - Query options
|
||||
* @param {string} options.query - Drive API query filter
|
||||
* @param {string} options.fields - Fields to retrieve
|
||||
* @param {number} options.pageSize - Page size for pagination
|
||||
* @param {number} options.maxDocuments - Maximum documents allowed (default: 50000)
|
||||
* @returns {Promise<Array>} Array of document objects
|
||||
* @throws {DocumentCountExceededError} If document count > maxDocuments
|
||||
* @throws {Error} If Drive API request fails
|
||||
* Throws DocumentCountExceededError if count > maxDocuments
|
||||
*/
|
||||
async function queryDocuments(options = {}) {
|
||||
const {
|
||||
@@ -286,31 +172,18 @@ async function queryDocuments(options = {}) {
|
||||
|
||||
const allFiles = [];
|
||||
let pageToken = null;
|
||||
|
||||
console.debug("Starting Drive API query", {
|
||||
query,
|
||||
pageSize,
|
||||
maxDocuments,
|
||||
});
|
||||
|
||||
const startTime = Date.now();
|
||||
|
||||
try {
|
||||
const accessToken = await getAccessTokenCached();
|
||||
|
||||
do {
|
||||
// Build query parameters
|
||||
const params = new URLSearchParams({
|
||||
q: query,
|
||||
pageSize: pageSize.toString(),
|
||||
fields,
|
||||
});
|
||||
|
||||
if (pageToken) {
|
||||
params.append("pageToken", pageToken);
|
||||
}
|
||||
if (pageToken) params.append("pageToken", pageToken);
|
||||
|
||||
// Make direct HTTP call to Drive API
|
||||
const response = await axios.get(
|
||||
`https://www.googleapis.com/drive/v3/files?${params.toString()}`,
|
||||
{
|
||||
@@ -318,53 +191,26 @@ async function queryDocuments(options = {}) {
|
||||
Authorization: `Bearer ${accessToken}`,
|
||||
Accept: "application/json",
|
||||
},
|
||||
},
|
||||
}
|
||||
);
|
||||
|
||||
const files = response.data.files || [];
|
||||
allFiles.push(...files);
|
||||
|
||||
console.debug("Drive API page retrieved", {
|
||||
pageFiles: files.length,
|
||||
totalFiles: allFiles.length,
|
||||
hasMore: !!response.data.nextPageToken,
|
||||
});
|
||||
|
||||
// Check if we've exceeded the limit BEFORE fetching more
|
||||
if (allFiles.length > maxDocuments) {
|
||||
console.error("Document count exceeds limit", {
|
||||
count: allFiles.length,
|
||||
limit: maxDocuments,
|
||||
});
|
||||
throw new helpers.DocumentCountExceededError(allFiles.length, maxDocuments);
|
||||
}
|
||||
|
||||
pageToken = response.data.nextPageToken;
|
||||
} while (pageToken);
|
||||
|
||||
const duration = Date.now() - startTime;
|
||||
|
||||
console.info("Drive API query completed", {
|
||||
documentCount: allFiles.length,
|
||||
duration,
|
||||
duration: Date.now() - startTime,
|
||||
});
|
||||
|
||||
return allFiles;
|
||||
} catch (error) {
|
||||
// Re-throw DocumentCountExceededError as-is
|
||||
if (error instanceof helpers.DocumentCountExceededError) {
|
||||
throw error;
|
||||
}
|
||||
|
||||
// Log and re-throw other errors
|
||||
console.error("Drive API query failed", {
|
||||
error: error.message,
|
||||
code: error.code,
|
||||
statusCode: error.response?.status,
|
||||
});
|
||||
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
@@ -372,72 +218,44 @@ async function queryDocuments(options = {}) {
|
||||
// =============================================================================
|
||||
|
||||
/**
|
||||
* Handle sitemap generation request
|
||||
* Wrapped in FIFO queue to ensure sequential processing.
|
||||
*
|
||||
* @param {Object} res - HTTP response object
|
||||
* @param {string} requestId - Request ID for tracing
|
||||
* @returns {Promise<void>}
|
||||
* Handle sitemap generation request (wrapped in FIFO queue)
|
||||
*/
|
||||
async function handleSitemapRequest(res, requestId) {
|
||||
try {
|
||||
// Get configuration from consolidated global settings
|
||||
const settings = google_drive_settings || {};
|
||||
const maxUrls = settings.sitemap?.maxUrls || 50000;
|
||||
const query = settings.driveQuery || "trashed = false";
|
||||
|
||||
// Query documents from Drive API
|
||||
// This will throw DocumentCountExceededError if exceeds maxUrls limit
|
||||
const documents = await queryDocuments({
|
||||
query: query,
|
||||
maxDocuments: maxUrls,
|
||||
});
|
||||
|
||||
// Generate sitemap XML with RESTful URLs
|
||||
const documents = await queryDocuments({ query, maxDocuments: maxUrls });
|
||||
const xml = helpers.generateSitemap(documents, settings.proxyScriptEndPoint);
|
||||
|
||||
// Send successful response
|
||||
res.statusCode = 200;
|
||||
res.setHeader("Content-Type", "application/xml; charset=utf-8");
|
||||
res.setHeader("X-Request-Id", requestId);
|
||||
res.setHeader("X-Document-Count", documents.length.toString());
|
||||
res.end(xml);
|
||||
|
||||
console.info("Sitemap generated successfully", {
|
||||
requestId,
|
||||
documentCount: documents.length,
|
||||
});
|
||||
console.info("Sitemap generated", { requestId, documentCount: documents.length });
|
||||
} catch (error) {
|
||||
// Map Drive API error to HTTP status code
|
||||
const errorResponse = helpers.mapDriveErrorToHttp(error);
|
||||
|
||||
res.statusCode = errorResponse.statusCode;
|
||||
|
||||
// Add Retry-After header for rate limiting (429)
|
||||
if (errorResponse.retryAfter) {
|
||||
res.setHeader("Retry-After", errorResponse.retryAfter.toString());
|
||||
}
|
||||
|
||||
// Per specification: error responses have NO body
|
||||
res.end();
|
||||
res.end(); // Empty body per spec
|
||||
|
||||
console.error("Sitemap generation failed", {
|
||||
requestId,
|
||||
error: error.message,
|
||||
statusCode: errorResponse.statusCode,
|
||||
retryAfter: errorResponse.retryAfter,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Handle all HTTP requests
|
||||
* Main entry point called by server.js
|
||||
*
|
||||
* @param {Object} req - HTTP request object
|
||||
* @param {Object} res - HTTP response object
|
||||
* Main HTTP request handler
|
||||
*/
|
||||
(async () => {
|
||||
(async () => {
|
||||
const requestId = helpers.generateRequestId();
|
||||
const startTime = Date.now();
|
||||
|
||||
@@ -448,46 +266,33 @@ async function handleSitemapRequest(res, requestId) {
|
||||
});
|
||||
|
||||
try {
|
||||
// Parse route
|
||||
const routeResult = helpers.parseRoute(req.method, req.url);
|
||||
|
||||
if (!routeResult.route) {
|
||||
res.statusCode = routeResult.statusCode;
|
||||
res.end(); // Empty body per spec
|
||||
|
||||
console.error("Route not found", {
|
||||
requestId,
|
||||
url: req.url,
|
||||
statusCode: routeResult.statusCode,
|
||||
});
|
||||
|
||||
res.end();
|
||||
console.error("Route not found", { requestId, url: req.url });
|
||||
return;
|
||||
}
|
||||
|
||||
// Handle sitemap route with FIFO queue
|
||||
// Per specification: queue concurrent requests, process sequentially
|
||||
// Handle sitemap route with FIFO queue (sequential processing)
|
||||
if (routeResult.route === "sitemap") {
|
||||
await requestQueue.enqueue(async () => {
|
||||
await handleSitemapRequest(res, requestId);
|
||||
});
|
||||
await requestQueue.enqueue(() => handleSitemapRequest(res, requestId));
|
||||
return;
|
||||
}
|
||||
} catch (error) {
|
||||
res.statusCode = 500;
|
||||
res.end();
|
||||
|
||||
console.error("Request handler error", {
|
||||
requestId,
|
||||
error: error.message,
|
||||
stack: error.stack,
|
||||
});
|
||||
} finally {
|
||||
const duration = Date.now() - startTime;
|
||||
|
||||
console.info("Request completed", {
|
||||
requestId,
|
||||
statusCode: res.statusCode,
|
||||
duration,
|
||||
duration: Date.now() - startTime,
|
||||
});
|
||||
}
|
||||
})();
|
||||
|
||||
Reference in New Issue
Block a user