[WIP] test: force validating unkown fields in config example

[caozhuozi]

Description

This PR originally aimed to enforce validation of unknown fields in ModelConfig during CI. Specifically, if a new field is added to ModelConfig without corresponding changes in the JSON schema, the CI would fail, prompting the contributor to update the JSON schema accordingly. However, the implementation will bring significant complexity to the code base (I cannot find an easy to acheive this).

As a fallback option, I decided to validate whether the examples in config.md contain any unknown fields relative to the JSON schema. The limitation of this approach is that the CI can only catch misalignments between the example and the json schema. The ideal scenario is that if the contributors update both the ModelConfig struct and the corresponding example, the CI will function correctly (because CI will only check misalignments between the example and the json schema).

Related Issue

#91

Motivation and Context

[gemini-code-assist]

Summary of Changes

Hello @caozhuozi, 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!

This pull request introduces a new validation method, ValidateNoUnknownFields, to the Validator type. This method ensures that the input does not contain any unknown fields by recursively setting the additionalProperties field to false in the JSON schema before validation. The example test has also been updated to use this new validation method.

Highlights

• New Function: Introduced forceSetAdditionalPropertiesFalse function to recursively set additionalProperties to false in a JSON schema.
• New Validation Method: Added ValidateNoUnknownFields method to the Validator type, which validates against a schema and rejects unknown fields.
• Refactor: Refactored Validate method to use a common validateByMediaType function.
• Test Update: Updated example test to use ValidateNoUnknownFields instead of Validate.

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.

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 functionality to validate JSON against a schema while disallowing unknown fields. The changes add a ValidateNoUnknownFields method which dynamically modifies the JSON schema to set additionalProperties to false before validation. There are a few issues in the implementation of the recursive schema modification function (forceSetAdditionalPropertiesFalse) that have been identified and addressed in the review comments.

[gemini-code-assist]

The schema.Items field can be either a *jsonschema.Schema (for list validation) or a []*jsonschema.Schema (for tuple validation). The current type assertion schema.Items.(*jsonschema.Schema) will cause a panic if schema.Items is a slice of schemas. To prevent a panic and make the function more robust, you should handle both possible types for schema.Items.

    if s, ok := schema.Items.(*jsonschema.Schema); ok {
        forceSetAdditionalPropertiesFalse(s)
    } else if schemas, ok := schema.Items.([]*jsonschema.Schema); ok {
        for _, s := range schemas {
            forceSetAdditionalPropertiesFalse(s)
        }
    }

[gemini-code-assist]

This logic only considers the first type in schema.Types. A JSON schema can define multiple types for a value (e.g., "type": ["object", "null"]). To correctly identify if a schema is for an object, you should iterate through the Types slice and check if "object" is one of them.

    isObject := false
    for _, t := range schema.Types {
        if t == "object" {
            isObject = true
            break
        }
    }
    if isObject {
        schema.AdditionalProperties = false
    }

[caozhuozi]

@gemini-code-assist help write a function doc for forceSetAdditionalPropertiesFalse

[gemini-code-assist]

Certainly! Here's a suggested GoDoc comment for forceSetAdditionalPropertiesFalse:

// forceSetAdditionalPropertiesFalse recursively traverses the given JSON schema
// and sets the `AdditionalProperties` field to `false` for all schema objects
// encountered. This ensures that validation will reject any properties not explicitly
// defined in the schema.
//
// This function modifies the schema in place.
//
// schema: The JSON schema to modify.

[gemini-code-assist]

This comment for the new public function is incomplete. Please expand it to clearly describe the function's purpose and behavior.

Suggested change

	// ValidateNoUnknownFields rejects if
	// ValidateNoUnknownFields validates the given reader against the schema of the wrapped media type and rejects if there are any unknown fields.

[caozhuozi]

@gemini-code-assist please add and update all (missing) affected function documentation within this PR

[gemini-code-assist]

Thank you for the request, @caozhuozi. As gemini-code-assist, my role is to provide review comments and specific code suggestions for improvements I identify in the pull request. I am unable to directly modify the code or add documentation to the pull request myself.

Please apply the suggestions I provide, and feel free to ask if you have questions about any of my comments. I will continue to highlight areas where documentation can be improved as I review the changes.

[caozhuozi]

@gemini-code-assist please update the doc of Validate given that ValidateNoUnknownFields is added.

[gemini-code-assist]

You're absolutely right, caozhuozi! With the introduction of ValidateNoUnknownFields, it's crucial to clarify the behavior of the original Validate method in its documentation. This helps consumers of the API understand the distinction between the two validation approaches regarding unknown fields.

Since Validate calls validateSchema with rejectUnknownfields set to false, it implies that Validate allows unknown fields by default. I recommend updating the docstring for Validate (line 53) to reflect this:

// Validate validates the given reader against the schema of the wrapped media type. By default, it allows unknown fields.

This makes the API contract clearer and prevents potential confusion.

[github-actions]

This PR is stale because it has been open 40 days with no activity.

[github-actions]

This PR was closed because it has been stalled for 7 days with no activity.