11421 should be able to add all missing fields and missing tables in one go #11426
Conversation
WalkthroughThe pull request introduces functionality enhancements in the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as DataAdministrationController
participant I as ItemFacade
U->>C: Trigger createTablesAndFieldsForAllCreateStatements()
C->>C: Parse allCreateStetements and split SQL statements
C->>C: Generate ALTER commands based on table names
C->>I: Execute ALTER statement(s)
I-->>C: Return execution result or error
C->>U: Display executionFeedback
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/main/webapp/resources/ezcomp/midding_data_fields.xhtml (2)
21-23: Fix typo in tab title.There is a spelling error in the tab title which should be corrected for better user experience.
- title="Add All Tables using create ddl fiel contents" > + title="Add All Tables using create ddl file contents" >
30-35: Inconsistent label naming.The label "All Create Statements in ddl file" is more descriptive than the header "Run SQL Commands". Consider aligning them for better user clarity.
- <h:outputLabel value="Run SQL Commands" class="mx-2"/> + <h:outputLabel value="All Create Statements" class="mx-2"/>src/main/java/com/divudi/bean/common/DataAdministrationController.java (4)
280-280: Fix spelling in variable name.The variable name contains a spelling error that should be corrected for consistency.
- private String allCreateStetements; + private String allCreateStatements;This spelling error appears throughout the code and should be fixed in all occurrences.
874-934: Implementation for batch processing of CREATE statements.The new method successfully implements the ability to process multiple CREATE statements at once, as required. However, there are several areas for improvement:
- The method is quite long and could be refactored into smaller, more focused methods
- Error handling could be improved with more specific exception handling
- SQL statement execution could benefit from transaction management
Consider refactoring this large method into smaller, more focused methods:
public void createTablesAndFieldsForAllCreateStatements() { StringBuilder executionResults = new StringBuilder(); System.out.println("===== Starting CREATE TABLE parsing and ALTER execution ====="); + String[] createStatements = splitCreateStatements(allCreateStetements); + processCreateStatements(createStatements, executionResults); - // Split by CREATE TABLE, keeping it in the output - String[] rawParts = allCreateStetements.split("(?i)CREATE TABLE"); - int counter = 0; - - for (String part : rawParts) { - part = part.trim(); - if (part.isEmpty()) { - continue; - } - - String createStatement = "CREATE TABLE " + part; - System.out.println("\n[INFO] Processing statement #" + (++counter)); - System.out.println("Create SQL:\n" + createStatement); - - try { - String tableName = extractTableName(createStatement); - if (tableName == null || tableName.isEmpty()) { - System.out.println("[WARNING] Skipping statement — table name not found."); - executionResults.append("<br/>Skipped malformed CREATE TABLE statement."); - continue; - } - - System.out.println("[INFO] Extracted table name: " + tableName); - - String alterSql = generateAlterStatements(createStatement); - System.out.println("[INFO] Generated ALTER statements:\n" + alterSql); - - String[] sqlStatements = alterSql.split(";"); - for (String sql : sqlStatements) { - sql = sql.trim(); - if (sql.isEmpty()) { - continue; - } - - try { - System.out.println("[EXEC] Running SQL: " + sql); - itemFacade.executeNativeSql(sql); - executionResults.append("<br/>Successfully executed: ").append(sql); - } catch (Exception e) { - System.out.println("[ERROR] SQL failed: " + sql); - System.out.println("[ERROR] Exception: " + e.getMessage()); - executionResults.append("<br/>Failed to execute: ").append(sql); - executionResults.append("<br/>Error: ").append(e.getMessage()); - } - } - } catch (Exception e) { - System.out.println("[ERROR] Unexpected error while processing create statement."); - e.printStackTrace(); - executionResults.append("<br/>Error processing create statement: ").append(e.getMessage()); - } - } executionFeedback = executionResults.toString(); System.out.println("===== All CREATE TABLE processing complete ====="); } + private String[] splitCreateStatements(String allStatements) { + // Split by CREATE TABLE, keeping it in the output + return allStatements.split("(?i)CREATE TABLE"); + } + + private void processCreateStatements(String[] rawParts, StringBuilder executionResults) { + int counter = 0; + + for (String part : rawParts) { + part = part.trim(); + if (part.isEmpty()) { + continue; + } + + String createStatement = "CREATE TABLE " + part; + System.out.println("\n[INFO] Processing statement #" + (++counter)); + + try { + processCreateStatement(createStatement, executionResults); + } catch (Exception e) { + System.out.println("[ERROR] Unexpected error while processing create statement."); + e.printStackTrace(); + executionResults.append("<br/>Error processing create statement: ").append(e.getMessage()); + } + } + } + + private void processCreateStatement(String createStatement, StringBuilder executionResults) { + String tableName = extractTableName(createStatement); + if (tableName == null || tableName.isEmpty()) { + System.out.println("[WARNING] Skipping statement — table name not found."); + executionResults.append("<br/>Skipped malformed CREATE TABLE statement."); + return; + } + + System.out.println("[INFO] Extracted table name: " + tableName); + + String alterSql = generateAlterStatements(createStatement); + executeAlterStatements(alterSql, executionResults); + } + + private void executeAlterStatements(String alterSql, StringBuilder executionResults) { + String[] sqlStatements = alterSql.split(";"); + for (String sql : sqlStatements) { + sql = sql.trim(); + if (sql.isEmpty()) { + continue; + } + + try { + System.out.println("[EXEC] Running SQL: " + sql); + itemFacade.executeNativeSql(sql); + executionResults.append("<br/>Successfully executed: ").append(sql); + } catch (Exception e) { + System.out.println("[ERROR] SQL failed: " + sql); + System.out.println("[ERROR] Exception: " + e.getMessage()); + executionResults.append("<br/>Failed to execute: ").append(sql); + executionResults.append("<br/>Error: ").append(e.getMessage()); + } + } + }
880-880: Use a regex pattern for better CREATE TABLE splitting.The current splitting approach has some limitations that could cause issues with certain SQL statements.
Consider using a more robust pattern matching approach:
- // Split by CREATE TABLE, keeping it in the output - String[] rawParts = allCreateStetements.split("(?i)CREATE TABLE"); + // Use a pattern that better handles SQL syntax + Pattern createTablePattern = Pattern.compile("(?i)(CREATE\\s+TABLE\\s+[^;]+;)"); + Matcher matcher = createTablePattern.matcher(allCreateStetements); + List<String> createStatements = new ArrayList<>(); + + while (matcher.find()) { + createStatements.add(matcher.group(1)); + }
894-901: Improve table name extraction validation.The current table name extraction could be more robust to handle edge cases.
Consider using a more precise regex pattern:
- String tableName = extractTableName(createStatement); - if (tableName == null || tableName.isEmpty()) { - System.out.println("[WARNING] Skipping statement — table name not found."); - executionResults.append("<br/>Skipped malformed CREATE TABLE statement."); - continue; - } + String tableName; + try { + tableName = extractTableName(createStatement); + if (tableName == null || tableName.isEmpty()) { + throw new IllegalArgumentException("Table name not found"); + } + } catch (Exception e) { + System.out.println("[WARNING] Skipping statement — " + e.getMessage()); + executionResults.append("<br/>Skipped malformed CREATE TABLE statement: " + e.getMessage()); + continue; + }Also update the
extractTableNamemethod to be more robust:private String extractTableName(String sql) { sql = sql.trim(); String upperSql = sql.toUpperCase(); - int createIndex = upperSql.indexOf("CREATE TABLE"); - int startIndex = createIndex + "CREATE TABLE".length(); - // Skip any whitespace after 'CREATE TABLE' - while (Character.isWhitespace(sql.charAt(startIndex))) { - startIndex++; - } - int endIndex = sql.indexOf("(", startIndex); - String tableName = sql.substring(startIndex, endIndex).trim(); - // Remove backticks or quotes if present - tableName = tableName.replaceAll("[`\"']", ""); + // Use regex to extract table name + Pattern pattern = Pattern.compile("(?i)CREATE\\s+TABLE\\s+[`\"']?([^`\"'(\\s]+)[`\"']?\\s*\\("); + Matcher matcher = pattern.matcher(sql); + if (matcher.find()) { + return matcher.group(1); + } + throw new IllegalArgumentException("Could not parse table name from SQL: " + sql); - return tableName; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/divudi/bean/common/DataAdministrationController.java(5 hunks)src/main/webapp/resources/ezcomp/midding_data_fields.xhtml(1 hunks)
🔇 Additional comments (8)
src/main/webapp/resources/ezcomp/midding_data_fields.xhtml (5)
17-19: Accordion panel initialized with controller state variable.The accordion panel is correctly configured with the active index bound to a controller variable, allowing state preservation across requests.
37-40: SQL execution button correctly configured.The command button is properly set up with ajax="false" to ensure complete page reload after the operation, which is important for database operations.
52-112: Second tab correctly migrated to accordion format.The original functionality for handling individual create statements has been properly migrated to the new accordion panel structure with all the necessary inputs and commands preserved.
114-146: Third tab provides simplified SQL execution.The third tab provides a streamlined interface for running any SQL commands. This enhances usability by separating generic SQL execution from the more specific table and field creation operations.
17-148: Good UI organization with accordion panel.The restructuring of the UI into three distinct tabs using an accordion panel significantly improves the user experience by logically grouping related functionality and reducing visual clutter.
src/main/java/com/divudi/bean/common/DataAdministrationController.java (3)
292-292: Added tab index variable for state management.The new
tabIndexMissingFieldsvariable properly supports the accordion panel state management in the UI.
651-656: Initialization of state variables in navigation method.The navigation method now properly initializes all required variables including the new ones, ensuring a clean state when entering the screen.
2340-2354: Added getter and setter methods for new variables.The getter and setter methods for the new class variables are properly implemented following JavaBean conventions.
src/main/java/com/divudi/bean/common/DataAdministrationController.java
Outdated
Show resolved
Hide resolved
…ler.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/com/divudi/bean/common/DataAdministrationController.java (3)
650-657: Initialization of fields when navigating to missing fields page.The method now initializes the new
allCreateStetementsvariable along with other related fields, ensuring a clean state when accessing the page.Consider fixing the typo in the variable name
allCreateStetements(should beallCreateStatements).
942-952: SQL validation method for security.This method properly validates SQL statements for security, allowing only CREATE TABLE, ALTER TABLE, and foreign key check operations while blocking potentially harmful statements.
While this is a good start, consider enhancing the validation with more sophisticated checks:
private boolean isValidSqlStatement(String sql) { sql = sql.trim().toLowerCase(); + // Remove comments that might be used to bypass validation + sql = sql.replaceAll("--.*?$|/\\*.*?\\*/", " "); // Only allow CREATE TABLE, ALTER TABLE statements, and setting foreign key checks return (sql.startsWith("create table") || sql.startsWith("alter table") || sql.startsWith("set foreign_key_checks")) && + // Check for SQL injection attempts more thoroughly !sql.contains("drop") && !sql.contains("truncate") && !sql.contains("delete") && !sql.contains("update") && + !sql.contains("insert") && + !sql.contains("exec") && + !sql.contains("execute") && + !sql.contains("union") && + !sql.matches(".*?;\\s*\\w+.*"); // Detect multiple statements }
2358-2364: Getter and setter for the allCreateStetements field.These methods allow access to the field that stores CREATE SQL statements.
Fix the typo in the variable name and methods (
allCreateStetementsshould beallCreateStatements).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/divudi/bean/common/DataAdministrationController.java(5 hunks)
🔇 Additional comments (5)
src/main/java/com/divudi/bean/common/DataAdministrationController.java (5)
280-280: New field for storing CREATE SQL statements.This field will be used to store the SQL CREATE statements that will be processed for adding missing tables and columns.
292-292: New tab index for the missing fields UI.This field will control which tab is active in the missing fields interface.
874-939: New method to process all CREATE statements in batch.This method implements the core functionality of processing multiple SQL CREATE statements in one go, which aligns with the PR objective.
The implementation:
- Splits input by "CREATE TABLE"
- Processes each statement to extract table name and generate ALTER statements
- Executes the SQL with validation and error handling
- Provides feedback on execution results
While the implementation includes good error handling and validation, consider adding a check at the beginning of the method to ensure
allCreateStetementsis not empty or null before processing:public void createTablesAndFieldsForAllCreateStatements() { StringBuilder executionResults = new StringBuilder(); + + if (allCreateStetements == null || allCreateStetements.trim().isEmpty()) { + executionFeedback = "No SQL statements provided to execute."; + return; + } System.out.println("===== Starting CREATE TABLE parsing and ALTER execution ====="); // Rest of the method remains the same }
914-921: SQL validation check added for security.Good implementation of security validation before executing SQL statements, addressing a potential security vulnerability.
The code now checks if the SQL statement is valid before execution, preventing potentially harmful operations like DROP, DELETE, etc.
2366-2372: Getter and setter for tabIndexMissingFields.These methods allow control of which tab is active in the missing fields interface.
GSMgeeth
deleted the
11421-should-be-able-to-add-all-missing-fields-and-missing-tables-in-one-go
branch
now all create statements can be pasted and run the query
Summary by CodeRabbit