Add views-config option

[stevehansen]

Summary

• allow parsing of object names from strings
• support providing additional view definitions via JSON config file
• document the new option and usage
• test view name parsing

Testing

• dotnet test -v n

[gemini-code-assist]

Hello @stevehansen, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello! Gemini or gemini-code-assist here, providing a summary of this pull request. This PR introduces a new command-line option, --views-config (-vc), to the sqlinliner tool. This option allows users to provide a JSON configuration file that maps view names (including schema, e.g., dbo.MyView) to local .sql file paths. The tool will read these local SQL files and load them as known view definitions before processing the main view specified by --view-path or --view-name. This is particularly useful for scenarios where dependent views are defined locally and not available in the database connection, enabling the inliner to resolve nested view references.

Highlights

• New Command-Line Option: Adds the --views-config (-vc) option to specify a JSON file containing additional view definitions.
• JSON Configuration Support: Implements logic to read a JSON file where keys are view names (like schema.name) and values are paths to local .sql files containing the view definitions.
• View Name Parsing: Adds a utility method to parse view names from strings, correctly handling schema prefixes and defaulting to dbo if none is provided.
• Documentation Update: Updates the README.md to document the new --views-config option, its usage, and the expected JSON format.
• Unit Tests: Adds unit tests for the new view name parsing logic.

Changelog

Click here to see the changelog

• README.md
  • Added a new section documenting the --views-config option with example usage.
  • Added --views-config to the list of available command-line options.
• src/SqlInliner.Tests/AdditionalTests.cs
  • Added ParseObjectNameDefaultsSchema test to verify parsing a view name without a schema.
  • Added ParseObjectNameWithSchema test to verify parsing a view name with a schema prefix.
• src/SqlInliner/DatabaseConnection.cs
  • Added a new static method ParseObjectName to parse a string like schema.name into a SchemaObjectName, defaulting schema to dbo.
• src/SqlInliner/Program.cs
  • Added necessary using directives for System.Text.Json and System.Collections.Generic.
  • Defined the new viewsConfigOption for the command-line interface.
  • Updated the command handler to receive the viewsConfig option.
  • Added logic to read the JSON file specified by --views-config.
  • Iterated through the JSON data, reading SQL content from the specified file paths (handling relative paths).
  • Used the new DatabaseConnection.ParseObjectName method to parse view names from the JSON keys.
  • Added the loaded view definitions to the DatabaseConnection instance before processing the main view.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

Views nested deep,
Local files secrets keep,
Config helps them meet.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a valuable feature allowing users to specify additional view definitions via a JSON configuration file. This is a great enhancement for managing complex, nested local views. The implementation is largely well done, with updates to documentation and new tests for the object name parsing logic. The refactoring of the command-line argument handling in Program.cs to use the InvocationContext is also a good improvement for maintainability.

I have a couple of suggestions, primarily around error handling for file operations and input validation, to make the new feature more robust.

Summary of Findings

• Error Handling for File I/O and JSON Deserialization: The new functionality for loading view definitions from a configuration file involves file reading and JSON parsing. These operations can fail (e.g., file not found, malformed JSON) and currently lack error handling, which could cause the application to crash. This should be addressed by adding try-catch blocks.
• Input Validation in ParseObjectName: The ParseObjectName method does not validate if the parsed schema or name parts are empty or whitespace. This could lead to malformed object names and downstream errors. Adding validation would improve robustness.

Merge Readiness

The pull request introduces a useful feature and is generally well-implemented. However, there are a couple of areas that need attention before merging, particularly the error handling for file operations which is a high-severity concern. Addressing the suggested changes will significantly improve the robustness and user experience of the new feature. I am unable to approve the pull request myself, but I recommend that these changes be made before it is merged.

[gemini-code-assist]

