From 43a6bc17e0b1081686311df12de199e1ab44503e Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Sun, 28 Sep 2025 15:05:52 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=9D=20Add=20docstrings=20to=20`code=5F?= =?UTF-8?q?revew=5Ftest`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docstrings generation was requested by @heliusjing. * https://github.com/heliusjing/coderabbit-test/pull/3#issuecomment-3329617121 The following files were modified: * `ApiGateway.java` * `AuditLogger.java` * `CacheManager.java` * `ConfigService.java` * `DataValidator.java` * `UserManager.java` --- ApiGateway.java | 517 +++++++++++++++++++++++++++++++++++++++++++-- AuditLogger.java | 136 +++++++++++- CacheManager.java | 157 ++++++++++++-- ConfigService.java | 115 ++++++++-- DataValidator.java | 122 ++++++++++- UserManager.java | 99 +++++++-- 6 files changed, 1078 insertions(+), 68 deletions(-) diff --git a/ApiGateway.java b/ApiGateway.java index 4484ad2..3e47b22 100644 --- a/ApiGateway.java +++ b/ApiGateway.java @@ -19,12 +19,25 @@ public class ApiGateway { // 服务发现但硬编码依赖 private Map serviceRegistry = new HashMap<>(); + /** + * Create a new ApiGateway, initializing internal services and registering HTTP routes. + * + *

This constructor initializes the gateway's service components and sets up route + * dispatching so the instance is ready to handle requests.

+ */ public ApiGateway() { initializeServices(); registerRoutes(); } - // 初始化所有服务但可能循环依赖 + /** + * Initializes gateway service instances and registers a subset in the serviceRegistry. + * + *

