feat(server): GetAssetSchemaFields which converts assets to schema fields #1461
Conversation
WalkthroughThis pull request adds functionality to retrieve and process asset schema fields via GraphQL. It introduces new types and methods for managing asset schema data in the GraphQL model and resolver layers. The changes update the execution context with marshaling functions and new error handling, and they extend the use case to process JSON and GeoJSON asset data. New tests ensure the conversion logic works as expected. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Resolver
participant Usecase
participant Converter
Client->>Resolver: Query assetSchemaFields(assetID)
Resolver->>Usecase: ConvertToSchemaFields(ctx, assetID, operator)
Usecase->>Converter: extractSchemaFieldData(contentType, content)
Converter-->>Usecase: schema fields data
Usecase-->>Resolver: AssetSchemaFieldsData
Resolver->>Client: GraphQL Response (AssetSchemaFieldResult)
Possibly related PRs
Suggested reviewers
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 (
|
✅ Deploy Preview for reearth-cms canceled.
|
![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 (3)
server/internal/usecase/interactor/asset_test.go (1)
1771-1885: Consider adding more edge cases to extractSchemaFieldData tests.While the current test cases cover the main scenarios, there are potential edge cases that aren't tested:
- GeoJSON with empty properties
- JSON Schema with properties that don't have a type field
- Mixed type arrays in GeoJSON properties
Here's how you could add a test case for GeoJSON with empty properties:
+ { + name: "valid GeoJSON with empty properties", + args: args{ + contentType: "application/geo+json", + content: mustMarshal(map[string]any{ + "type": "Feature", + "geometry": map[string]any{ + "type": "Point", + "coordinates": []interface{}{102.0, 0.5}, + }, + "properties": map[string]any{}, + }), + }, + expectedFields: []interfaces.AssetSchemaField{}, + expectedCount: 0, + expectErr: nil, + },server/internal/usecase/interactor/asset.go (2)
672-708: Add null handling when returning error responses.Currently, you're returning a non-nil empty
AssetSchemaFieldsDatastruct along with errors, which could be misleading.Consider returning nil instead of an empty struct when an error occurs:
func (i *Asset) ConvertToSchemaFields(ctx context.Context, assetID id.AssetID, operator *usecase.Operator) (*interfaces.AssetSchemaFieldsData, error) { if operator.AcOperator.User == nil && operator.Integration == nil { - return &interfaces.AssetSchemaFieldsData{}, interfaces.ErrInvalidOperator + return nil, interfaces.ErrInvalidOperator } asset, err := i.repos.Asset.FindByID(ctx, assetID) if err != nil { - return &interfaces.AssetSchemaFieldsData{}, err + return nil, err } assetFileData, err := i.repos.AssetFile.FindByID(ctx, assetID) if err != nil { - return &interfaces.AssetSchemaFieldsData{}, err + return nil, err } // check if file is a json or geojson if assetFileData.ContentType() != "application/json" && assetFileData.ContentType() != "application/geo+json" { - return &interfaces.AssetSchemaFieldsData{}, interfaces.ErrInvalidContentTypeForSchemaConversion + return nil, interfaces.ErrInvalidContentTypeForSchemaConversion } // read file file, _, err := i.gateways.File.ReadAsset(ctx, asset.UUID(), assetFileData.Path(), nil) if err != nil { - return &interfaces.AssetSchemaFieldsData{}, err + return nil, err } // read file content content, err := io.ReadAll(file) if err != nil { - return &interfaces.AssetSchemaFieldsData{}, err + return nil, err } schemaFieldData, err := extractSchemaFieldData(assetFileData.ContentType(), content) if err != nil { - return &interfaces.AssetSchemaFieldsData{}, err + return nil, err } return schemaFieldData, nil }
763-782: Consider enhancing field type detection for better type precision.The current implementation correctly identifies basic types, but could be enhanced for more precise type detection.
You might want to consider additional type information or more granular type detection:
func detectFieldType(value any) string { switch v := value.(type) { case string: + // Optionally detect if string is a date/time format + // e.g., if timeVal, err := time.Parse(time.RFC3339, v); err == nil { + // return "datetime" + // } return "string" case float64: // If the number is whole, assume it's an integer if v == float64(int(v)) { return "integer" } return "float" case bool: return "boolean" case map[string]interface{}: return "object" case []interface{}: + // Optionally detect homogeneous arrays + // e.g., if all elements are strings, return "string[]" return "array" default: return "unknown" } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
server/internal/adapter/gql/generated.go(12 hunks)server/internal/adapter/gql/gqlmodel/convert_asset.go(2 hunks)server/internal/adapter/gql/gqlmodel/models_gen.go(1 hunks)server/internal/adapter/gql/resolver_asset.go(1 hunks)server/internal/usecase/interactor/asset.go(2 hunks)server/internal/usecase/interactor/asset_test.go(2 hunks)server/internal/usecase/interfaces/asset.go(3 hunks)server/schemas/asset.graphql(1 hunks)
🧰 Additional context used
🧬 Code Definitions (6)
server/internal/adapter/gql/resolver_asset.go (3)
server/internal/adapter/gql/gqlmodel/models_gen.go (3)
AssetSchemaFieldResult(130-133)Asset(76-94)Asset(96-96)server/internal/usecase/interfaces/asset.go (1)
Asset(75-93)server/internal/adapter/gql/gqlmodel/convert_asset.go (1)
AssetsToSchemaFieldData(146-165)
server/internal/adapter/gql/gqlmodel/convert_asset.go (2)
server/internal/usecase/interfaces/asset.go (2)
AssetSchemaFieldsData(65-68)AssetSchemaField(70-73)server/internal/adapter/gql/gqlmodel/models_gen.go (3)
AssetSchemaFieldResult(130-133)AssetSchemaField(125-128)FieldType(1767-1767)
server/internal/adapter/gql/gqlmodel/models_gen.go (2)
web/src/components/molecules/Schema/types.ts (1)
FieldType(43-60)server/internal/usecase/interfaces/asset.go (1)
AssetSchemaField(70-73)
server/internal/usecase/interactor/asset_test.go (5)
server/internal/infrastructure/mongo/asset_test.go (2)
args(278-281)args(530-532)server/internal/infrastructure/memory/asset_test.go (1)
args(263-266)server/internal/usecase/interactor/asset.go (3)
Asset(29-33)detectFieldType(763-782)isJSONSchema(756-761)server/internal/usecase/interfaces/asset.go (3)
Asset(75-93)AssetSchemaFieldsData(65-68)AssetSchemaField(70-73)server/internal/usecase/interactor/request.go (1)
f(54-54)
server/internal/adapter/gql/generated.go (1)
server/internal/adapter/gql/gqlmodel/models_gen.go (19)
AssetSchemaField(125-128)FieldType(1767-1767)AssetSchemaFieldResult(130-133)e(1613-1619)e(1621-1623)e(1625-1636)e(1638-1640)e(1656-1662)e(1664-1666)e(1668-1679)e(1681-1683)e(1697-1703)e(1705-1707)e(1709-1720)e(1722-1724)e(1738-1744)e(1746-1748)e(1750-1761)e(1763-1765)
server/internal/usecase/interactor/asset.go (1)
server/internal/usecase/interfaces/asset.go (5)
Asset(75-93)AssetSchemaFieldsData(65-68)ErrInvalidContentTypeForSchemaConversion(47-47)AssetSchemaField(70-73)ErrInvalidJSONSchema(46-46)
🔇 Additional comments (25)
server/internal/adapter/gql/gqlmodel/models_gen.go (1)
125-133: LGTM! Good addition of the new types to support asset schema fields.The new
AssetSchemaFieldandAssetSchemaFieldResulttypes are well-structured and properly aligned with both the GraphQL schema and the domain model. The JSON field tags follow the snake_case convention expected in GraphQL responses.server/internal/adapter/gql/gqlmodel/convert_asset.go (1)
146-165: Function implementation looks good and handles edge cases properly.The
AssetsToSchemaFieldDatafunction:
- Handles nil input correctly
- Pre-allocates the result slice with proper capacity (good for performance)
- Correctly maps from domain model to GraphQL model
This follows best practices for conversion functions in Go.
server/internal/adapter/gql/resolver_asset.go (1)
199-210: Resolver implementation is correct and follows existing patterns.The
AssetSchemaFieldsresolver:
- Properly converts the asset ID from string to the domain type
- Returns appropriate errors when ID conversion fails
- Uses the operator pattern for authorization
- Converts the domain model to GraphQL model using the new converter function
The implementation is consistent with other resolvers in the file.
server/schemas/asset.graphql (2)
157-165: New GraphQL types look good and follow schema conventions.The
AssetSchemaFieldandAssetSchemaFieldResulttypes are well-defined with appropriate field names using snake_case as per GraphQL conventions.
170-170: Query definition looks good and aligns with the PR objectives.The new
assetSchemaFieldsquery takes anassetIdparameter and returns anAssetSchemaFieldResultas expected. The non-nullable return type is appropriate since error states are handled via GraphQL errors, not null returns.server/internal/usecase/interactor/asset_test.go (4)
1486-1641: Excellent test coverage for the new ConvertToSchemaFields functionality.The test is well-structured with clear test cases covering both successful scenarios (GeoJSON and JSON Schema) and error cases (invalid operator and asset not found). The test setup using mock assets and files is thorough.
1642-1705: Comprehensive unit test for the detectFieldType function.Your test cases cover all the possible type conversions, including edge cases like float values that should be treated as integers. Good job on the test organization and parallel execution.
1706-1770: Well-structured tests for isJSONSchema function.The test cases appropriately cover valid JSON schema and various invalid scenarios. The use of descriptive test names makes it clear what each test is verifying.
1886-1923: Good implementation of test helper functions.The
mustMarshalandmockAssetFileshelper functions are well-implemented and make the tests more readable.server/internal/usecase/interfaces/asset.go (3)
43-47: Well-defined error variables with clear descriptions.The new error variables use consistent naming and appropriate i18n translations.
65-73: Appropriate data structure for schema fields.The
AssetSchemaFieldsDataandAssetSchemaFieldtypes are well-defined for representing schema information extracted from assets.
92-92: Interface extension follows the existing pattern.The new
ConvertToSchemaFieldsmethod follows the established pattern for the Asset interface, maintaining consistency.server/internal/usecase/interactor/asset.go (1)
665-670: Good representation of the GeoJSON structure.The
GeoJSONFeaturetype correctly represents the basic structure needed for GeoJSON processing.server/internal/adapter/gql/generated.go (12)
124-132: Complexity calculation definitions look correct.These GraphQL complexity calculation definitions for
AssetSchemaFieldandAssetSchemaFieldResultmatch the model structures shown in the relevant code snippets. The field names follow the snake_case convention used in the schema, which is consistent with GraphQL best practices.
577-577: Query complexity calculation properly added.The complexity calculation function for the new
AssetSchemaFieldsquery is correctly added to the Query struct, with proper parameter signature matching the expected GraphQL input (assetID).
1044-1044: Resolver interface correctly defines the new query.The signature for
AssetSchemaFieldsmatches the GraphQL schema definition, taking a context and assetID parameter and returning the appropriate result type with error handling.
1344-1371: Complexity calculation cases properly implemented.All field complexity calculations for
AssetSchemaFieldandAssetSchemaFieldResultare correctly added to the execution context with proper null checks. This ensures accurate query complexity calculations for GraphQL execution.
3479-3490: Query complexity calculation with proper argument handling.The implementation correctly extracts and validates the assetId argument before calculating complexity. Error handling is appropriately implemented for argument extraction failures.
5373-5386: GraphQL schema definitions match requirements.The schema definitions for
AssetSchemaField,AssetSchemaFieldResult, and theassetSchemaFieldsquery match the requirements specified in the PR objectives. The field names and types are correctly defined with non-nullable constraints.
8908-8934: Argument parsing for the query is correctly implemented.The implementation properly extracts and validates the assetId argument for the new query with appropriate error handling and path context for GraphQL error reporting.
11481-11661: Field resolvers are properly implemented.The resolvers for all fields in the new types (
AssetSchemaFieldandAssetSchemaFieldResult) are correctly implemented with proper error handling, null checks, and context management. The child field contexts are also correctly defined for nested type resolution.
25103-25162: Query resolver implementation is correct.The implementation of the
assetSchemaFieldsquery resolver correctly calls the resolver middleware and handles errors appropriately. The field context is properly set up with child field resolution for the result type.
42715-42801: Type resolvers are correctly implemented.The type resolvers for
AssetSchemaFieldandAssetSchemaFieldResultare properly implemented with field selection handling, concurrent execution support via deferred processing, and appropriate error handling. The implementation follows the pattern of other type resolvers in the codebase.
46825-46847: Concurrent execution for the query is properly set up.The query is integrated into the concurrent execution context with proper error recovery and field invalidation handling, ensuring that GraphQL execution can process the query efficiently.
50629-50695: Marshaling functions are correctly implemented.The marshaling functions for the new types handle null cases properly and support concurrent processing of arrays. The implementation follows the pattern used for other types in the codebase.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
server/i18n/en.yml (2)
40-41: Clarify New Error Message Keys.
The new entries forinvalid content typeandinvalid content type for schema conversionhave been added. Please verify that these keys exactly match the error constants defined in the asset conversion use case and that any downstream logic referring to these messages uses the identical keys. Also, consider if providing a non-empty default message would be beneficial for debugging purposes.
49-49: Review the 'invalid json schema' Message.
Theinvalid json schemamessage is newly added. Ensure that it aligns with the validation logic in use and consider whether a more descriptive default message might aid in resolving JSON schema validation errors.server/i18n/ja.yml (2)
40-41: Verify Japanese Translations for New Error Messages.
The messages forinvalid content typeandinvalid content type for schema conversionhave been translated as "無効なコンテンツタイプです。" and "スキーマ変換のための無効なコンテンツタイプです。", respectively. Please confirm these translations are consistent with the application's terminology and that they convey enough context for end users encountering content validation issues.
49-49: Confirm Translation for 'invalid json schema'.
The translation "無効なJSONスキーマです。" appears correct. Verify that it is coherent with similar error messages in the file and with overall localization standards used in the project.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
server/i18n/en.yml(1 hunks)server/i18n/ja.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci-server / test
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
server/internal/usecase/interactor/asset.go (4)
692-708: Consider using defer to close the file resource.The file resource opened by
ReadAssetisn't explicitly closed, which might lead to resource leaks.Add a defer statement to ensure the file is closed:
file, _, err := i.gateways.File.ReadAsset(ctx, asset.UUID(), assetFileData.Path(), nil) if err != nil { return &interfaces.AssetSchemaFieldsData{}, err } +defer file.Close()
710-744: Check for invalid JSON Schema before accessing properties.The code properly handles type assertions for property values, but it doesn't verify the existence of the "properties" field before checking if it's a valid JSON Schema.
Consider reordering the validation logic:
// ✅ Detect JSON Schema if isJSONSchema(raw) { // Extract from "properties" - if props, ok := raw["properties"].(map[string]any); ok { + props, ok := raw["properties"].(map[string]any) + if !ok { + return result, interfaces.ErrInvalidJSONSchema + } for key, val := range props { if def, ok := val.(map[string]interface{}); ok { typeValue, ok := def["type"].(string) if !ok { continue } result.Fields = append(result.Fields, interfaces.AssetSchemaField{ FieldName: key, FieldType: typeValue, }) } } result.TotalCount = len(result.Fields) return result, nil - } else { - return result, interfaces.ErrInvalidJSONSchema - } } else { return result, interfaces.ErrInvalidJSONSchema }
774-793: Improve handling of numeric types.The current implementation differentiates between integers and floats based on whether the value has decimal digits. Consider a more robust approach for numeric type detection.
func detectFieldType(value any) string { switch v := value.(type) { case string: return "string" case float64: - // If the number is whole, assume it's an integer - if v == float64(int(v)) { + // Check if the float can be represented as an integer without loss + intVal := int64(v) + if float64(intVal) == v { return "integer" } return "number" case bool: return "boolean" case map[string]interface{}: return "object" case []interface{}: return "array" + case nil: + return "null" default: return "unknown" } }
767-772:❓ Verification inconclusive
Add more robust JSON Schema validation.
The current JSON Schema validation is minimal. Consider adding more checks to ensure it's truly a valid JSON Schema.
func isJSONSchema(data map[string]interface{}) bool { _, hasSchema := data["$schema"] _, hasProperties := data["properties"] typeVal, isTypeObj := data["type"].(string) - return hasSchema && hasProperties && isTypeObj && typeVal == "object" + + // Check for standard JSON Schema URI patterns + isStandardSchema := false + if schemaVal, ok := data["$schema"].(string); ok { + isStandardSchema = strings.Contains(schemaVal, "json-schema.org") || + strings.Contains(schemaVal, "schema.json") + } + + return (hasSchema && isStandardSchema) && hasProperties && isTypeObj && typeVal == "object" }
🌐 Web query:
What are standard JSON Schema URI patterns?💡 Result:
JSON Schema defines specific patterns and conventions for URIs to ensure unambiguous references and validation. Here are the key standards and practices:
Base URI Determination
- The base URI is resolved using the JSON Schema
$idkeyword at the root level, which sets the absolute URI for the schema[1][7].
Example:In this case, the base URI becomes{ "$id": "https://example.com/schemas/address", "type": "object" }https://example.com/schemas/address, overriding the retrieval URI[1].URI Validation
- The
format: "uri"keyword enforces RFC 3986 compliance, requiring absolute URIs (not relative references)[6][9].
Example validation rule:This rejects relative URIs like{ "type": "string", "format": "uri" }../pathor#fragmentunless resolved against a base URI[6].Resolution Rules
$refresolves URIs against the base URI defined by$id[1][3].
For instance, a$ref: "billing-address"underhttps://example.com/schemas/addressresolves tohttps://example.com/schemas/billing-address.Relative references (e.g.,
subschema#/definitions/entry) are resolved using the base URI but are not valid standalone forformat: "uri"[6][9].Key Considerations
- Avoid Ambiguity: Always use absolute URIs in
$idto prevent resolution failures in anonymous or bundled schemas[1][7].- Terminology:
- URI: Absolute identifier per RFC 3986.
- URI Reference: Relative path requiring a base URI for resolution[6][9].
- Rejected Formats: A proposed
format: "url"(for WHATWG URL standards) was declined to maintain consistency with RFC 3986[9].Example Usage in Schemas
{ "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "https://example.com/schemas/geocoordinates", "type": "object", "properties": { "website": { "type": "string", "format": "uri" }, "coordinates": { "$ref": "https://example.com/schemas/geocoordinates#/definitions/latlng" } } }By adhering to these patterns, JSON Schemas ensure interoperability and strict validation of resource identifiers.
Citations:
- 1: https://json-schema.org/understanding-json-schema/structuring
- 2: https://shop.machinemfg.com/comprehensive-guide-to-json-schema-data-types-and-validation/
- 3: https://json-schema.org/learn/getting-started-step-by-step
- 4: https://apidog.com/blog/json-schema-types/
- 5: https://opis.io/json-schema/2.x/structure.html
- 6: https://groups.google.com/g/json-schema/c/eAJ_4yLLaME
- 7: https://www.w3.org/TR/vc-json-schema/
- 8: https://www.mongodb.com/resources/languages/json-schema-examples
- 9: Proposal: add format "url" json-schema-org/json-schema-spec#233
Improve JSON Schema URI Validation
The current update adds a check for standard URI patterns by looking for substrings like
"json-schema.org/draft"or"schema.json". While this is a step up from the minimal check, the JSON Schema specification recommends stricter URI validation. For instance, valid schemas typically include a draft version (e.g.,"https://json-schema.org/draft-07/schema#"or"https://json-schema.org/draft/2020-12/schema") and often rely on an absolute URI—possibly defined via the$idkeyword—to avoid ambiguity.Suggestions:
- Consider using a regex or URL parser (e.g., Go’s
net/urlpackage) to validate that the$schemavalue is a well-formed and absolute URI.- Optionally, check for the existence of a
$idfield to further ensure proper schema identification.- Update unit tests to cover a range of valid and invalid schema URIs.
Affected File:
server/internal/usecase/interactor/asset.go(Lines: 767-772)Diff Suggestion:
func isJSONSchema(data map[string]interface{}) bool { _, hasSchema := data["$schema"] _, hasProperties := data["properties"] typeVal, isTypeObj := data["type"].(string) - return hasSchema && hasProperties && isTypeObj && typeVal == "object" + // Validate against standard JSON Schema URI patterns + isStandardSchema := false + if schemaVal, ok := data["$schema"].(string); ok { + // Basic check for known JSON Schema patterns. + // Consider using a regex or URL parsing to enforce proper URI format (e.g., ensure the URI is absolute and contains a valid draft version marker). + isStandardSchema = strings.Contains(schemaVal, "json-schema.org/draft") || + strings.Contains(schemaVal, "schema.json") + } + + return hasSchema && isStandardSchema && hasProperties && isTypeObj && typeVal == "object" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/internal/usecase/interactor/asset.go(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
server/internal/usecase/interactor/asset.go (4)
server/internal/usecase/interfaces/asset.go (5)
Asset(75-93)AssetSchemaFieldsData(65-68)ErrInvalidContentTypeForSchemaConversion(47-47)AssetSchemaField(70-73)ErrInvalidJSONSchema(46-46)server/internal/usecase/repo/asset.go (1)
Asset(17-26)server/internal/usecase/gateway/file.go (1)
File(63-73)server/pkg/file/file.go (1)
File(19-25)
🔇 Additional comments (2)
server/internal/usecase/interactor/asset.go (2)
665-670: LGTM: Clean GeoJSONFeature struct definition.The struct definition for GeoJSONFeature is well-structured and includes all the essential fields for a GeoJSON feature with proper JSON tags.
672-691: Good implementation with proper validation.The method correctly validates the operator, retrieves the asset and file data, and ensures the content type is appropriate before proceeding. The error handling is also comprehensive.
Overview
Implement GetAssetSchemaFields GQL to convert asset to schema fields based on Asset ID
GQL query
response example
{ "data": { "assetSchemaFields": { "total_count": 2, "fields": [ { "field_name": "productName", "field_type": "string" }, { "field_name": "productId", "field_type": "integer" } ] } } }tech doc: https://www.notion.so/eukarya/Schema-or-model-exporting-importing-with-Items-b2ac52dfc8bc4a6eadeaf20edec8427f?pvs=4
What I've done
What I haven't done
How I tested
Screenshot
Which point I want you to review particularly
Memo
Summary by CodeRabbit
New Features
Bug Fixes