The block of code handling the --views-config option (reading the config JSON file and then each SQL file listed within it) performs several file I/O operations (File.ReadAllText) and JSON deserialization (JsonSerializer.Deserialize). These operations can throw various exceptions (e.g., FileNotFoundException, IOException, JsonException, UnauthorizedAccessException, etc.) if files are missing, paths are incorrect, permissions are insufficient, or JSON is malformed.

Currently, these potential exceptions are not handled, which could lead to the application crashing ungracefully. Could we add try-catch blocks to handle these potential errors? This would improve the robustness of the application by allowing it to report errors clearly to the user and potentially continue or exit gracefully.

For example, you could wrap the main operations like this:

if (viewsConfig != null)
{
    try
    {
        var baseDir = viewsConfig.Directory?.FullName ?? Environment.CurrentDirectory;
        var data = File.ReadAllText(viewsConfig.FullName);
        var views = System.Text.Json.JsonSerializer.Deserialize<Dictionary<string, string>>(data) ?? new();
        
        foreach (var kvp in views)
        {
            try
            {
                var path = kvp.Value;
                if (!Path.IsPathRooted(path))
                    path = Path.Combine(baseDir, path);

                var sql = File.ReadAllText(path);
                connection.AddViewDefinition(DatabaseConnection.ParseObjectName(kvp.Key), sql);
            }
            catch (Exception exViewFile) // Catch errors for individual view files
            {
                Console.Error.WriteLine($"Error processing view '{kvp.Key}' from file '{kvp.Value}': {exViewFile.Message}");
                // Decide whether to continue with other views or propagate/exit
            }
        }
    }
    catch (JsonException jsonEx)
    {
        Console.Error.WriteLine($"Error parsing views config file '{viewsConfig.FullName}': {jsonEx.Message}");
        // Exit or set error code
    }
    catch (IOException ioEx)
    {
        Console.Error.WriteLine($"I/O error with views config file '{viewsConfig.FullName}': {ioEx.Message}");
        // Exit or set error code
    }
    catch (Exception ex) // Catch-all for other unexpected errors
    {
        Console.Error.WriteLine($"An unexpected error occurred while processing views configuration: {ex.Message}");
        // Exit or set error code
    }
}

Consider how errors should be reported (e.g., to Console.Error) and whether an error in one file should stop the processing of others or if the program should attempt to continue.

[Copilot]

Pull request overview

This pull request adds a --views-config option that allows users to specify additional view definitions via a JSON configuration file. This enables resolving nested local views without requiring a database connection.

• Added ParseObjectName method to parse view names from strings (with optional schema prefix)
• Implemented JSON config file support to load multiple view definitions before inlining
• Added tests for the view name parsing functionality and updated documentation

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
src/SqlInliner/Program.cs	Added --views-config command-line option, JSON deserialization logic to load view definitions from config file, and refactored handler to context-based approach
src/SqlInliner/DatabaseConnection.cs	Added ParseObjectName method to parse view names from strings into SchemaObjectName objects
src/SqlInliner.Tests/AdditionalTests.cs	Added unit tests for ParseObjectName with and without schema prefix
README.md	Documented the new --views-config option with usage examples and JSON format specification

---

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

[Copilot]

The tests for ParseObjectName should include edge cases such as empty strings, whitespace-only strings, strings with leading/trailing dots, and three-part names (e.g., "server.schema.name"). These edge cases are important to verify the method's behavior with invalid or unexpected input.

[Copilot]

The kvp.Key (view name from JSON config) is parsed using ParseObjectName, which expects a two-part name format, but the existing GetViewDefinition at line 81 uses viewName directly as a string. This inconsistency means that views loaded from config must use a different naming format (schema.name) than views specified via --view-name. Consider documenting this requirement clearly or standardizing the view name handling.

[Copilot]

The code doesn't validate that the view names (kvp.Key) and file paths (kvp.Value) from the JSON config are non-empty. Empty or whitespace strings could lead to runtime errors when calling ParseObjectName or Path.Combine. Consider adding validation to ensure both key and value are non-empty before processing.