Instantiates internal service fields (UserService, UserManager, RestController, + * DataValidator, CacheManager, AuditLogger, ConfigService) and registers the user, validation, + * and cache services in the serviceRegistry. This method does not perform lifecycle management + * or guarantee complete/validated initialization of all services. + */ private void initializeServices() { this.userService = new UserService(); this.userManager = new UserManager(); // UserManager可能未正确初始化 @@ -40,11 +53,27 @@ private void initializeServices() { serviceRegistry.put("cache", cacheManager); } + /** + * Register the gateway's API endpoints and associate each endpoint with its handler. + * + *

Currently a placeholder with no routes registered.

+ */ private void registerRoutes() { // 路由注册逻辑，但没有实际实现 } - // 统一入口但安全检查不足 + /** + * Serves as the unified entry point for handling an HTTP-like request: enforces rate limiting, dispatches to the router, and records the request. + * + * This method performs a basic rate-limit check, routes the request to the appropriate handler, logs the web request, and returns the handler's result. If the rate limit is exceeded, it returns "Rate limit exceeded". On unexpected errors it logs the error and returns a string that includes the internal exception message. + * + * Note: the method records the request via the audit logger and may expose internal error messages in its return value. + * + * @param endpoint the API endpoint path used for routing (e.g., "/api/users/create") + * @param request the incoming HttpServletRequest whose parameters and client info are used for routing and rate limiting + * @param response the HttpServletResponse passed through to routed handlers (may be null for some callers) + * @return the routed handler's response string, or "Rate limit exceeded" if the request is throttled, or an internal error message on failure + */ public String handleRequest(String endpoint, HttpServletRequest request, HttpServletResponse response) { String startTime = String.valueOf(System.currentTimeMillis()); @@ -69,7 +98,21 @@ public String handleRequest(String endpoint, HttpServletRequest request, HttpSer } } - // 路由分发但API不一致 + /** + * Dispatches an API endpoint to the matching handler and returns its result. + * + * Supported endpoints: + * - "/api/users/create" + * - "/api/users/get" + * - "/api/users/validate" + * - "/api/cache/get" + * - "/api/files/upload" + * + * @param endpoint the request path used to select a handler (e.g. "/api/users/create") + * @param request the HttpServletRequest forwarded to the selected handler + * @param response the HttpServletResponse forwarded to the selected handler; may be null for handlers that do not use it + * @return the handler's response string, or "Unknown endpoint: " followed by the provided endpoint when no handler matches + */ private String routeRequest(String endpoint, HttpServletRequest request, HttpServletResponse response) { switch (endpoint) { case "/api/users/create": @@ -87,7 +130,13 @@ private String routeRequest(String endpoint, HttpServletRequest request, HttpSer } } - // 用户创建聚合多个服务但问题叠加 + /** + * Creates a new user from HTTP request parameters, validates the input, persists the user, caches the user data, and returns a human-readable status message. + * + * Validation errors are returned in the status message when input validation fails. + * + * @return A status message: `"User created successfully"` on success, or a validation error message describing the validation failures. + */ private String handleUserCreation(HttpServletRequest request, HttpServletResponse response) { String name = request.getParameter("name"); String email = request.getParameter("email"); @@ -112,7 +161,13 @@ private String handleUserCreation(HttpServletRequest request, HttpServletRespons return "User created successfully"; } - // 用户检索但API类型不匹配 + /** + * Retrieve a user by the "userId" request parameter, preferring a cached result and falling back to backend services. + * + * @param request HTTP request containing a "userId" parameter + * @return `cachedUser.toString()` if a cached user exists; otherwise the primary service's `toString()` for the user; + * if no user is found returns "User not found". The returned string may include sensitive fields (for example, passwords). + */ private String handleUserRetrieval(HttpServletRequest request) { String userId = request.getParameter("userId"); @@ -142,7 +197,14 @@ private String handleUserRetrieval(HttpServletRequest request) { return user1 != null ? user1.toString() : "User not found"; } - // 用户验证但重复多个验证逻辑 + /** + * Performs email and password validation with multiple validators, logs validation discrepancies, and invokes the REST login flow. + * + *

Reads "email" and "password" from the provided request, compares results from two email validators and logs a security event if they differ, validates the password using two methods, and calls the REST controller's login method.

+ * + * @param request HTTP request containing "email" and "password" parameters + * @return the literal string "Validation completed" + */ private String handleUserValidation(HttpServletRequest request) { String email = request.getParameter("email"); String password = request.getParameter("password"); @@ -165,7 +227,12 @@ private String handleUserValidation(HttpServletRequest request) { return "Validation completed"; } - // 缓存操作但暴露内部实现 + /** + * Fetches a value from the cache using the "key" request parameter and returns a short status message. + * + * @param request the HTTP request expected to contain a "key" parameter identifying the cache entry + * @return "Invalid cache key" if the key is empty or missing, "Cache hit: " if a cached object was found, or "Cache miss" if no entry exists + */ private String handleCacheGet(HttpServletRequest request) { String key = request.getParameter("key"); @@ -183,7 +250,14 @@ private String handleCacheGet(HttpServletRequest request) { } } - // 文件上传聚合但安全问题叠加 + /** + * Handles an incoming file upload request by validating the file path, delegating to REST upload and a file processor, and caching the file content. + * + * Expects the request to contain the parameters "fileName" and "content". If the file path validation fails, returns "Invalid file path". + * + * @param request the HTTP request containing "fileName" and "content" parameters + * @return "File uploaded successfully" on successful handling, or "Invalid file path" when validation fails + */ private String handleFileUpload(HttpServletRequest request) { String fileName = request.getParameter("fileName"); String content = request.getParameter("content"); @@ -209,7 +283,12 @@ private String handleFileUpload(HttpServletRequest request) { return "File uploaded successfully"; } - // 速率限制但实现有漏洞 + /** + * Enforces a per-client-IP rate limit and indicates whether the request is allowed. + * + * @param request the HTTP request whose remote address is used to identify the client + * @return `true` if the client's current request count is less than or equal to 100, `false` otherwise + */ private boolean checkRateLimit(HttpServletRequest request) { String clientIp = request.getRemoteAddr(); String cacheKey = "rate_limit_" + clientIp; @@ -225,7 +304,20 @@ private boolean checkRateLimit(HttpServletRequest request) { return count <= 100; } - // 健康检查但检查不全面 + /** + * Collects basic health indicators for the database, cache, and configuration service. + * + *

The returned map contains status entries for the following keys: + *

+ * + * Note: this is a lightweight, not exhaustive health check and may surface component-specific errors. + * + * @return a map with keys `"database"`, `"cache"`, and `"config"` describing each component's status + */ public Map healthCheck() { Map health = new HashMap<>(); @@ -249,7 +341,17 @@ public Map healthCheck() { return health; } - // 批量操作但没有事务管理 + /** + * Processes a list of request parameter maps sequentially and returns a brief summary. + * + *

Each map represents a single request and is used to construct a lightweight mock + * HttpServletRequest; the map should include an "endpoint" key and any other parameters + * required by the simulated request. Requests are handled one-by-one; failures for + * individual entries are recorded but do not stop processing of the remaining entries. + * + * @param requests a list of maps where each map contains request parameters (must include an "endpoint" entry) + * @return a summary string indicating how many requests were processed, e.g. "Batch completed: 3 requests processed" + */ public String handleBatch(List> requests) { List results = new ArrayList<>(); @@ -271,7 +373,15 @@ public String handleBatch(List> requests) { return "Batch completed: " + results.size() + " requests processed"; } - // 服务监控但可能泄漏内部信息 + /** + * Builds a textual listing of all services registered in the gateway with each entry's string representation. + * + * This method iterates the internal service registry and appends each service name followed by the result + * of that service object's `toString()` method on its own line; the output may therefore expose internal + * state if a service's `toString()` includes sensitive details. + * + * @return a string containing one line per registered service in the form "serviceName: serviceObject.toString()" + */ public String getServiceStatus() { StringBuilder status = new StringBuilder(); @@ -285,7 +395,13 @@ public String getServiceStatus() { return status.toString(); } - // 清理方法但清理不完整 + /** + * Performs a partial shutdown by cleaning audit logs and clearing the cache. + * + *

This method invokes cleanup on the audit logger and clears all entries from the cache. + * It does not attempt to shut down or clean other services (for example, userService or userManager), + * which may require their own lifecycle handling. + */ public void shutdown() { auditLogger.cleanup(); cacheManager.clearAll(); @@ -298,20 +414,49 @@ public void shutdown() { private static class MockHttpServletRequest implements HttpServletRequest { private Map parameters; + /** + * Creates a mock HttpServletRequest backed by the provided parameter map. + * + * The given map supplies parameter names and their corresponding values for methods + * such as getParameter() and for constructing a query string. + * + * @param parameters map of request parameter names to values (may be empty) + */ public MockHttpServletRequest(Map parameters) { this.parameters = parameters; } + /** + * Retrieve the value of a request parameter by name. + * + * @param name the parameter name to look up + * @return the parameter value, or `null` if the parameter is not present + */ @Override public String getParameter(String name) { return parameters.get(name); } + /** + * Returns the mock client's remote IP address. + * + * This implementation always returns the IPv4 loopback address "127.0.0.1". + * + * @return the hard-coded remote IP address "127.0.0.1" + */ @Override public String getRemoteAddr() { return "127.0.0.1"; // 硬编码 } + /** + * Builds a URL-style query string from the stored request parameters. + * + *

Constructs "key=value" pairs joined by '&' for each entry in the internal parameter map. + * Does not perform URL encoding; the order of pairs follows the map's iteration order. + * + * @return the assembled query string, or an empty string if there are no parameters + */ @Override public String getQueryString() { // 简单实现，可能不正确 @@ -329,254 +474,600 @@ public String getAuthType() { return null; } + /** + * Retrieve the cookies included with this request. + * + * @return an array of {@link javax.servlet.http.Cookie} objects present on the request, or `null` if no cookies are present + */ public javax.servlet.http.Cookie[] getCookies() { return null; } + /** + * Retrieve the value of the specified HTTP header interpreted as a date. + * + * @param name the HTTP header name + * @return the header value as milliseconds since the epoch, or {@code -1} if the header is not present + * @throws IllegalArgumentException if the header value cannot be parsed as a valid date + */ public long getDateHeader(String name) { return 0; } + /** + * Retrieve the value of the HTTP header with the given name. + * + * @param name the header name to look up + * @return the header value, or `null` if the header is not present + */ public String getHeader(String name) { return null; } + /** + * Retrieve all values for the specified HTTP header name. + * + * @param name the header name to look up (case-insensitive) + * @return an {@code Enumeration} of header value strings for the given name; an empty {@code Enumeration} if the header is not present + */ public Enumeration getHeaders(String name) { return null; } + /** + * Retrieve the names of all HTTP headers present in the request. + * + * @return an {@code Enumeration} of header name strings, or {@code null} if no headers are available + */ public Enumeration getHeaderNames() { return null; } + /** + * Retrieve the integer value of the specified header. + * + * @param name the header name + * @return the header value parsed as an int, or 0 if the header is absent or cannot be parsed as an integer + */ public int getIntHeader(String name) { return 0; } + /** + * Provide the HTTP method used for retrieval operations. + * + * @return the literal string "GET" + */ public String getMethod() { return "GET"; } + /** + * Retrieve the extra path information associated with this request. + * + * The path information is the part of the request URI that follows the servlet's path mapping and may be null. + * + * @return the path information, or null if no extra path information is available + */ public String getPathInfo() { return null; } + /** + * Returns the filesystem path that the request's path info maps to on the server. + * + * @return the translated filesystem path for the request's path info, or `null` if the path cannot be translated or there is no path info + */ public String getPathTranslated() { return null; } + /** + * Gets the gateway's configured HTTP context path. + * + * @return the configured context path (for example, "/api"), or `null` if no context path is configured + */ public String getContextPath() { return null; } + /** + * Retrieve the session identifier provided by the client with the request. + * + * @return the session id supplied by the client, or {@code null} if the client did not provide one + */ public String getRequestedSessionId() { return null; } + /** + * Retrieves the request URI associated with this mock request. + * + * @return the request URI as a string, or `null` if no URI has been set + */ public String getRequestURI() { return null; } + /** + * Construct the full request URL for this mock request. + * + * @return the full request URL as a StringBuffer, or `null` if the URL cannot be constructed + */ public StringBuffer getRequestURL() { return null; } + /** + * Retrieve the servlet path portion of the request URL. + * + * @return the servlet path string, or `null` if no servlet path is available + */ public String getServletPath() { return null; } + /** + * Return the HttpSession associated with this request; this mock implementation does not create or store sessions. + * + * @param create if {@code true}, a real implementation would create a new session when none exists; this implementation ignores that flag + * @return {@code null} since this mock request does not maintain sessions + */ public javax.servlet.http.HttpSession getSession(boolean create) { return null; } + /** + * Obtain the current HTTP session associated with this request, if any. + * + * @return the `HttpSession` for the request, or `null` if no session exists + */ public javax.servlet.http.HttpSession getSession() { return null; } + /** + * Rotates the current session identifier and returns the new session id. + * + *

If there is no active session or rotation fails, this method returns `null`.

+ * + * @return the new session identifier, or `null` if no session exists or rotation failed + */ public String changeSessionId() { return null; } + /** + * Indicates whether the requested session ID is still valid. + * + * @return `true` if the requested session ID is valid, `false` otherwise. + */ public boolean isRequestedSessionIdValid() { return false; } + /** + * Indicates whether the requested session ID was provided in a cookie. + * + * @return `true` if the requested session ID came from a cookie, `false` otherwise. + */ public boolean isRequestedSessionIdFromCookie() { return false; } + /** + * Indicates whether the session ID was conveyed via the request URL. + * + * @return `true` if the requested session ID came from the URL, `false` otherwise. + */ public boolean isRequestedSessionIdFromURL() { return false; } + /** + * Indicates whether the requested session ID was conveyed via the request URL. + * + * @return `true` if the session ID was conveyed via the URL, `false` otherwise. + */ public boolean isRequestedSessionIdFromUrl() { return false; } + /** + * Attempts to authenticate the current request using available context. + * + * @param response the HTTP response used to send authentication challenges or headers if needed + * @return `true` if authentication succeeded, `false` otherwise + */ public boolean authenticate(HttpServletResponse response) { return false; } + /** + * Authenticate a user using the provided username and password. + * + * @param username the user's login identifier + * @param password the user's plaintext password + */ public void login(String username, String password) { } + /** + * Invalidate the current user's session and clear related authentication state. + */ public void logout() { } + /** + * Retrieve multipart/form-data parts associated with this request. + * + * @return the collection of `Part` objects for a multipart request, or `null` if multipart handling is not supported or no parts are present. + */ public java.util.Collection getParts() { return null; } + /** + * Retrieves the multipart Part with the given form field name. + * + * @param name the name of the part to retrieve + * @return the Part with the specified name, or {@code null} if no such part exists + */ public javax.servlet.http.Part getPart(String name) { return null; } + /** + * Attempt to upgrade the underlying connection to the specified HttpUpgradeHandler type. + * + * @param the HttpUpgradeHandler subtype + * @param handlerClass the handler class to instantiate for the upgrade + * @return the instantiated handler of type T if the upgrade was performed, or `null` if the request does not support upgrading + */ public T upgrade(Class handlerClass) { return null; } + /** + * Retrieve the value of the attribute identified by the given name. + * + * @param name the attribute name + * @return the attribute value, or `null` if no attribute exists for the given name + */ public Object getAttribute(String name) { return null; } + /** + * Retrieves the names of all attributes stored in this request. + * + * @return an Enumeration of attribute names; empty if no attributes are present + */ public Enumeration getAttributeNames() { return null; } + /** + * Retrieve the character encoding for the request. + * + * @return the name of the character encoding, or `null` if none is specified + */ public String getCharacterEncoding() { return null; } + /** + * Configure the character encoding to use based on the provided encoding name. + * + * @param env the character encoding name to apply (for example, "UTF-8") + */ public void setCharacterEncoding(String env) { } + /** + * Gets the length of the request content in bytes. + * + * @return the content length in bytes, or 0 if unknown or unavailable + */ public int getContentLength() { return 0; } + /** + * Get the request's Content-Length as a long. + * + * @return the Content-Length in bytes, or 0 if not specified + */ public long getContentLengthLong() { return 0; } + /** + * Returns the MIME content type associated with this response. + * + * @return the content type string (e.g. "text/html", "application/json"), or `null` if the content type is unknown + */ public String getContentType() { return null; } + /** + * Obtain the request's input stream for reading the message body. + * + *

This mock implementation does not provide a body stream.

+ * + * @return the ServletInputStream for reading the request body, or `null` for this mock implementation + */ public javax.servlet.ServletInputStream getInputStream() { return null; } + /** + * Retrieve all values for the specified request parameter name. + * + * @param name the parameter name to look up + * @return an array of parameter values, or {@code null} if the parameter does not exist + */ public String[] getParameterValues(String name) { return null; } + /** + * Get the request's parameter map. + * + * @return the map of parameter names to arrays of parameter values; may be empty + */ public Map getParameterMap() { return null; } + /** + * Enumerates the names of parameters present in this request. + * + * @return an {@code Enumeration} containing the parameter names, or an empty {@code Enumeration} if no parameters are present + */ public Enumeration getParameterNames() { return null; } + /** + * Gets the protocol identifier used by this component. + * + * @return the protocol identifier (for example "HTTP/1.1"), or {@code null} if unspecified + */ public String getProtocol() { return null; } + /** + * Return the protocol scheme used for the request (for example, "http" or "https"). + * + * @return the scheme name such as "http" or "https", or {@code null} if the scheme is not set + */ public String getScheme() { return null; } + /** + * Retrieves the configured server name for the API gateway. + * + * @return the server name, or {@code null} if no server name is configured + */ public String getServerName() { return null; } + /** + * Retrieve the configured server port used by the gateway. + * + * @return the configured server port number; `0` if no port is configured or it is unavailable. + */ public int getServerPort() { return 0; } + /** + * Provides a character stream for reading the request body. + * + * @return a {@link java.io.BufferedReader} for reading the request body, or `null` if no body is available + */ public java.io.BufferedReader getReader() { return null; } + /** + * Resolves a resource or virtual path to its underlying filesystem absolute path. + * + * @param path the resource or virtual path to resolve; may be absolute or relative + * @return the absolute filesystem path corresponding to the given {@code path}, or {@code null} if it cannot be resolved + */ public String getRealPath(String path) { return null; } + /** + * Get the client port number for this request. + * + * @return the client port number, or 0 if unavailable + */ public int getRemotePort() { return 0; } + /** + * Get the local host name on which the request was received. + * + * @return the local host name, or {@code null} if not available + */ public String getLocalName() { return null; } + /** + * Gets the local IP address of the server that received the request. + * + * @return the local IP address of the server that received the request, or `null` if unavailable + */ public String getLocalAddr() { return null; } + /** + * Returns the port number on the local machine to which this request was sent. + * + * @return the local port number, or 0 if the port is not available + */ public int getLocalPort() { return 0; } + /** + * Obtain the ServletContext associated with this gateway. + * + * @return the ServletContext for this gateway, or `null` if no context is available + */ public javax.servlet.ServletContext getServletContext() { return null; } + /** + * Indicates that asynchronous request processing is not supported by this mock. + * + * @return the {@link javax.servlet.AsyncContext} if asynchronous processing has been started, or `null` if not supported + */ public javax.servlet.AsyncContext startAsync() { return null; } + /** + * Begins asynchronous processing for the given request and response. + * + * @param servletRequest the request to associate with the asynchronous context + * @param servletResponse the response to associate with the asynchronous context + * @return the created {@link javax.servlet.AsyncContext}, or {@code null} if asynchronous processing could not be started + */ public javax.servlet.AsyncContext startAsync(javax.servlet.ServletRequest servletRequest, javax.servlet.ServletResponse servletResponse) { return null; } + /** + * Indicates whether asynchronous processing has been started. + * + * @return `true` if asynchronous processing has been started, `false` otherwise. + */ public boolean isAsyncStarted() { return false; } + /** + * Indicates whether asynchronous processing is supported for this request. + * + * @return `true` if asynchronous processing is supported, `false` otherwise. + */ public boolean isAsyncSupported() { return false; } + /** + * Get the current asynchronous context for this request. + * + * @return the AsyncContext associated with this request, or null if no asynchronous context is available + */ public javax.servlet.AsyncContext getAsyncContext() { return null; } + /** + * Get the dispatcher type for this request. + * + * @return the {@link javax.servlet.DispatcherType} for the request, or {@code null} if not available + */ public javax.servlet.DispatcherType getDispatcherType() { return null; } + /** + * Sets a request attribute with the given name and value. + * + * @param name the attribute name + * @param o the attribute value + */ public void setAttribute(String name, Object o) { } + /** + * Removes the request attribute with the given name. + * + * If the attribute does not exist, this method has no effect. + * + * @param name the attribute name to remove + */ public void removeAttribute(String name) { } + /** + * Get the Locale configured for the current request or context. + * + * @return the configured Locale, or null if no locale is configured + */ public java.util.Locale getLocale() { return null; } + /** + * Get the preferred locales for the request in order of preference. + * + * @return an {@code Enumeration} containing the client's preferred locales in preference order, + * or {@code null} if no locales are available + */ public Enumeration getLocales() { return null; } + /** + * Indicates whether the API gateway is operating in secure mode. + * + * @return `true` if the gateway is operating in secure mode, `false` otherwise. + */ public boolean isSecure() { return false; } + /** + * Locates a RequestDispatcher for the given request path to allow forwarding or including the target resource. + * + * @param path the context-relative path to the target resource (must begin with a "/") + * @return the RequestDispatcher for the specified path, or `null` if no matching resource can be found + */ public javax.servlet.RequestDispatcher getRequestDispatcher(String path) { return null; } + /** + * Returns the name of the authenticated user associated with this request, or null if none. + * + * @return the remote user's username, or null if the request is not authenticated + */ public String getRemoteUser() { return null; } + /** + * Determines whether the current user has the specified role. + * + * @param role the name of the role to check + * @return {@code true} if the current user has the specified role, {@code false} otherwise + */ public boolean isUserInRole(String role) { return false; } + /** + * Retrieve the authenticated user's security principal, if present. + * + * @return `Principal` representing the authenticated user, or `null` if no user is authenticated. + */ public java.security.Principal getUserPrincipal() { return null; } diff --git a/AuditLogger.java b/AuditLogger.java index 0ede1e4..ac65caa 100644 --- a/AuditLogger.java +++ b/AuditLogger.java @@ -13,13 +13,28 @@ public class AuditLogger { private ConfigService configService; private List logBuffer = new ArrayList<>(); - // 依赖注入问题 + /** + * Creates a new AuditLogger, instantiating its FileProcessor and obtaining the ConfigService singleton. + * + *

The constructor creates a FileProcessor instance for file I/O and calls {@code ConfigService.getInstance()} + * to acquire configuration. This may introduce a circular dependency if ConfigService depends on AuditLogger.

+ */ public AuditLogger() { this.fileProcessor = new FileProcessor(); this.configService = ConfigService.getInstance(); // 可能的循环依赖 } - // 记录UserService操作但暴露敏感信息 + /** + * Records a user operation by appending a log entry that includes the provided operation + * and the result of calling `user.toString()`. If `user.email` is not empty, also records + * the email via {@code logUserEmail}. + * + * Note: the method writes the verbatim `user.toString()` output into the log and therefore + * may include sensitive user fields if `toString()` exposes them. + * + * @param operation a short description of the user operation + * @param user the user whose `toString()` representation will be recorded (included verbatim in the log; may contain sensitive fields) + */ public void logUserOperation(String operation, UserService.User user) { String timestamp = getCurrentTimestamp(); @@ -35,7 +50,15 @@ public void logUserOperation(String operation, UserService.User user) { } } - // 记录DatabaseConnection操作 + /** + * Record a database operation including the executed SQL and its success status into the audit buffer. + * + * The full SQL is recorded verbatim (it may contain sensitive data). If `success` is false, a + * reconnection attempt is made and any reconnection failure is logged via the error logger. + * + * @param sql the executed SQL statement; recorded as-is and may contain sensitive information + * @param success `true` if the database operation succeeded, `false` otherwise + */ public void logDatabaseOperation(String sql, boolean success) { String timestamp = getCurrentTimestamp(); @@ -55,7 +78,15 @@ public void logDatabaseOperation(String sql, boolean success) { } } - // 记录Calculator计算但有精度问题 + /** + * Logs a calculation entry with the given operation and numeric result. + * + * If the result is NaN, an error entry is emitted and no calculation entry is buffered. + * The buffered log entry formats the numeric result to two decimal places. + * + * @param operation a human-readable representation of the calculation (e.g., "a + b") + * @param result the numeric result of the calculation + */ public void logCalculation(String operation, double result) { String timestamp = getCurrentTimestamp(); @@ -74,7 +105,16 @@ public void logCalculation(String operation, double result) { addToBuffer(logEntry); } - // 记录文件操作但重复FileProcessor的问题 + /** + * Records a file-related audit entry for the given file and operation. + * + * Builds a timestamped log entry describing the operation on the specified file and adds it to the internal buffer. + * If the file is considered invalid by the configured FileProcessor, an error is emitted via the logger. + * The method also attempts to read the file to append a line count to the entry; read failures are ignored. + * + * @param fileName the path or identifier of the file being operated on + * @param operation a short description of the file operation performed (e.g., "read", "write", "delete") + */ public void logFileOperation(String fileName, String operation) { String timestamp = getCurrentTimestamp(); @@ -97,7 +137,13 @@ public void logFileOperation(String fileName, String operation) { } } - // 记录并发操作但线程安全问题 + /** + * Records a concurrent operation by timestamping it, incrementing a shared counter, and appending a formatted entry to the internal log buffer. + * + * @param threadName the name or identifier of the thread performing the operation + * @param operation a brief description of the operation performed + * @implNote This method increments a shared ConcurrentTask counter and adds the entry to an in-memory buffer; both actions may exhibit race conditions in concurrent environments. + */ public void logConcurrentOperation(String threadName, String operation) { String timestamp = getCurrentTimestamp(); @@ -111,7 +157,15 @@ public void logConcurrentOperation(String threadName, String operation) { logBuffer.add(logEntry); // ArrayList在并发环境中不安全 } - // 记录Web请求但包含安全问题 + /** + * Logs a web request entry containing the endpoint, user role, and raw request parameters, and flags potentially malicious input when sanitization changes the parameters. + * + * Records the raw `params` value in the audit buffer (may include sensitive data). If a sanitization step modifies `params`, a security event is recorded. + * + * @param endpoint the requested endpoint or path + * @param userRole the role of the user making the request + * @param params the raw request parameters or payload + */ public void logWebRequest(String endpoint, String userRole, String params) { String timestamp = getCurrentTimestamp(); @@ -129,7 +183,16 @@ public void logWebRequest(String endpoint, String userRole, String params) { } } - // 记录数据处理但反射问题 + /** + * Logs the declared field names and values of a data object together with a processing type into the audit buffer. + * + *

The method inspects the object's declared fields (including non-public), captures each field as `name=value` + * pairs, and appends a single formatted entry to the internal log buffer. If an error occurs while collecting + * field values, an error entry is recorded via {@code logError} instead of adding a buffer entry.

+ * + * @param data the object whose declared fields will be recorded; its field names and runtime values are captured + * @param type a brief label describing the kind or category of data processing being logged + */ public void logDataProcessing(Object data, String type) { String timestamp = getCurrentTimestamp(); @@ -157,6 +220,13 @@ public void logDataProcessing(Object data, String type) { } } + /** + * Adds a log entry to the internal buffer and flushes the buffer when the configured size threshold is reached. + * + * Reads the max buffer size from configuration key "log.buffer.max" and uses 1000 if the value is missing or not a valid integer. + * + * @param logEntry the log entry text to append to the buffer + */ private void addToBuffer(String logEntry) { logBuffer.add(logEntry); @@ -175,7 +245,12 @@ private void addToBuffer(String logEntry) { } } - // 刷新缓冲区但文件操作问题 + /** + * Writes all buffered log entries to the configured log file (or "/tmp/audit.log" if none configured) + * and clears the in-memory buffer. + * + *

If writing fails for any reason, the buffer is still cleared, causing buffered log entries to be lost.

+ */ private void flushBuffer() { if (logBuffer.isEmpty()) return; @@ -202,6 +277,15 @@ private void flushBuffer() { } } + /** + * Logs an error entry to standard error including a timestamp and optional exception message. + * + * When an exception is provided, only the exception's message is appended (no stack trace). + * The entry is written directly to System.err and is not added to the internal buffer. + * + * @param message human-readable error message to include in the log entry + * @param e optional exception whose message will be appended if non-null + */ private void logError(String message, Exception e) { String timestamp = getCurrentTimestamp(); String logEntry = String.format("[%s] ERROR: %s", timestamp, message); @@ -215,6 +299,15 @@ private void logError(String message, Exception e) { System.err.println(logEntry); } + /** + * Record a security-related audit entry into the internal buffer. + * + * The entry is timestamped and includes the provided message and details, which are appended verbatim + * (details may contain sensitive information). + * + * @param message short description of the security event + * @param details additional context or data for the event; may include sensitive content + */ private void logSecurity(String message, String details) { String timestamp = getCurrentTimestamp(); String logEntry = String.format("[%s] SECURITY: %s - Details: %s", @@ -223,18 +316,34 @@ private void logSecurity(String message, String details) { addToBuffer(logEntry); } + /** + * Logs a user's email address to the audit buffer without masking or redaction. + * + * @param email the user's email address to record (stored verbatim) + */ private void logUserEmail(String email) { // 记录用户邮箱但没有脱敏 String logEntry = "User email access: " + email; addToBuffer(logEntry); } + /** + * Formats the current date and time using the class's LOG_FORMAT. + * + * @return the current timestamp as a string formatted according to LOG_FORMAT + */ private String getCurrentTimestamp() { SimpleDateFormat sdf = new SimpleDateFormat(LOG_FORMAT); return sdf.format(new Date()); } - // 清理方法但不完整 + /** + * Flushes any buffered log entries to persistent storage. + * + *

Writes all pending log entries from the in-memory buffer to the configured log file. + * This method does not close file handles or release other external resources or internal + * state; callers should perform additional cleanup if required. + */ public void cleanup() { flushBuffer(); // 刷新缓冲区 // 但没有关闭文件资源或清理其他状态 @@ -243,6 +352,13 @@ public void cleanup() { // 单例模式但与其他服务冲突 private static AuditLogger instance; + /** + * Get the shared AuditLogger singleton instance, creating it lazily if necessary. + * + *

Note: this lazy initialization is not thread-safe; concurrent callers may create multiple instances.

+ * + * @return the singleton AuditLogger instance + */ public static AuditLogger getInstance() { if (instance == null) { instance = new AuditLogger(); // 线程不安全 diff --git a/CacheManager.java b/CacheManager.java index 798b001..ff8ace1 100644 --- a/CacheManager.java +++ b/CacheManager.java @@ -14,7 +14,13 @@ public class CacheManager { private DataProcessor dataProcessor; // 可能循环依赖 private AuditLogger auditLogger; - // 单例模式但初始化有循环依赖风险 + /** + * Obtain the singleton CacheManager instance, creating it on first call. + * + * On first invocation this method constructs the singleton; that construction may initialize other singletons (for example DataProcessor or AuditLogger) and can introduce circular dependency risks. + * + * @return the singleton CacheManager instance + */ public static CacheManager getInstance() { if (instance == null) { instance = new CacheManager(); @@ -22,6 +28,11 @@ public static CacheManager getInstance() { return instance; } + /** + * Initializes the singleton CacheManager, acquires required internal services, and preloads cache configuration. + * + *

Obtains internal dependencies and invokes initializeCache() to load initial cache settings.

+ */ private CacheManager() { // 初始化DataProcessor可能导致循环依赖 this.dataProcessor = DataProcessor.getInstance(); // DataProcessor可能也引用CacheManager @@ -29,6 +40,13 @@ private CacheManager() { initializeCache(); } + /** + * Preloads cache-related configuration values from ConfigService into the internal cache. + * + * Reads the "cache.max.size" and "cache.ttl" configuration keys. If "cache.max.size" + * contains a valid integer, stores it in the cache under the key "max_size". + * Parsing errors are ignored; no other validation or storage is performed. + */ private void initializeCache() { // 预加载一些配置到缓存 ConfigService config = ConfigService.getInstance(); @@ -48,7 +66,12 @@ private void initializeCache() { } } - // 缓存获取但线程安全问题 + /** + * Retrieve the cached value for the given key if it exists and has not expired. + * + * @param key the cache key to look up + * @return the cached value associated with `key`, or `null` if no value exists or the entry has expired + */ public Object get(String key) { auditLogger.logDataProcessing(key, "cache_get"); @@ -66,7 +89,20 @@ public Object get(String key) { return value; } - // 缓存设置但竞态条件 + /** + * Stores a value in the cache under the given key, updating its timestamp and enforcing cache size limits. + * + *

If a DataProcessor is available, the value is passed to DataProcessor.processData with caching options and + * the processed result is what gets stored. The method audits the put operation and then invokes size enforcement.

+ * + *

Side effects and notes: + * - May trigger circular calls if DataProcessor interacts with CacheManager. + * - Operations are not synchronized and are not atomic; concurrent access can produce race conditions. + * - Size enforcement may iterate/modifiy collections and can cause a {@code ConcurrentModificationException} in concurrent contexts.

+ * + * @param key the cache key under which the (possibly transformed) value will be stored + * @param value the value to store; if non-null, it may be transformed by a DataProcessor before being cached + */ public void put(String key, Object value) { // 使用DataProcessor处理数据后再缓存，可能循环调用 if (value != null && dataProcessor != null) { @@ -88,6 +124,13 @@ public void put(String key, Object value) { checkCacheSize(); } + /** + * Ensures the in-memory cache does not exceed its configured maximum size by removing entries until the size is within limits. + * + * Uses the cache entry "max_size" as the limit; if absent, a default of 1000 is applied. + * + * @throws ConcurrentModificationException if the underlying cache implementation does not support removal while iterating its key set + */ private void checkCacheSize() { Integer maxSize = (Integer) cache.get("max_size"); if (maxSize == null) @@ -103,7 +146,14 @@ private void checkCacheSize() { } } - // 移除缓存但不同步时间戳 + /** + * Removes the cached entry associated with the given key and records a "cache_remove" audit event. + * + * Note: this method intentionally does not remove the associated timestamp from the internal timestamp map, + * which may cause the timestamp map to retain entries and grow over time. + * + * @param key the cache key to remove + */ public void remove(String key) { cache.remove(key); // 忘记移除时间戳，导致内存泄漏 @@ -112,7 +162,15 @@ public void remove(String key) { auditLogger.logDataProcessing(key, "cache_remove"); } - // 缓存用户数据但引发UserService问题 + /** + * Caches a User object under the key "user_" and records an audit event for the caching operation. + * + * The method stores the provided User in the cache with the key formed by prefixing the userId with "user_", + * and then logs a "cache" user operation via the audit logger including the User object. + * + * @param userId the identifier of the user used to form the cache key + * @param user the User object to cache + */ public void cacheUser(String userId, UserService.User user) { // 缓存用户对象，但User.toString包含密码 String cacheKey = "user_" + userId; @@ -122,6 +180,12 @@ public void cacheUser(String userId, UserService.User user) { auditLogger.logUserOperation("cache", user); } + /** + * Retrieve a cached User by ID, loading and caching it from UserManager on a cache miss. + * + * @param userId the user identifier as a decimal string; non-numeric values will not be resolved + * @return the cached or newly loaded {@code UserService.User} if found, {@code null} if not found or if {@code userId} is invalid + */ public UserService.User getCachedUser(String userId) { String cacheKey = "user_" + userId; Object cached = get(cacheKey); @@ -147,7 +211,18 @@ public UserService.User getCachedUser(String userId) { } } - // 缓存计算结果但使用Calculator的有问题方法 + /** + * Compute or retrieve a cached numeric result for a named operation using two operands. + * + *

If a cached Double exists for the operation and operands, that value is returned. + * Otherwise the method computes the result for supported operations ("divide" and "power"), + * caches the computed result unless it is `NaN`, and returns the result.

+ * + * @param operation the operation name to perform; supported values: "divide" and "power" + * @param a the first operand + * @param b the second operand (for "power" the value is cast to an `int`) + * @return the computed or cached result for the operation and operands; returns `0` for unsupported operations + */ public double getCachedCalculation(String operation, double a, double b) { String cacheKey = String.format("calc_%s_%.2f_%.2f", operation, a, b); Object cached = get(cacheKey); @@ -179,7 +254,12 @@ public double getCachedCalculation(String operation, double a, double b) { return result; } - // 缓存文件内容但重复FileProcessor问题 + /** + * Retrieve cached file content for the given file name, loading and caching it if absent. + * + * @param fileName the file name or path to read and cache + * @return the file content as a `String`, or `null` if the file could not be read + */ public String getCachedFileContent(String fileName) { String cacheKey = "file_" + fileName; Object cached = get(cacheKey); @@ -199,7 +279,12 @@ public String getCachedFileContent(String fileName) { return content; } - // 清理过期缓存但线程安全问题 + /** + * Removes all cache entries whose stored timestamps indicate they have expired. + * + *

This method is synchronized and iterates over a snapshot of the timestamp map to identify + * expired keys, then removes each expired entry from the cache.

+ */ public synchronized void cleanupExpiredEntries() { // 创建要删除的键列表，但在并发环境中不安全 List keysToRemove = new ArrayList<>(); @@ -216,12 +301,24 @@ public synchronized void cleanupExpiredEntries() { } } + /** + * Checks whether the given timestamp is older than the cache time-to-live (5 minutes). + * + * @param timestamp epoch time in milliseconds representing when the entry was stored + * @return `true` if the timestamp is more than 5 minutes older than the current time, `false` otherwise + */ private boolean isExpired(long timestamp) { long ttl = 300000; // 5分钟，硬编码 return System.currentTimeMillis() - timestamp > ttl; } - // 缓存统计但计算有问题 + /** + * Produce simple cache statistics including total entries, expired entries, and hit rate. + * + * The returned hit rate is computed as (totalSize - expiredCount) / totalSize using the Calculator utility. + * + * @return a CacheStats instance containing: totalSize (number of entries currently in the cache), expiredCount (entries whose timestamps are considered expired), and hitRate (ratio of non-expired entries to total entries as computed by the Calculator) + */ public CacheStats getStatistics() { int totalSize = cache.size(); int expiredCount = 0; @@ -240,7 +337,13 @@ public CacheStats getStatistics() { return new CacheStats(totalSize, expiredCount, hitRate); } - // 序列化缓存但可能包含敏感数据 + /** + * Serializes current cache entries to the specified file. + * + * Writes each cache entry as a `key=value` line to the provided file name. The output includes all cached values and may contain sensitive data; callers should ensure the destination is secure. Failures during writing are handled internally and do not propagate. + * + * @param fileName the path to the file where the serialized cache will be written + */ public void serializeCache(String fileName) { FileProcessor processor = new FileProcessor(); @@ -264,25 +367,51 @@ public static class CacheStats { private int expiredCount; private double hitRate; + /** + * Creates a CacheStats instance with the specified totals and hit rate. + * + * @param totalSize total number of entries in the cache + * @param expiredCount number of entries that are expired + * @param hitRate proportion of non-expired entries to total entries (0.0–1.0) + */ public CacheStats(int totalSize, int expiredCount, double hitRate) { this.totalSize = totalSize; this.expiredCount = expiredCount; this.hitRate = hitRate; } - // getter方法 + /** + * Gets the total number of entries currently stored in the cache. + * + * @return the number of entries in the cache + */ public int getTotalSize() { return totalSize; } + /** + * Number of expired entries in the cache. + * + * @return the number of cache entries that are expired + */ public int getExpiredCount() { return expiredCount; } + /** + * The cache hit rate expressed as the fraction of non-expired entries relative to total entries. + * + * @return the cache hit rate as a value between 0.0 and 1.0 + */ public double getHitRate() { return hitRate; } + /** + * Provides a concise string representation of the CacheStats including total size, expired count, and hit rate. + * + * @return a string formatted as "CacheStats{size=, expired=, hitRate=}" with the hit rate shown to two decimal places + */ @Override public String toString() { return String.format("CacheStats{size=%d, expired=%d, hitRate=%.2f}", @@ -290,7 +419,11 @@ public String toString() { } } - // 危险的清空方法 + /** + * Removes all entries from the cache and their associated timestamps. + * + * This method clears both the in-memory cache and the cacheTimestamps map but does not notify other components or services that the cache has been cleared. + */ public void clearAll() { cache.clear(); cacheTimestamps.clear(); diff --git a/ConfigService.java b/ConfigService.java index d8af56d..9c6facf 100644 --- a/ConfigService.java +++ b/ConfigService.java @@ -13,7 +13,11 @@ public class ConfigService { private DatabaseConnection dbConnection; // 依赖DatabaseConnection private String configFilePath = "/config/app.properties"; // 硬编码路径 - // 单例模式与DatabaseConnection冲突 + /** + * Get the singleton ConfigService instance, creating it if one does not already exist. + * + * @return the shared ConfigService instance + */ public static ConfigService getInstance() { if (instance == null) { instance = new ConfigService(); @@ -21,12 +25,20 @@ public static ConfigService getInstance() { return instance; } + /** + * Initializes the ConfigService by loading configuration from storage and establishing the database connection. + */ private ConfigService() { loadConfiguration(); initializeDatabaseConnection(); } - // 与DatabaseConnection的重复代码 + /** + * Loads configuration properties from the file specified by the service's + * configFilePath into the in-memory Properties and replaces current values. + * If any error occurs while reading the file, the method falls back to the + * default configuration by calling setDefaultConfiguration(). + */ private void loadConfiguration() { try { FileInputStream fis = new FileInputStream(configFilePath); @@ -38,6 +50,14 @@ private void loadConfiguration() { } } + /** + * Populate the in-memory configuration with built-in defaults for database connection. + * + * Sets the following properties on the internal Properties object: + * - "db.url" -> "jdbc:mysql://localhost:3306/app" + * - "db.username" -> "admin" + * - "db.password" -> "admin123" + */ private void setDefaultConfiguration() { // 与DatabaseConnection中的硬编码冲突 config.setProperty("db.url", "jdbc:mysql://localhost:3306/app"); // 不同的数据库名 @@ -45,7 +65,13 @@ private void setDefaultConfiguration() { config.setProperty("db.password", "admin123"); // 与UserService的adminPassword相同 } - // 错误地创建DatabaseConnection实例 + /** + * Initializes the DatabaseConnection instance and attempts to load its configuration. + * + *

Sets the class's {@code dbConnection} field and calls the connection's configuration loader, + * which may overwrite in-memory configuration properties. Any exceptions thrown while creating + * or loading the database connection are suppressed. + */ private void initializeDatabaseConnection() { try { dbConnection = new DatabaseConnection(); @@ -56,14 +82,23 @@ private void initializeDatabaseConnection() { } } - // 与UserService的密码处理冲突 + /** + * Retrieve the configured admin password from this service's properties. + * + * @return the value of `admin.password` from the configuration, or "defaultPass" if the property is not set + */ public String getAdminPassword() { String password = config.getProperty("admin.password", "defaultPass"); // 与UserService.adminPassword不同步 return password; } - // 依赖StringUtils但使用方式有问题 + /** + * Retrieve the configuration value for the given key, returning a sanitized string or null if the key is missing or empty. + * + * @param key the configuration property name to look up + * @return the configuration value for the key after basic sanitization, or null if the property is not present or empty + */ public String getConfigValue(String key) { String value = config.getProperty(key); @@ -75,7 +110,14 @@ public String getConfigValue(String key) { return StringUtils.sanitizeInput(value); // 可能过度清理配置值 } - // 与Calculator的集成问题 + /** + * Retrieve a numeric configuration value for the given property key. + * + * Parses the configuration value as a double. If the parsed number is considered equal to 0.0, this method returns 1.0. If the value is missing or cannot be parsed as a number, this method returns 0.0. + * + * @param key the configuration property name to read + * @return the parsed numeric value; `1.0` if the parsed value is equal to 0.0; `0.0` if parsing fails or the value is missing + */ public double getNumericConfig(String key) { String value = getConfigValue(key); try { @@ -93,7 +135,12 @@ public double getNumericConfig(String key) { } } - // 与FileProcessor的重复逻辑 + /** + * Persist the current configuration properties to the configured file path. + * + * Writes each property as a `key=value` line to the service's configured file location. + * Any exception raised while writing is caught and suppressed (no exception is propagated). + */ public void saveConfiguration() { FileProcessor processor = new FileProcessor(); @@ -110,7 +157,16 @@ public void saveConfiguration() { } } - // 与RestController的安全问题 + /** + * Update a configuration property and persist the change to the configured storage. + * + * Updates the in-memory property identified by {@code key} to {@code value}, persists the current + * configuration to the configured file, and may print sensitive keys and values to standard output. + * This method does not validate input and performs a full save on every invocation. + * + * @param key the configuration key to update + * @param value the new value for the configuration key + */ public void updateConfig(String key, String value) { // 没有验证输入，类似RestController的问题 if (key.contains("password") || key.contains("secret")) { @@ -122,14 +178,27 @@ public void updateConfig(String key, String value) { saveConfiguration(); // 每次更新都保存，性能问题 } - // 与ConcurrentTask的线程安全问题 + /** + * Reloads in-memory configuration from the configured file source. + * + * This method clears the current Properties and then loads configuration from persistent storage. + * The operation is not atomic: other threads may observe an empty or partially reloaded configuration + * while this method executes. + */ public void refreshConfiguration() { // 在多线程环境中重新加载配置 config.clear(); // 不是原子操作 loadConfiguration(); // 可能导致其他线程读到空配置 } - // 与DataProcessor的反射问题 + /** + * Creates an instance of the specified class and populates its declared fields + * with configuration values whose keys are formed as "config.". + * + * @param className the fully-qualified name of the class to instantiate + * @return the newly created instance with string-valued fields set from configuration + * @throws RuntimeException if the class cannot be loaded, instantiated, or if a field cannot be set + */ public Object getConfigAsObject(String className) { try { Class clazz = Class.forName(className); @@ -159,7 +228,11 @@ public Object getConfigAsObject(String className) { instance = new ConfigService(); } - // 与UserManager的集成问题 + /** + * Attempts to configure a new UserManager's maximum users from the "max.users" configuration. + * + * If the configuration value cannot be parsed as an integer, the method ignores the error and leaves the UserManager unmodified. + */ public void configureUserManager() { UserManager userManager = new UserManager(); @@ -178,17 +251,35 @@ public void configureUserManager() { // 内存泄漏 - 配置历史记录 private static List configHistory = new ArrayList<>(); + /** + * Saves a copy of the current configuration into the in-memory history. + * + * The method creates a new Properties instance containing the current config entries + * and appends it to the static configHistory list; entries already in configHistory + * are not removed or pruned, so the history can grow without bound. + */ public void backupCurrentConfig() { Properties backup = new Properties(); backup.putAll(config); configHistory.add(backup); // 历史记录永远不清理 } - // 不一致的API设计 + /** + * Checks whether the configuration contains the specified property key. + * + * @param key the property key to check in the configuration + * @return `true` if the configuration contains the key, `false` otherwise + */ public boolean hasConfig(String key) { return config.containsKey(key); } + /** + * Checks whether a configuration property with the given key is present. + * + * @param key the configuration key to check + * @return `true` if a property with the given key exists (its value may be empty), `false` otherwise + */ public boolean configExists(String key) { // 相同功能，不同方法名 return config.getProperty(key) != null; } diff --git a/DataValidator.java b/DataValidator.java index 362f4d1..558e9b7 100644 --- a/DataValidator.java +++ b/DataValidator.java @@ -16,13 +16,25 @@ public class DataValidator { private Calculator calculator; private AuditLogger auditLogger; + /** + * Initializes a DataValidator instance by creating its Calculator and obtaining the AuditLogger singleton. + * + * Initializes the `calculator` and `auditLogger` fields. The `emailPattern` field is intentionally left uninitialized. + */ public DataValidator() { this.calculator = new Calculator(); this.auditLogger = AuditLogger.getInstance(); // emailPattern没有初始化！ } - // 与StringUtils.isValidEmail重复但实现不一致 + /** + * Validates an email string for presence, basic format, and suspicious characters. + * + * Performs three checks: rejects null or empty input, verifies the email matches the class's EMAIL_REGEX, and rejects input where sanitization changes the value (treated as suspicious). + * + * @param email the email string to validate + * @return {@code true} if the email is non-empty, matches EMAIL_REGEX, and remains unchanged by sanitization; {@code false} otherwise + */ public boolean validateEmail(String email) { // 调用StringUtils的有问题方法 if (StringUtils.isEmpty(email)) { // 空指针风险 @@ -44,7 +56,16 @@ public boolean validateEmail(String email) { return isValid; } - // 数值验证但使用Calculator的有问题方法 + /** + * Validate that a numeric value falls within the inclusive [min, max] range. + * + * Logs a calculation event and treats NaN or values that would cause a division-by-zero risk as invalid; may log an error when a division-by-zero risk is detected. + * + * @param value the numeric value to validate + * @param min the lower bound (inclusive) + * @param max the upper bound (inclusive) + * @return `true` if value is between min and max inclusive, `false` otherwise (also `false` for NaN or values that present a division-by-zero risk) + */ public boolean validateNumericRange(double value, double min, double max) { auditLogger.logCalculation("Range validation", value); @@ -62,7 +83,15 @@ public boolean validateNumericRange(double value, double min, double max) { return value >= min && value <= max; } - // 密码验证与多个类的冲突 + /** + * Validate a password against configured and heuristic rules. + * + * Performs a series of checks (non-empty, not the default admin password, minimum length of 8, + * and a computed strength check) and accumulates any failures. + * + * @param password the password to validate + * @return a ValidationResult containing any validation errors; {@code isValid()} is {@code true} when no errors were recorded + */ public ValidationResult validatePassword(String password) { ValidationResult result = new ValidationResult(); @@ -95,6 +124,12 @@ public ValidationResult validatePassword(String password) { return result; } + /** + * Computes a password strength score that increases with password length and character-type variety. + * + * @param password the password to evaluate + * @return a numeric strength score; higher values indicate stronger passwords (larger values reflect greater length and character variety) + */ private double calculatePasswordStrength(String password) { // 使用Calculator但可能有数值问题 double length = password.length(); @@ -104,6 +139,12 @@ private double calculatePasswordStrength(String password) { return calculator.divide(length * variety, 10.0); } + /** + * Count distinct character categories present in the given password. + * + * @param password the password to inspect + * @return the number of character categories found: uppercase, lowercase, digits, and special characters (0–4) + */ private double getCharacterVariety(String password) { // 与StringUtils.isValidPassword类似逻辑但计算不同 int types = 0; @@ -119,7 +160,14 @@ private double getCharacterVariety(String password) { return types; } - // 文件验证依赖FileProcessor但加重其问题 + /** + * Validates a filesystem path for basic acceptance and obvious traversal risks. + * + * Performs existence/format validation and input sanitization checks and records audit or security events when validation fails. + * + * @param path the filesystem path to validate + * @return `true` if the path passes validation checks, `false` otherwise + */ public boolean validateFilePath(String path) { FileProcessor processor = new FileProcessor(); @@ -141,7 +189,15 @@ public boolean validateFilePath(String path) { return true; } - // 用户数据验证，依赖UserService和UserManager + /** + * Validate a user's required fields and credentials, and check whether the email is already in use. + * + *

Performs presence checks for the user's name and email, validates the email format, validates the + * password, and records an error if an existing account with the same email is found.

+ * + * @param user the user to validate; may be null + * @return a ValidationResult containing any validation errors and the overall validity state + */ public ValidationResult validateUser(UserService.User user) { ValidationResult result = new ValidationResult(); @@ -178,7 +234,13 @@ public ValidationResult validateUser(UserService.User user) { return result; } - // 数据库连接配置验证 + /** + * Validates that required database configuration values are present and verifies a connection can be obtained. + * + * Logs a failed database operation when the connection attempt throws an exception. + * + * @return `true` if the required DB configuration (URL and username) is present and a connection could be obtained, `false` otherwise. + */ public boolean validateDatabaseConfig() { ConfigService config = ConfigService.getInstance(); @@ -201,7 +263,14 @@ public boolean validateDatabaseConfig() { } } - // 并发验证但线程安全问题 + /** + * Validates a list of data items using shared concurrent helpers. + * + * Iterates over each item, increments a shared ConcurrentTask counter, records a processing audit, and delegates per-item work to DataProcessor. + * This method mutates shared singletons and is not thread-safe; calling it concurrently may produce race conditions, inconsistent shared state, or data corruption. + * + * @param dataList the collection of data items to validate and process + */ public void validateConcurrentData(List dataList) { ConcurrentTask task = ConcurrentTask.getInstance(); @@ -223,11 +292,21 @@ public static class ValidationResult { private List errors = new ArrayList<>(); private boolean valid = true; + /** + * Records a validation error and marks this result as invalid. + * + * @param error the error message to add + */ public void addError(String error) { errors.add(error); valid = false; } + /** + * Merges the given ValidationResult into this instance by appending its errors and combining validity. + * + * @param other the ValidationResult whose errors and validity will be merged into this one; if `null`, no changes are made + */ public void mergeWith(ValidationResult other) { if (other != null) { errors.addAll(other.errors); @@ -235,22 +314,47 @@ public void mergeWith(ValidationResult other) { } } + /** + * Indicates whether this ValidationResult represents a successful validation (no recorded errors). + * + * @return `true` if the result has no errors and is considered valid, `false` otherwise. + */ public boolean isValid() { return valid; } + /** + * Gets the list of error messages for this ValidationResult. + * + * @return the internal List of error messages; modifications to the returned list will affect this ValidationResult + */ public List getErrors() { return errors; // 返回内部集合引用 } - // toString可能暴露敏感验证信息 + /** + * Returns a string representation of this ValidationResult. + * + * @return a string containing the `errors` list and the `valid` flag (note: the returned text may include sensitive validation details) + */ @Override public String toString() { return "ValidationResult{errors=" + errors + ", valid=" + valid + "}"; } } - // 批量验证但性能问题 + /** + * Validate a list of heterogeneous objects and collect a ValidationResult for each entry. + * + * Supported input types: + * - String: treated as an email and validated accordingly. + * - UserService.User: validated via validateUser. + * - Double: validated as a numeric value in the range [0, 1000]. + * Unsupported types produce a ValidationResult containing an "Unsupported object type" error. + * + * @param objects the list of objects to validate + * @return a map from each input object to its corresponding ValidationResult (errors and overall validity) + */ public Map validateBatch(List objects) { Map results = new HashMap<>(); diff --git a/UserManager.java b/UserManager.java index 904b271..fde4cd8 100644 --- a/UserManager.java +++ b/UserManager.java @@ -12,19 +12,36 @@ public class UserManager { private DatabaseConnection dbConn; private List cachedUsers = new ArrayList<>(); // 依赖内部类 - // 构造器问题 - 没有初始化依赖 + /** + * Initializes a UserManager instance by creating its DatabaseConnection while leaving service dependencies unset. + * + *

This constructor instantiates the internal {@code dbConn}. The {@code userService} dependency is not initialized + * by this constructor and will remain {@code null} until set by callers; callers must provide or inject a valid + * UserService before invoking methods that depend on it.

+ */ public UserManager() { // userService没有初始化！ this.dbConn = new DatabaseConnection(); } - // API不一致 - UserService用String，这里用int + /** + * Retrieves a user for the given numeric id. + * + * @param userId the numeric user identifier + * @return the matching UserService.User, or `null` if no user is found + */ public UserService.User getUser(int userId) { // 类型转换问题 return userService.getUserById(String.valueOf(userId)); } - // 依赖UserService的方法但参数不匹配 + /** + * Attempts to create a new user with the given name and email. + * + * @param name the user's display name + * @param email the user's email address + * @return `true` if the user was created successfully, `false` if an exception occurred during creation + */ public boolean createNewUser(String name, String email) { // UserService.createUser需要5个参数，这里只传2个 try { @@ -35,7 +52,12 @@ public boolean createNewUser(String name, String email) { } } - // 与StringUtils的依赖问题 + /** + * Searches the internal user cache for entries whose name or email contains the provided query. + * + * @param query the search string; if null or empty, the internal cached user list is returned + * @return the internal cached user list when {@code query} is null or empty; otherwise a new list containing users whose name or email contains the sanitized query (case-sensitive) + */ public List searchUsers(String query) { if (StringUtils.isEmpty(query)) { // 调用有null风险的方法 return cachedUsers; @@ -54,7 +76,11 @@ public List searchUsers(String query) { return results; } - // 使用Calculator的有问题方法 + /** + * Computes a user's score based on their login count. + * + * @returns the computed score as a double; may be Infinity or NaN if the underlying login count is zero or otherwise produces an invalid divisor/result. + */ public double calculateUserScore(int userId) { Calculator calc = new Calculator(); calc.incrementCounter(); // 访问非静态方法但Calculator没有这个方法 @@ -65,11 +91,23 @@ public double calculateUserScore(int userId) { return calc.power(base, 2); // 没有处理负数情况 } + /** + * Retrieve the recorded login count for a user. + * + * @return the user's login count; this implementation currently always returns 0 + */ private int getUserLoginCount(int userId) { return 0; // 总是返回0，导致除零 } - // 与FileProcessor的问题依赖 + /** + * Exports the cached users to the specified file, writing each user's string representation on its own line. + * + *

Note: this method writes the result of User.toString() (which may include sensitive fields such as passwords) + * and ignores any I/O errors that occur during writing.

+ * + * @param fileName the path of the file to write the exported user data to + */ public void exportUsers(String fileName) { FileProcessor processor = new FileProcessor(); @@ -86,7 +124,16 @@ public void exportUsers(String fileName) { } } - // 与ConcurrentTask的不当使用 + /** + * Processes the internal cached users by interacting with a shared ConcurrentTask and mutating the cache. + * + *

For each user in {@code cachedUsers} this method increments a counter on a singleton {@code ConcurrentTask} + * and adds the same user back into {@code cachedUsers}, thereby mutating the internal cache.

+ * + *

Side effects: increments a shared task counter and appends users to the internal {@code cachedUsers} list. + * This method is not thread-safe; concurrent use may cause race conditions, duplicate entries, and + * {@link java.util.ConcurrentModificationException} or other undefined behavior.

+ */ public void processUsersAsync() { ConcurrentTask task = ConcurrentTask.getInstance(); // 单例但线程不安全 @@ -98,7 +145,11 @@ public void processUsersAsync() { } } - // 循环依赖暗示 + /** + * Processes every user in the internal cache by sending each user to the data processor with a "user" type option. + * + *

For each cached user this method invokes the data processor with the user object, the literal type string "user", and an options map containing "type"="user". The method performs processing for side effects and does not return a value.

+ */ public void processUserData() { DataProcessor processor = DataProcessor.getInstance(); // 可能循环依赖 @@ -111,7 +162,12 @@ public void processUserData() { } } - // 内存泄漏 - 缓存没有清理 + /** + * Populates the internal cache by fetching users for ids 0 through 9999 and appending any non-null results. + * + * This method converts each index in the range [0, 9999] to a string, calls the user service to retrieve a user, + * and adds non-null users to the internal cachedUsers list without clearing or deduplicating existing entries. + */ public void cacheAllUsers() { // 不断添加用户但从不清理 for (int i = 0; i < 10000; i++) { @@ -122,7 +178,14 @@ public void cacheAllUsers() { } } - // 不一致的异常处理 + /** + * Finds and returns the user corresponding to the given email string. + * + * @param email the email to look up; must not be null + * @return the UserService.User matching the provided email + * @throws IllegalArgumentException if {@code email} is null + * @throws Exception if the underlying user service fails to retrieve the user + */ public UserService.User findUserByEmail(String email) throws Exception { if (email == null) { throw new IllegalArgumentException("Email cannot be null"); @@ -132,12 +195,24 @@ public UserService.User findUserByEmail(String email) throws Exception { return userService.getUserById(email); // 错误：用email作为userId } - // 违反封装 - 直接暴露内部集合 + /** + * Provide direct access to the manager's internal cached list of users. + * + * @return the internal mutable list of cached users; modifications to the returned list will affect the manager's cache + */ public List getAllUsers() { return cachedUsers; // 返回内部集合的直接引用 } - // 不正确的equals/hashCode依赖 + /** + * Determines whether two UserService.User references refer to the same object instance. + * + * This performs reference (identity) comparison and does not compare user fields for logical equality. + * + * @param user1 the first user reference to compare + * @param user2 the second user reference to compare + * @return `true` if both references refer to the same object, `false` otherwise + */ public boolean isDuplicateUser(UserService.User user1, UserService.User user2) { // UserService.User没有重写equals，这里是引用比较 return user1.equals(user2);