diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1eddb81..f6fffeb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,7 +44,6 @@ jobs: uses: actions/checkout@v4 with: clean: 'true' - submodules: recursive - name: Set up Go uses: actions/setup-go@v5 @@ -83,8 +82,6 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 - with: - submodules: recursive - name: Set up Go uses: actions/setup-go@v5 @@ -106,8 +103,6 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 - with: - submodules: recursive - name: Set up Go uses: actions/setup-go@v5 @@ -129,8 +124,6 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 - with: - submodules: recursive - name: Set up Go uses: actions/setup-go@v5 @@ -150,16 +143,12 @@ jobs: fail_ci_if_error: false # token: ${{ secrets.CODECOV_TOKEN }} # Uncomment if required for private repos - e2e: - name: E2E Tests (gts-spec) + gts-spec-tests: + name: GTS Spec Tests runs-on: ubuntu-latest - env: - PYTHONDONTWRITEBYTECODE: 1 steps: - name: Checkout uses: actions/checkout@v4 - with: - submodules: recursive - name: Set up Go uses: actions/setup-go@v5 @@ -167,55 +156,9 @@ jobs: go-version: ${{ env.GO_VERSION }} cache: true - - name: Build release binary - run: go build -o ./bin/gts ./cmd/gts - - - name: Set up Python - uses: actions/setup-python@v5 - with: - python-version: '3.11' - - - name: Install pytest and dependencies - run: pip install pytest requests httprunner - - - name: Run e2e tests - run: | - # Start server in background with logs - ./bin/gts server --port 8000 > .server.log 2>&1 & - SERVER_PID=$! - echo "Started GTS server with PID $SERVER_PID" - - # Wait for server to be ready - echo "Waiting for GTS server to start..." - for i in $(seq 1 30); do - if ! kill -0 $SERVER_PID 2>/dev/null; then - echo "ERROR: Server process died unexpectedly" - echo "=== Server logs ===" - cat .server.log || true - exit 1 - fi - if curl -sf http://127.0.0.1:8000/entities > /dev/null 2>&1; then - echo "GTS server is ready!" - break - fi - if [ $i -eq 30 ]; then - echo "ERROR: GTS server failed to start within 30 seconds" - echo "=== Server logs ===" - cat .server.log || true - kill $SERVER_PID 2>/dev/null || true - exit 1 - fi - echo "Attempt $i/30: Server not ready yet, waiting..." - sleep 1 - done - - # Run tests - pytest -p no:cacheprovider ./.gts-spec/tests -v - TEST_EXIT=$? - - # Stop server - echo "Stopping GTS server..." - kill $SERVER_PID 2>/dev/null || true - wait $SERVER_PID 2>/dev/null || true - - exit $TEST_EXIT + # Builds the gts binary, pulls the gts-spec test-runner image from + # GHCR (tag from .gts-spec-version), starts the server natively, waits + # for readiness, then runs pytest inside the container against + # host.docker.internal:$PORT. See Makefile. + - name: Run gts-spec tests via docker + run: make gts-spec-tests diff --git a/.gitmodules b/.gitmodules deleted file mode 100644 index e168b55..0000000 --- a/.gitmodules +++ /dev/null @@ -1,3 +0,0 @@ -[submodule ".gts-spec"] - path = .gts-spec - url = https://github.com/GlobalTypeSystem/gts-spec.git diff --git a/.gts-spec b/.gts-spec deleted file mode 160000 index 23ed727..0000000 --- a/.gts-spec +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 23ed727f1e85e808c2a4337d767d8309a5f40d0d diff --git a/.gts-spec-version b/.gts-spec-version new file mode 100644 index 0000000..87a1cf5 --- /dev/null +++ b/.gts-spec-version @@ -0,0 +1 @@ +v0.12.0 diff --git a/CLAUDE.md b/CLAUDE.md index ec90465..1254b19 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,59 +6,62 @@ Guidance for Claude Code when working in this repository. `gts-go` is the Go reference implementation of [GTS](https://github.com/GlobalTypeSystem/gts-spec) — library (`gts/`), CLI (`cmd/gts`), and an HTTP server (`cmd/gts-server` / `gts server`) that answers the REST API exercised by the shared gts-spec conformance suite. -`.gts-spec/` is the spec vendored as a git submodule; `tests/` inside it is the conformance suite. See `README.md` for API/CLI details and `make help` for all targets. +The spec version this implementation targets is pinned in +[`.gts-spec-version`](.gts-spec-version) (used verbatim as the GHCR tag for +the test-runner image). Bumping the pin and running the suite is how new +spec features land. See `README.md` for API/CLI details and `make help` for +all targets. ## Running the gts-spec Test Suite -The short form is `make e2e PORT=8001` (PORT defaults to 8000; override if busy — the target fails fast if the port is already in use). It bootstraps the venv, rebuilds, starts the server, runs pytest, and shuts everything down. - -### Bootstrap the venv (first time only) - -Tests depend on `httprunner`. Installed into `.gts-spec/.venv/` (gitignored by the submodule). +`make gts-spec-tests` runs the gts-spec conformance suite against a freshly +built server. Tests come from the published runner image +`ghcr.io/globaltypesystem/gts-spec-tests`; the tag comes from +[`.gts-spec-version`](.gts-spec-version) as an immutable +`vMAJOR.MINOR.PATCH` — every commit reproduces the same test run, and +rolling forward is a deliberate bump of that file. Requires a working +Docker daemon plus the Go toolchain (the target builds the server binary +before pulling the test-runner image). ```bash -make e2e-venv # uses python3 by default -make e2e-venv PYTHON=python3.11 # Python 3.11 is the safest (httprunner still pins pydantic<2) +make gts-spec-tests # full suite on :8000 +make gts-spec-tests PORT=8001 # different port +make gts-spec-tests TEST=test_op1_id_validation.py # single file / selector ``` -### Run pytest manually against a running server - -Useful when iterating on a single test without the full `make e2e` cycle. +Opt into the rolling minor tag, try a different patch, or test a fork: ```bash -make build -./bin/gts server --port 8001 & - -PYTEST=".gts-spec/.venv/bin/python -m pytest" - -# Whole suite -$PYTEST .gts-spec/tests --gts-base-url http://127.0.0.1:8001 - -# One file / one class -$PYTEST .gts-spec/tests/test_refimpl_x_gts_final_abstract.py --gts-base-url http://127.0.0.1:8001 -$PYTEST .gts-spec/tests/test_op12_schema_vs_schema_validation.py::TestCaseOp12_FinalBase_RejectDerived --gts-base-url http://127.0.0.1:8001 +make gts-spec-tests GTS_SPEC_VERSION=v0.11 # rolling vMAJOR.MINOR +make gts-spec-tests GTS_SPEC_VERSION=v0.11.0 # specific patch +make gts-spec-tests GTS_SPEC_IMAGE=ghcr.io/your-fork/gts-spec-tests ``` -`GTS_BASE_URL` env var works too. The server holds state in memory with no reset endpoint — restart between full-suite runs. +Iterating on the test suite itself? Mount a local checkout over `/tests`: -### Open-files limit on macOS +```bash +make gts-spec-tests GTS_SPEC_TESTS_DIR=../gts-spec/tests +``` -Before running the full suite, raise the FD limit in the calling shell: +For tight test-edit loops, keep a long-running server in one terminal and +re-run targeted tests in another: ```bash -ulimit -n 4096 -``` +# Terminal 1 +make gts-server PORT=8001 -`httprunner` leaks a keep-alive socket per test class (the underlying -`requests.Session` is never closed), so FDs grow linearly. macOS's -default 256 soft cap is hit around test ~240 with `EMFILE: Too many -open files`. Linux defaults (1024+) are usually fine. `make e2e` runs -its commands in a subshell, so the `ulimit` must be set in the parent -shell that invokes `make`. Don't bake `ulimit` into the Makefile — -it's an environment concern, not a build step. +# Terminal 2 +make gts-spec-tests-run PORT=8001 TEST=test_op12_type_derivation_validation.py +``` ## Working in This Repo -- `.gts-spec` is a submodule. Bump with `make update-spec`, then rerun `make e2e` to pick up new spec tests. -- Handlers in `server/` stay thin — logic goes in `gts/` where it is unit-testable. New REST behavior usually already has coverage in `.gts-spec/tests/`; run the relevant file before and after to confirm. -- `make check` is the full local gate: fmt + vet + lint + test + e2e. +- The spec is no longer a git submodule. To bump the conformance target, + edit `.gts-spec-version` and run `make gts-spec-tests` — the new image + is pulled on demand. +- Handlers in `server/` stay thin — logic goes in `gts/` where it is + unit-testable. New REST behavior usually already has coverage in the + gts-spec test suite; iterating on a single test via + `make gts-spec-tests TEST=...` is the fastest signal. +- `make check` is the full local gate: fmt + vet + lint + test + + gts-spec-tests. diff --git a/Makefile b/Makefile index ead2abe..83cda04 100644 --- a/Makefile +++ b/Makefile @@ -1,18 +1,10 @@ CI := 1 -.PHONY: help build dev-fmt all check fmt vet lint test security coverage update-spec e2e-venv e2e - -# Python interpreter used to bootstrap the venv; override with `make e2e PYTHON=python3.11` -PYTHON ?= python3 -VENV_DIR := .gts-spec/.venv -VENV_PY := $(VENV_DIR)/bin/python - -# Port for the reference server during `make e2e`; override with `make e2e PORT=8001` -PORT ?= 8000 - # Default target - show help .DEFAULT_GOAL := help +.PHONY: help build dev-fmt all check fmt vet lint test security coverage + # Show this help message help: @awk '/^# / { desc=substr($$0, 3) } /^[a-zA-Z0-9_-]+:/ && desc { target=$$1; sub(/:$$/, "", target); printf "%-20s - %s\n", target, desc; desc="" }' Makefile | sort @@ -66,33 +58,156 @@ coverage: go tool cover -func=coverage.out @echo "To view HTML report: go tool cover -html=coverage.out" -# Update gts-spec submodule to latest -update-spec: - git submodule update --remote .gts-spec - -# Create/refresh the Python venv used by the gts-spec e2e test suite -e2e-venv: $(VENV_PY) +# Run all quality checks +check: fmt vet lint test gts-spec-tests + + +# ============================================================================== +# gts-spec conformance tests +# ============================================================================== +# +# Two flows: +# +# * `make gts-spec-tests` — one-shot: builds the gts binary, pulls the +# test-runner image, starts the server, runs pytest, shuts the server down. +# This is what CI runs. +# +# * `make gts-server` + `make gts-spec-tests-run` — long-running server in +# one terminal, repeated test runs in another. For iterative test work. +# +# Both use the gts-spec test-runner image from GHCR; the tag comes from +# .gts-spec-version (used verbatim, format `vMAJOR.MINOR.PATCH`). + +.PHONY: gts-server gts-spec-tests gts-spec-tests-run gts-spec-tests-pull gts-spec-version-check + +# Port for the reference server; override with `PORT=8001` +PORT ?= 8000 -$(VENV_PY): - @if [ ! -x "$(VENV_PY)" ]; then \ - echo "Creating venv at $(VENV_DIR) using $(PYTHON)..."; \ - $(PYTHON) -m venv $(VENV_DIR); \ +# gts-spec test-runner image. The tag is read from `.gts-spec-version` +# (canonical pin for this implementation, format `vMAJOR.MINOR.PATCH` — an +# immutable patch tag so every commit is reproducible). Override either +# variable to follow a rolling minor tag or test a fork: +# make gts-spec-tests GTS_SPEC_VERSION=v0.11 +# make gts-spec-tests GTS_SPEC_IMAGE=ghcr.io/your-fork/gts-spec-tests +GTS_SPEC_IMAGE ?= ghcr.io/globaltypesystem/gts-spec-tests +GTS_SPEC_VERSION ?= $(strip $(shell cat .gts-spec-version 2>/dev/null)) +GTS_SPEC_REF ?= $(GTS_SPEC_IMAGE):$(GTS_SPEC_VERSION) + +# Optional: path to a local checkout of gts-spec/tests to bind-mount over +# /tests inside the runner image. Useful when iterating on the test suite +# itself alongside the server, e.g.: +# make gts-spec-tests GTS_SPEC_TESTS_DIR=../gts-spec/tests +GTS_SPEC_TESTS_DIR ?= + +# Optional pytest selector passed straight to the runner. Examples: +# make gts-spec-tests TEST=test_op1_id_validation.py +# make gts-spec-tests TEST=test_op12_type_derivation_validation.py::TestCaseOp12_FinalBase_RejectDerived +TEST ?= + +# Seconds to wait for the GTS server to start accepting requests. +SERVER_READY_TIMEOUT ?= 10 + +# Shell snippet (expanded inline) that dumps the last 50 lines of .server.log +# to stderr if the file exists and is non-empty. Reused by error paths so the +# operator sees *why* the server failed rather than just "did not respond". +DUMP_SERVER_LOG = \ + if [ -s .server.log ]; then \ + echo "=== last 50 lines of .server.log ==="; \ + tail -n 50 .server.log; \ + echo "=== end .server.log ==="; \ fi - @echo "Installing gts-spec e2e test dependencies..." - @$(VENV_PY) -m pip install --quiet --upgrade pip setuptools wheel - @$(VENV_PY) -m pip install --quiet httprunner -# Run end-to-end tests against gts-spec. -e2e: build e2e-venv - @rm -rf logs e2e.log - @set -e; \ +# Shell snippet (expanded inline) that polls /entities every 200 ms until the +# server answers, or fails after $(SERVER_READY_TIMEOUT) seconds. Assumes the +# server PID has been written to .server.pid and stdout/stderr to .server.log +# by the caller. +WAIT_FOR_SERVER = \ + echo "Waiting up to $(SERVER_READY_TIMEOUT)s for server on :$(PORT)..."; \ + attempts=$$(( $(SERVER_READY_TIMEOUT) * 5 )); \ + for i in $$(seq 1 $$attempts); do \ + if ! kill -0 $$(cat .server.pid) 2>/dev/null; then \ + echo "ERROR: server process exited before becoming ready"; \ + $(DUMP_SERVER_LOG); \ + exit 1; \ + fi; \ + if curl -sf "http://127.0.0.1:$(PORT)/entities" >/dev/null 2>&1; then \ + echo "Server ready after ~$$(( i * 200 ))ms."; \ + break; \ + fi; \ + sleep 0.2; \ + done; \ + curl -sf "http://127.0.0.1:$(PORT)/entities" >/dev/null 2>&1 || { \ + echo "ERROR: server did not respond within $(SERVER_READY_TIMEOUT)s"; \ + $(DUMP_SERVER_LOG); \ + exit 1; \ + } + +# Shell snippet (expanded inline) that fails fast if $(PORT) is already bound, +# wipes the previous run's logs, installs a cleanup trap, starts the gts server +# in the background on $(PORT) with stdout/stderr redirected to .server.log, +# and waits for readiness via $(WAIT_FOR_SERVER). Binds to 0.0.0.0 so the +# test-runner container can reach the server via host.docker.internal on +# Linux (Docker Desktop on Mac/Windows routes loopback transparently, but the +# Linux docker0 bridge does not — a 127.0.0.1 bind is unreachable from there). +START_SERVER = \ + if lsof -nP -iTCP:$(PORT) -sTCP:LISTEN >/dev/null 2>&1; then \ + echo "ERROR: port $(PORT) is already in use"; \ + lsof -nP -iTCP:$(PORT) -sTCP:LISTEN; \ + exit 1; \ + fi; \ + rm -rf logs; rm -f .server.log; \ trap 'kill `cat .server.pid 2>/dev/null` 2>/dev/null || true; rm -f .server.pid' INT TERM EXIT; \ - echo "Starting server in background on port $(PORT)..."; \ - ./bin/gts server --port $(PORT) & echo $$! > .server.pid; \ - sleep 2; \ - echo "Running e2e tests..."; \ - PYTHONDONTWRITEBYTECODE=1 $(VENV_PY) -m pytest -p no:cacheprovider --log-file=e2e.log --gts-base-url http://127.0.0.1:$(PORT) ./.gts-spec/tests; \ - echo "E2E tests completed successfully" - -# Run all quality checks -check: fmt vet lint test e2e + echo "Starting server in background on port $(PORT) (logs: .server.log)..."; \ + ./bin/gts server -host 0.0.0.0 -port $(PORT) >.server.log 2>&1 & echo $$! > .server.pid; \ + $(WAIT_FOR_SERVER) + +# Shell snippet (expanded inline) that runs the gts-spec test-runner image +# against host.docker.internal:$(PORT). Shared by `gts-spec-tests` (full +# one-shot cycle) and `gts-spec-tests-run` (against an externally-running +# server, paired with `make gts-server`). +RUN_TESTS_DOCKER = \ + echo "Running gts-spec tests via $(GTS_SPEC_REF)..."; \ + docker run --rm \ + --add-host=host.docker.internal:host-gateway \ + $(if $(GTS_SPEC_TESTS_DIR),-v "$(abspath $(GTS_SPEC_TESTS_DIR)):/tests") \ + $(GTS_SPEC_REF) \ + --gts-base-url http://host.docker.internal:$(PORT) \ + $(TEST) + +# Run the gts server in the foreground. Pair with `make gts-spec-tests-run +# TEST=...` for iterative test development. Binds to 0.0.0.0 so the +# test-runner container can reach the server via host.docker.internal on +# Linux (see START_SERVER for the full rationale). +gts-server: build + ./bin/gts server -host 0.0.0.0 -port $(PORT) + +# Validate $(GTS_SPEC_VERSION) is non-empty and looks like a version tag +# (vMAJOR.MINOR or vMAJOR.MINOR.PATCH). Catches missing/garbled +# .gts-spec-version before it turns into an opaque docker error. +gts-spec-version-check: + @case "$(GTS_SPEC_VERSION)" in \ + "") echo "ERROR: spec version is empty — populate .gts-spec-version or pass GTS_SPEC_VERSION=..."; exit 1 ;; \ + *[!0-9v.]*) echo "ERROR: GTS_SPEC_VERSION='$(GTS_SPEC_VERSION)' — expected 'vMAJOR.MINOR' or 'vMAJOR.MINOR.PATCH'"; exit 1 ;; \ + v[0-9]*.[0-9]*) ;; \ + v[0-9]*.[0-9]*.[0-9]*) ;; \ + *) echo "ERROR: GTS_SPEC_VERSION='$(GTS_SPEC_VERSION)' — expected 'vMAJOR.MINOR' or 'vMAJOR.MINOR.PATCH'"; exit 1 ;; \ + esac + +# Pull the gts-spec test-runner image from GHCR. Skips the network round-trip +# when the image is already in the local cache (safe because the default tag +# is an immutable vMAJOR.MINOR.PATCH; bumping .gts-spec-version produces a new +# ref, which forces a fresh pull). Use `docker rmi $(GTS_SPEC_REF)` to force +# a refetch of a rolling tag. +gts-spec-tests-pull: gts-spec-version-check + @docker image inspect $(GTS_SPEC_REF) >/dev/null 2>&1 || docker pull $(GTS_SPEC_REF) + +# Run gts-spec conformance suite against an already-running server on $(PORT) +gts-spec-tests-run: gts-spec-tests-pull + @$(RUN_TESTS_DOCKER) + +# Run gts-spec conformance suite (docker) against the local server +gts-spec-tests: build gts-spec-tests-pull + @set -e; \ + $(START_SERVER); \ + $(RUN_TESTS_DOCKER); \ + echo "gts-spec tests completed successfully" diff --git a/README.md b/README.md index 602e543..0d9a1f0 100644 --- a/README.md +++ b/README.md @@ -302,42 +302,46 @@ go run ./cmd/gts-server -host 127.0.0.1 -port 8000 -verbose 1 ### Testing -You can test the gts-go library by utilizing the shared test suite from the [gts-spec](https://github.com/GlobalTypeSystem/gts-spec) specification and executing the tests against the web server. - -Executing gts-spec Tests on the Server: +`make gts-spec-tests` runs the shared +[gts-spec](https://github.com/GlobalTypeSystem/gts-spec) conformance suite +against a freshly built server. Tests come from the published runner +image `ghcr.io/globaltypesystem/gts-spec-tests`; the tag is pinned in +[`.gts-spec-version`](.gts-spec-version) as an immutable +`vMAJOR.MINOR.PATCH` — every commit reproduces the same test run, and +rolling forward is a deliberate bump of that file. Requires a working +Docker daemon plus the Go toolchain (the target builds the server binary +before pulling the test-runner image). ```bash -# getting the tests -git clone https://github.com/GlobalTypeSystem/gts-spec.git -cd gts-spec/tests - -# run tests against the web server on port 8000 (default) -pytest +make gts-spec-tests # full suite on :8000 +make gts-spec-tests PORT=8001 # different port +make gts-spec-tests TEST=test_op1_id_validation.py # single file / selector +``` -# override server URL using GTS_BASE_URL environment variable -GTS_BASE_URL=http://127.0.0.1:8001 pytest +Opt into the rolling minor tag, try a different patch, or test a fork: -# or set it persistently -export GTS_BASE_URL=http://127.0.0.1:8001 -pytest +```bash +make gts-spec-tests GTS_SPEC_VERSION=v0.11 # rolling vMAJOR.MINOR +make gts-spec-tests GTS_SPEC_VERSION=v0.11.0 # specific patch +make gts-spec-tests GTS_SPEC_IMAGE=ghcr.io/your-fork/gts-spec-tests ``` -#### Raising the open-files limit (macOS) - -The gts-spec suite uses `httprunner`, which leaks a keep-alive socket per -test class (the underlying `requests.Session` is never closed). File -descriptors grow linearly and hit macOS's default 256 soft cap around -test ~240 with `EMFILE: Too many open files`. Before running the full -suite, raise the limit in your shell: +Iterating on the test suite itself? Mount a local checkout over `/tests`: ```bash -ulimit -n 4096 -pytest +make gts-spec-tests GTS_SPEC_TESTS_DIR=../gts-spec/tests ``` -Linux defaults (typically 1024+) are usually high enough, but bumping -the limit there is harmless. `make e2e` runs in a subshell, so set -`ulimit` in the shell that invokes it. +For tight test-edit loops, keep a long-running server in one terminal and +re-run targeted tests in another: + +```bash +# Terminal 1 +make gts-server PORT=8001 + +# Terminal 2 +make gts-spec-tests-run PORT=8001 TEST=test_op12_type_derivation_validation.py +``` ## License diff --git a/cmd/gts/cast.go b/cmd/gts/cast.go index 5e482b0..48d449d 100644 --- a/cmd/gts/cast.go +++ b/cmd/gts/cast.go @@ -6,13 +6,13 @@ Released under Apache License 2.0 package main var cmdCast = &Command{ - UsageLine: "cast -from -to ", - Short: "cast an instance to a target schema", + UsageLine: "cast -from -to ", + Short: "cast an instance to a target type-schema", Long: ` -Cast transforms an instance to conform to a target schema version. +Cast transforms an instance to conform to a target type-schema version. The -from flag specifies the source instance GTS ID. -The -to flag specifies the target schema GTS ID. +The -to flag specifies the target type-schema GTS ID. Requires -path to be set to load entities. Example: @@ -29,7 +29,7 @@ var ( func init() { cmdCast.Run = runCast cmdCast.Flag.StringVar(&castFrom, "from", "", "source instance GTS ID") - cmdCast.Flag.StringVar(&castTo, "to", "", "target schema GTS ID") + cmdCast.Flag.StringVar(&castTo, "to", "", "target type-schema GTS ID") } func runCast(cmd *Command, args []string) { diff --git a/cmd/gts/compatibility.go b/cmd/gts/compatibility.go index 58dce42..e19c2b7 100644 --- a/cmd/gts/compatibility.go +++ b/cmd/gts/compatibility.go @@ -6,13 +6,13 @@ Released under Apache License 2.0 package main var cmdCompatibility = &Command{ - UsageLine: "compatibility -old -new ", - Short: "check compatibility between two schemas", + UsageLine: "compatibility -old -new ", + Short: "check compatibility between two type-schemas", Long: ` -Compatibility checks whether two schema versions are compatible. +Compatibility checks whether two type-schema versions are compatible. -The -old flag specifies the old schema GTS ID. -The -new flag specifies the new schema GTS ID. +The -old flag specifies the old type-schema GTS ID. +The -new flag specifies the new type-schema GTS ID. Requires -path to be set to load entities. Example: @@ -28,8 +28,8 @@ var ( func init() { cmdCompatibility.Run = runCompatibility - cmdCompatibility.Flag.StringVar(&compatOld, "old", "", "old schema GTS ID") - cmdCompatibility.Flag.StringVar(&compatNew, "new", "", "new schema GTS ID") + cmdCompatibility.Flag.StringVar(&compatOld, "old", "", "old type-schema GTS ID") + cmdCompatibility.Flag.StringVar(&compatNew, "new", "", "new type-schema GTS ID") } func runCompatibility(cmd *Command, args []string) { diff --git a/cmd/gts/helpers.go b/cmd/gts/helpers.go index 12b0d3c..50bbfaa 100644 --- a/cmd/gts/helpers.go +++ b/cmd/gts/helpers.go @@ -70,7 +70,7 @@ func loadConfig(path string) *gts.GtsConfig { var data struct { EntityIDFields []string `json:"entity_id_fields"` - SchemaIDFields []string `json:"schema_id_fields"` + TypeIDFields []string `json:"type_id_fields"` } if err := json.NewDecoder(f).Decode(&data); err != nil { @@ -80,7 +80,7 @@ func loadConfig(path string) *gts.GtsConfig { return >s.GtsConfig{ EntityIDFields: data.EntityIDFields, - SchemaIDFields: data.SchemaIDFields, + TypeIDFields: data.TypeIDFields, } } diff --git a/gts/cast.go b/gts/cast.go index 0d8d7ed..8cc068b 100644 --- a/gts/cast.go +++ b/gts/cast.go @@ -21,7 +21,7 @@ type CastResult struct { // Cast transforms an instance to conform to a target schema version // see gts-python store.py cast method -func (s *GtsStore) Cast(instanceID, toSchemaID string) (*CastResult, error) { +func (s *GtsStore) Cast(instanceID, toTypeID string) (*CastResult, error) { // Get instance entity instanceEntity := s.Get(instanceID) if instanceEntity == nil { @@ -29,26 +29,26 @@ func (s *GtsStore) Cast(instanceID, toSchemaID string) (*CastResult, error) { } // Get target schema - toSchema := s.Get(toSchemaID) + toSchema := s.Get(toTypeID) if toSchema == nil { - return nil, &StoreGtsSchemaNotFoundError{EntityID: toSchemaID} + return nil, &StoreGtsSchemaNotFoundError{EntityID: toTypeID} } - // Determine source schema - var fromSchemaID string + // Determine source type-schema + var fromTypeID string var fromSchema *JsonEntity - if instanceEntity.IsSchema { - // Not allowed to cast directly from a schema + if instanceEntity.IsTypeSchema { + // Not allowed to cast directly from a type-schema return nil, &StoreGtsCastFromSchemaNotAllowedError{FromID: instanceID} } else { - // Casting an instance - need to find its schema - fromSchemaID = instanceEntity.SchemaID - if fromSchemaID == "" { + // Casting an instance - need to find its type-schema + fromTypeID = instanceEntity.TypeID + if fromTypeID == "" { return nil, &StoreGtsSchemaForInstanceNotFoundError{EntityID: instanceID} } - fromSchema = s.Get(fromSchemaID) + fromSchema = s.Get(fromTypeID) if fromSchema == nil { - return nil, &StoreGtsSchemaNotFoundError{EntityID: fromSchemaID} + return nil, &StoreGtsSchemaNotFoundError{EntityID: fromTypeID} } } @@ -58,13 +58,13 @@ func (s *GtsStore) Cast(instanceID, toSchemaID string) (*CastResult, error) { toSchemaContent := toSchema.Content // Perform the cast - return castInstance(instanceID, toSchemaID, instanceContent, fromSchemaContent, toSchemaContent, s) + return castInstance(instanceID, toTypeID, instanceContent, fromSchemaContent, toSchemaContent, s) } // castInstance performs the actual casting logic // see gts-python schema_cast.py cast method func castInstance( - fromInstanceID, toSchemaID string, + fromInstanceID, toTypeID string, fromInstanceContent, fromSchemaContent, toSchemaContent map[string]any, store *GtsStore, ) (*CastResult, error) { @@ -72,7 +72,7 @@ func castInstance( targetSchema := flattenSchema(toSchemaContent) // Determine direction - direction := inferDirection(fromInstanceID, toSchemaID) + direction := inferDirection(fromInstanceID, toTypeID) // Determine which is old/new based on direction var oldSchema, newSchema map[string]any @@ -116,9 +116,9 @@ func castInstance( return &CastResult{ CompatibilityResult: &CompatibilityResult{ FromID: fromInstanceID, - ToID: toSchemaID, + ToID: toTypeID, OldID: fromInstanceID, - NewID: toSchemaID, + NewID: toTypeID, Direction: direction, AddedProperties: deduplicate(added), RemovedProperties: deduplicate(removed), @@ -333,7 +333,7 @@ func validateWithGtsIDTolerance(instance, schema map[string]any, store *GtsStore // Pre-load all schemas from the store for id, entity := range store.byID { - if entity.IsSchema { + if entity.IsTypeSchema { compiler.AddResource(id, entity.Content) } } diff --git a/gts/compatibility.go b/gts/compatibility.go index 85c6cf9..fc1b374 100644 --- a/gts/compatibility.go +++ b/gts/compatibility.go @@ -26,16 +26,16 @@ type CompatibilityResult struct { // CheckCompatibility checks compatibility between two schemas // see gts-python store.py is_minor_compatible method -func (s *GtsStore) CheckCompatibility(oldSchemaID, newSchemaID string) *CompatibilityResult { - oldEntity := s.Get(oldSchemaID) - newEntity := s.Get(newSchemaID) +func (s *GtsStore) CheckCompatibility(oldTypeID, newTypeID string) *CompatibilityResult { + oldEntity := s.Get(oldTypeID) + newEntity := s.Get(newTypeID) if oldEntity == nil || newEntity == nil { return &CompatibilityResult{ - FromID: oldSchemaID, - ToID: newSchemaID, - OldID: oldSchemaID, - NewID: newSchemaID, + FromID: oldTypeID, + ToID: newTypeID, + OldID: oldTypeID, + NewID: newTypeID, Direction: "unknown", AddedProperties: []string{}, RemovedProperties: []string{}, @@ -53,10 +53,10 @@ func (s *GtsStore) CheckCompatibility(oldSchemaID, newSchemaID string) *Compatib newSchema, ok2 := newEntity.Content, newEntity.Content != nil if !ok1 || !ok2 { return &CompatibilityResult{ - FromID: oldSchemaID, - ToID: newSchemaID, - OldID: oldSchemaID, - NewID: newSchemaID, + FromID: oldTypeID, + ToID: newTypeID, + OldID: oldTypeID, + NewID: newTypeID, Direction: "unknown", AddedProperties: []string{}, RemovedProperties: []string{}, @@ -75,13 +75,13 @@ func (s *GtsStore) CheckCompatibility(oldSchemaID, newSchemaID string) *Compatib isForward, forwardErrors := checkForwardCompatibility(oldSchema, newSchema) // Determine direction - direction := inferDirection(oldSchemaID, newSchemaID) + direction := inferDirection(oldTypeID, newTypeID) return &CompatibilityResult{ - FromID: oldSchemaID, - ToID: newSchemaID, - OldID: oldSchemaID, - NewID: newSchemaID, + FromID: oldTypeID, + ToID: newTypeID, + OldID: oldTypeID, + NewID: newTypeID, Direction: direction, AddedProperties: []string{}, RemovedProperties: []string{}, diff --git a/gts/config.go b/gts/config.go index 7f3a2c3..e19075a 100644 --- a/gts/config.go +++ b/gts/config.go @@ -8,7 +8,7 @@ package gts // GtsConfig holds configuration for extracting GTS IDs from JSON content type GtsConfig struct { EntityIDFields []string - SchemaIDFields []string + TypeIDFields []string } // DefaultGtsConfig returns the default configuration for ID extraction @@ -25,7 +25,7 @@ func DefaultGtsConfig() *GtsConfig { "gts_iid", "id", }, - SchemaIDFields: []string{ + TypeIDFields: []string{ "gtsTid", "gtsType", "gtsT", diff --git a/gts/extract.go b/gts/extract.go index e3224ff..baebf19 100644 --- a/gts/extract.go +++ b/gts/extract.go @@ -19,25 +19,25 @@ type JsonFile struct { // JsonEntity represents a JSON object with extracted GTS identifiers type JsonEntity struct { - GtsID *GtsID - SchemaID string - SelectedEntityField string - SelectedSchemaIDField string - IsSchema bool - Content map[string]any - File *JsonFile - ListSequence *int - Label string - GtsRefs []*GtsReference // All GTS ID references found in content + GtsID *GtsID + TypeID string + SelectedEntityField string + SelectedTypeIDField string + IsTypeSchema bool + Content map[string]any + File *JsonFile + ListSequence *int + Label string + GtsRefs []*GtsReference // All GTS ID references found in content } // ExtractIDResult holds the result of extracting ID information from JSON content type ExtractIDResult struct { - ID string `json:"id"` - SchemaID *string `json:"schema_id"` - SelectedEntityField *string `json:"selected_entity_field"` - SelectedSchemaIDField *string `json:"selected_schema_id_field"` - IsSchema bool `json:"is_schema"` + ID string `json:"id"` + TypeID *string `json:"type_id"` + SelectedEntityField *string `json:"selected_entity_field"` + SelectedTypeIDField *string `json:"selected_type_id_field"` + IsTypeSchema bool `json:"is_type_schema"` } // NewJsonEntity creates a JsonEntity from JSON content using the provided config @@ -53,7 +53,7 @@ func NewJsonEntityWithFile(content map[string]any, cfg *GtsConfig, file *JsonFil entity := &JsonEntity{ Content: content, - IsSchema: isJSONSchema(content), + IsTypeSchema: isJSONSchema(content), File: file, ListSequence: listSequence, } @@ -61,12 +61,12 @@ func NewJsonEntityWithFile(content map[string]any, cfg *GtsConfig, file *JsonFil // Extract entity ID entityIDValue := entity.calcJSONEntityID(cfg) - // Extract schema ID - entity.SchemaID = entity.calcJSONSchemaID(cfg, entityIDValue) + // Extract type ID + entity.TypeID = entity.calcJSONTypeID(cfg, entityIDValue) // ID extraction logic based on entity type - if entity.IsSchema { - // For schemas: use entity ID (should be from $id field) + if entity.IsTypeSchema { + // For type-schemas: use entity ID (should be from $id field) if entityIDValue != "" && IsValidGtsID(entityIDValue) { gtsID, _ := NewGtsID(entityIDValue) entity.GtsID = gtsID @@ -77,14 +77,14 @@ func NewJsonEntityWithFile(content map[string]any, cfg *GtsConfig, file *JsonFil // Well-known instance: GTS ID in id field gtsID, _ := NewGtsID(entityIDValue) entity.GtsID = gtsID - // Schema ID should be derived from the chain if not explicitly set - if entity.SchemaID == "" && entity.SelectedEntityField != "" { - entity.SchemaID = entity.calcJSONSchemaID(cfg, entityIDValue) + // Type ID should be derived from the chain if not explicitly set + if entity.TypeID == "" && entity.SelectedEntityField != "" { + entity.TypeID = entity.calcJSONTypeID(cfg, entityIDValue) } } else { // Anonymous instance: non-GTS ID in id field, GTS type in type field // GtsID remains nil for anonymous instances - // entity.SchemaID should be set from type field + // entity.TypeID should be set from type field } } @@ -111,7 +111,7 @@ func (e *JsonEntity) EffectiveID() string { if e.GtsID != nil && e.GtsID.ID != "" { return e.GtsID.ID } - if !e.IsSchema && e.SelectedEntityField != "" { + if !e.IsTypeSchema && e.SelectedEntityField != "" { if val, ok := e.Content[e.SelectedEntityField].(string); ok { if id := strings.TrimSpace(val); id != "" { return id @@ -216,47 +216,53 @@ func (e *JsonEntity) calcJSONEntityID(cfg *GtsConfig) string { return value } -// calcJSONSchemaID extracts the schema ID from JSON content -func (e *JsonEntity) calcJSONSchemaID(cfg *GtsConfig, entityIDValue string) string { - if e.IsSchema { - // For derived schemas, derive parent type from chain +// calcJSONTypeID extracts the type ID from JSON content +func (e *JsonEntity) calcJSONTypeID(cfg *GtsConfig, entityIDValue string) string { + if e.IsTypeSchema { + // For derived type-schemas, derive parent type from chain if entityIDValue != "" && IsValidGtsID(entityIDValue) && strings.HasSuffix(entityIDValue, "~") { firstTilde := strings.Index(entityIDValue, "~") if firstTilde > 0 { secondTilde := strings.Index(entityIDValue[firstTilde+1:], "~") if secondTilde > 0 { - // This is a derived schema, derive parent from chain - e.SelectedSchemaIDField = e.SelectedEntityField + // This is a derived type-schema, derive parent from chain + e.SelectedTypeIDField = e.SelectedEntityField return entityIDValue[:firstTilde+1] } } } - // For base schemas: get schema ID from $schema field + // For base type-schemas: get type ID from $schema field. + // Per gts-spec v0.11, type_id MUST be a GTS Type Identifier or null — + // JSON Schema dialect URLs (and other non-GTS values) are no longer + // accepted as type_id. Leave SelectedTypeIDField set either way so + // callers can see we did inspect $schema. if schemaValue := e.getFieldValue("$schema"); schemaValue != "" { - e.SelectedSchemaIDField = "$schema" - return schemaValue + e.SelectedTypeIDField = "$schema" + if strings.HasSuffix(schemaValue, "~") && IsValidGtsID(schemaValue) { + return schemaValue + } } return "" } - // For instances: try entity ID chain first, then SchemaIDFields + // For instances: try entity ID chain first, then TypeIDFields if entityIDValue != "" && IsValidGtsID(entityIDValue) { // For instances, find last ~ and return everything up to and including it // But skip if entity ID ends with ~ (that would be a type, not an instance) if !strings.HasSuffix(entityIDValue, "~") { lastTilde := strings.LastIndex(entityIDValue, "~") if lastTilde > 0 { - e.SelectedSchemaIDField = e.SelectedEntityField + e.SelectedTypeIDField = e.SelectedEntityField return entityIDValue[:lastTilde+1] } } } - // If no entity ID found, use SchemaIDFields to find schema reference - field, value := e.firstNonEmptyField(cfg.SchemaIDFields) + // If no entity ID found, use TypeIDFields to find type reference + field, value := e.firstNonEmptyField(cfg.TypeIDFields) if value != "" { - e.SelectedSchemaIDField = field + e.SelectedTypeIDField = field return value } @@ -268,12 +274,12 @@ func ExtractID(content map[string]any, cfg *GtsConfig) *ExtractIDResult { entity := NewJsonEntity(content, cfg) result := &ExtractIDResult{ - IsSchema: entity.IsSchema, + IsTypeSchema: entity.IsTypeSchema, } - // Set SchemaID as pointer (nil if empty) - if entity.SchemaID != "" { - result.SchemaID = &entity.SchemaID + // Set TypeID as pointer (nil if empty) + if entity.TypeID != "" { + result.TypeID = &entity.TypeID } // Set SelectedEntityField as pointer (nil if empty) @@ -281,19 +287,19 @@ func ExtractID(content map[string]any, cfg *GtsConfig) *ExtractIDResult { result.SelectedEntityField = &entity.SelectedEntityField } - // Set SelectedSchemaIDField as pointer (nil if empty) - if entity.SelectedSchemaIDField != "" { - result.SelectedSchemaIDField = &entity.SelectedSchemaIDField + // Set SelectedTypeIDField as pointer (nil if empty) + if entity.SelectedTypeIDField != "" { + result.SelectedTypeIDField = &entity.SelectedTypeIDField } - // Diagnostic: for schemas and well-known instances, return the parsed GTS ID. - // For non-schema entities without a valid GTS ID (anonymous or malformed), - // fall back to the raw value of the selected id field. This differs from - // EffectiveID (registrable), which requires a resolvable schema. + // Diagnostic: for type-schemas and well-known instances, return the parsed + // GTS ID. For non-type-schema entities without a valid GTS ID (anonymous or + // malformed), fall back to the raw value of the selected id field. This + // differs from EffectiveID (registrable), which requires a resolvable type. switch { case entity.GtsID != nil: result.ID = entity.GtsID.ID - case !entity.IsSchema && entity.SelectedEntityField != "": + case !entity.IsTypeSchema && entity.SelectedEntityField != "": if val, ok := content[entity.SelectedEntityField].(string); ok { result.ID = val } diff --git a/gts/extract_test.go b/gts/extract_test.go index 4d9f216..fc73030 100644 --- a/gts/extract_test.go +++ b/gts/extract_test.go @@ -77,7 +77,7 @@ func TestExtractID_SchemaID(t *testing.T) { tests := []struct { name string content map[string]any - expectedSchemaID string + expectedTypeID string expectedSchemaField string }{ { @@ -86,7 +86,7 @@ func TestExtractID_SchemaID(t *testing.T) { "gtsId": "gts.vendor.package.namespace.type.v0.1", "$schema": "gts.vendor.package.namespace.type.v0~", }, - expectedSchemaID: "gts.vendor.package.namespace.type.v0~", + expectedTypeID: "gts.vendor.package.namespace.type.v0~", expectedSchemaField: "$schema", }, { @@ -95,7 +95,7 @@ func TestExtractID_SchemaID(t *testing.T) { "gtsId": "gts.vendor.package.namespace.type.v0.1", "gtsTid": "gts.vendor.package.namespace.type.v0~", }, - expectedSchemaID: "gts.vendor.package.namespace.type.v0~", + expectedTypeID: "gts.vendor.package.namespace.type.v0~", expectedSchemaField: "gtsTid", }, { @@ -103,7 +103,7 @@ func TestExtractID_SchemaID(t *testing.T) { content: map[string]any{ "gtsId": "gts.vendor.package.namespace.type.v0~a.b.c.d.v1.0", }, - expectedSchemaID: "gts.vendor.package.namespace.type.v0~", + expectedTypeID: "gts.vendor.package.namespace.type.v0~", expectedSchemaField: "gtsId", // Derived from the chained ID }, } @@ -111,18 +111,18 @@ func TestExtractID_SchemaID(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := ExtractID(tt.content, nil) - var gotSchemaID string - if result.SchemaID != nil { - gotSchemaID = *result.SchemaID + var gotTypeID string + if result.TypeID != nil { + gotTypeID = *result.TypeID } - if gotSchemaID != tt.expectedSchemaID { - t.Errorf("Expected SchemaID %q, got %q", tt.expectedSchemaID, gotSchemaID) + if gotTypeID != tt.expectedTypeID { + t.Errorf("Expected TypeID %q, got %q", tt.expectedTypeID, gotTypeID) } // Handle both empty string expectation and actual value var got string - if result.SelectedSchemaIDField != nil { - got = *result.SelectedSchemaIDField + if result.SelectedTypeIDField != nil { + got = *result.SelectedTypeIDField } if got != tt.expectedSchemaField { t.Errorf("Expected schema field %q, got %q", tt.expectedSchemaField, got) @@ -181,8 +181,8 @@ func TestExtractID_IsSchema(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := ExtractID(tt.content, nil) - if result.IsSchema != tt.expectedSchema { - t.Errorf("Expected IsSchema %v, got %v", tt.expectedSchema, result.IsSchema) + if result.IsTypeSchema != tt.expectedSchema { + t.Errorf("Expected IsTypeSchema %v, got %v", tt.expectedSchema, result.IsTypeSchema) } }) } @@ -192,7 +192,7 @@ func TestExtractID_IsSchema(t *testing.T) { func TestExtractID_CustomConfig(t *testing.T) { customCfg := &GtsConfig{ EntityIDFields: []string{"customId", "id"}, - SchemaIDFields: []string{"customType", "type"}, + TypeIDFields: []string{"customType", "type"}, } content := map[string]any{ @@ -212,17 +212,17 @@ func TestExtractID_CustomConfig(t *testing.T) { } t.Errorf("Expected customId field, got %q", got) } - var gotSchemaID string - if result.SchemaID != nil { - gotSchemaID = *result.SchemaID + var gotTypeID string + if result.TypeID != nil { + gotTypeID = *result.TypeID } - if gotSchemaID != "gts.vendor.package.namespace.type.v0~" { - t.Errorf("Expected SchemaID from customType field, got %q", gotSchemaID) + if gotTypeID != "gts.vendor.package.namespace.type.v0~" { + t.Errorf("Expected TypeID from customType field, got %q", gotTypeID) } - if result.SelectedSchemaIDField == nil || *result.SelectedSchemaIDField != "customType" { + if result.SelectedTypeIDField == nil || *result.SelectedTypeIDField != "customType" { var got string - if result.SelectedSchemaIDField != nil { - got = *result.SelectedSchemaIDField + if result.SelectedTypeIDField != nil { + got = *result.SelectedTypeIDField } t.Errorf("Expected customType field, got %q", got) } @@ -260,7 +260,9 @@ func TestExtractID_InvalidIDInField(t *testing.T) { } } -// TestExtractID_SchemaIDFallback tests schema ID extraction for schemas with $schema field +// TestExtractID_SchemaIDFallback tests type ID extraction for type-schemas with $schema field. +// Per gts-spec v0.11, type_id MUST be a GTS Type Identifier; a JSON Schema dialect URL +// in $schema is no longer carried through to the type_id (it's rejected). func TestExtractID_SchemaIDFallback(t *testing.T) { content := map[string]any{ "$id": "gts.vendor.package.namespace.type.v0~", @@ -269,17 +271,53 @@ func TestExtractID_SchemaIDFallback(t *testing.T) { result := ExtractID(content, nil) - // For schemas, ID comes from $id field + // For type-schemas, ID comes from $id field if result.ID != "gts.vendor.package.namespace.type.v0~" { t.Errorf("Expected ID from $id field, got %q", result.ID) } - var gotSchemaID string - if result.SchemaID != nil { - gotSchemaID = *result.SchemaID + // JSON Schema dialect URL must NOT be carried into type_id. + if result.TypeID != nil { + t.Errorf("Expected TypeID to be nil for non-GTS $schema, got %q", *result.TypeID) } - // For base schemas, schema_id comes from $schema field - if gotSchemaID != "http://json-schema.org/draft-07/schema#" { - t.Errorf("Expected SchemaID from $schema field, got %q", gotSchemaID) + // We did inspect $schema, so the selected-field marker should still be set. + if result.SelectedTypeIDField == nil || *result.SelectedTypeIDField != "$schema" { + var got string + if result.SelectedTypeIDField != nil { + got = *result.SelectedTypeIDField + } + t.Errorf("Expected SelectedTypeIDField=\"$schema\", got %q", got) + } +} + +// TestExtractID_DollarSchema_GtsTypeID accepts a GTS Type Identifier as type_id. +func TestExtractID_DollarSchema_GtsTypeID(t *testing.T) { + content := map[string]any{ + "$id": "gts.vendor.pkg.ns.type.v1~", + "$schema": "gts.vendor.pkg.ns.type.v0~", + } + + result := ExtractID(content, nil) + + if result.TypeID == nil || *result.TypeID != "gts.vendor.pkg.ns.type.v0~" { + var got string + if result.TypeID != nil { + got = *result.TypeID + } + t.Errorf("Expected TypeID=gts.vendor.pkg.ns.type.v0~, got %q", got) + } +} + +// TestExtractID_DollarSchema_NonGtsValueRejected rejects non-GTS strings. +func TestExtractID_DollarSchema_NonGtsValueRejected(t *testing.T) { + content := map[string]any{ + "$id": "gts.vendor.pkg.ns.type.v1~", + "$schema": "gts.but.not.a.type.identifier", // valid-looking but missing trailing ~ + } + + result := ExtractID(content, nil) + + if result.TypeID != nil { + t.Errorf("Expected TypeID to be nil for non-Type-Identifier $schema, got %q", *result.TypeID) } } @@ -303,8 +341,8 @@ func TestExtractID_GtsURIPrefix_InDollarIdField(t *testing.T) { if result.ID != "gts.vendor.package.namespace.type.v1.0~" { t.Errorf("Expected ID without gts:// prefix %q, got %q", "gts.vendor.package.namespace.type.v1.0~", result.ID) } - if !result.IsSchema { - t.Errorf("Expected IsSchema to be true") + if !result.IsTypeSchema { + t.Errorf("Expected IsTypeSchema to be true") } } diff --git a/gts/ops.go b/gts/ops.go index 8464eae..81ee7cd 100644 --- a/gts/ops.go +++ b/gts/ops.go @@ -14,7 +14,7 @@ import ( type IDValidationResult struct { ID string `json:"id"` Valid bool `json:"valid"` - IsSchema bool `json:"is_schema"` + IsType bool `json:"is_type"` IsWildcard bool `json:"is_wildcard"` Error string `json:"error,omitempty"` } @@ -33,13 +33,13 @@ func ValidateGtsID(gtsID string) *IDValidationResult { _, err := validateWildcard(gtsID) if err != nil { result.Valid = false - result.IsSchema = false + result.IsType = false result.Error = formatValidateError(gtsID, err) return result } result.Valid = true - result.IsSchema = strings.HasSuffix(gtsID, "~*") || strings.HasSuffix(gtsID, ".*") + result.IsType = strings.HasSuffix(gtsID, "~*") || strings.HasSuffix(gtsID, ".*") return result } @@ -47,13 +47,13 @@ func ValidateGtsID(gtsID string) *IDValidationResult { id, err := NewGtsID(gtsID) if err != nil { result.Valid = false - result.IsSchema = false + result.IsType = false result.Error = formatValidateError(gtsID, err) return result } result.Valid = true - result.IsSchema = id.IsType() + result.IsType = id.IsType() return result } diff --git a/gts/parse.go b/gts/parse.go index b858fef..9cb70bf 100644 --- a/gts/parse.go +++ b/gts/parse.go @@ -24,7 +24,7 @@ type ParseIDResult struct { ID string `json:"id"` OK bool `json:"ok"` IsWildcard bool `json:"is_wildcard"` - IsSchema bool `json:"is_schema"` + IsType bool `json:"is_type"` Segments []ParseIDSegment `json:"segments"` Error string `json:"error,omitempty"` } @@ -43,7 +43,7 @@ func ParseID(gtsID string) ParseIDResult { ID: gtsID, OK: false, IsWildcard: true, - IsSchema: false, + IsType: false, Segments: nil, Error: err.Error(), } @@ -63,14 +63,14 @@ func ParseID(gtsID string) ParseIDResult { } } - // Wildcard patterns ending with .* are type patterns (schemas) - isSchema := strings.HasSuffix(gtsID, ".*") || strings.HasSuffix(gtsID, "~*") + // Wildcard patterns ending with .* or ~* are type patterns + isType := strings.HasSuffix(gtsID, ".*") || strings.HasSuffix(gtsID, "~*") return ParseIDResult{ ID: gtsID, OK: true, IsWildcard: true, - IsSchema: isSchema, + IsType: isType, Segments: segments, Error: "", } @@ -83,7 +83,7 @@ func ParseID(gtsID string) ParseIDResult { ID: gtsID, OK: false, IsWildcard: false, - IsSchema: false, + IsType: false, Segments: nil, Error: err.Error(), } @@ -107,7 +107,7 @@ func ParseID(gtsID string) ParseIDResult { ID: gtsID, OK: true, IsWildcard: false, - IsSchema: id.IsType(), + IsType: id.IsType(), Segments: segments, Error: "", } diff --git a/gts/parse_test.go b/gts/parse_test.go index 9adba3c..96222ce 100644 --- a/gts/parse_test.go +++ b/gts/parse_test.go @@ -277,9 +277,9 @@ func TestParseID_VersionComponents(t *testing.T) { t.Errorf("Expected ver_minor=%d, got %d", *tt.verMinor, *seg.VerMinor) } - // Check overall ID type (IsSchema), not individual segment's IsType - if result.IsSchema != tt.isType { - t.Errorf("Expected is_schema=%v, got %v", tt.isType, result.IsSchema) + // Check overall ID type (IsType), not individual segment's IsType + if result.IsType != tt.isType { + t.Errorf("Expected is_type=%v, got %v", tt.isType, result.IsType) } }) } @@ -424,11 +424,11 @@ func TestParseID_AllComponents(t *testing.T) { } } - // Check overall ID type (IsSchema), not individual segment's IsType + // Check overall ID type (IsType), not individual segment's IsType // The first segment is a type segment (ends with ~), so seg.IsType is true - // But the overall ID is an instance (doesn't end with ~), so result.IsSchema is false - if result.IsSchema { - t.Error("Expected is_schema=false for instance ID") + // But the overall ID is an instance (doesn't end with ~), so result.IsType is false + if result.IsType { + t.Error("Expected is_type=false for instance ID") } } diff --git a/gts/registry_test.go b/gts/registry_test.go index 87bbd50..54ef13c 100644 --- a/gts/registry_test.go +++ b/gts/registry_test.go @@ -201,7 +201,7 @@ func TestValidateSchema(t *testing.T) { if err == nil { t.Fatal("Expected error for non-schema ID") } - if !strings.Contains(err.Error(), "is not a schema") { + if !strings.Contains(err.Error(), "is not a type-schema") { t.Errorf("Expected 'is not a schema' error, got: %v", err) } }) @@ -228,8 +228,8 @@ func TestValidateSchema(t *testing.T) { "name": "Test Instance", }, DefaultGtsConfig()) - // Force it to be treated as non-schema - instance.IsSchema = false + // Force it to be treated as non-type-schema + instance.IsTypeSchema = false err := store.Register(instance) if err != nil { t.Fatalf("Failed to register instance: %v", err) @@ -237,9 +237,9 @@ func TestValidateSchema(t *testing.T) { err = store.ValidateSchema("gts.test.pkg.ns.instance.v1~") if err == nil { - t.Fatal("Expected error for entity that is not a schema") + t.Fatal("Expected error for entity that is not a type-schema") } - if !strings.Contains(err.Error(), "is not a schema") { + if !strings.Contains(err.Error(), "is not a type-schema") { t.Errorf("Expected 'is not a schema' error, got: %v", err) } }) diff --git a/gts/relationships.go b/gts/relationships.go index 38d064e..a0f42c2 100644 --- a/gts/relationships.go +++ b/gts/relationships.go @@ -7,10 +7,10 @@ package gts // SchemaGraphNode represents a node in the schema relationship graph type SchemaGraphNode struct { - ID string `json:"id"` - Refs map[string]*SchemaGraphNode `json:"refs,omitempty"` - SchemaID *SchemaGraphNode `json:"schema_id,omitempty"` - Errors []string `json:"errors,omitempty"` + ID string `json:"id"` + Refs map[string]*SchemaGraphNode `json:"refs,omitempty"` + TypeID *SchemaGraphNode `json:"type_id,omitempty"` + Errors []string `json:"errors,omitempty"` } // BuildSchemaGraph recursively builds a relationship graph for a GTS entity @@ -57,14 +57,14 @@ func (s *GtsStore) buildNode(gtsID string, seen map[string]bool) *SchemaGraphNod node.Refs = refs } - // Process schema ID if present - if entity.SchemaID != "" { - if !isJSONSchemaURL(entity.SchemaID) { - node.SchemaID = s.buildNode(entity.SchemaID, seen) + // Process type ID if present + if entity.TypeID != "" { + if !isJSONSchemaURL(entity.TypeID) { + node.TypeID = s.buildNode(entity.TypeID, seen) } - } else if !entity.IsSchema { - // Instance without schema ID is an error - node.Errors = append(node.Errors, "Schema not recognized") + } else if !entity.IsTypeSchema { + // Instance without type ID is an error + node.Errors = append(node.Errors, "Type-schema not recognized") } return node diff --git a/gts/schema_compat.go b/gts/schema_compat.go index b2ebc17..d43bac3 100644 --- a/gts/schema_compat.go +++ b/gts/schema_compat.go @@ -28,9 +28,9 @@ import ( // ValidateSchemaChainResult is the result of OP#12 schema chain validation. type ValidateSchemaChainResult struct { - SchemaID string `json:"schema_id"` - OK bool `json:"ok"` - Error string `json:"error,omitempty"` + TypeID string `json:"type_id"` + OK bool `json:"ok"` + Error string `json:"error,omitempty"` } // ValidateSchemaChain validates each derived schema against its base across the chain (OP#12). @@ -38,14 +38,14 @@ func (s *GtsStore) ValidateSchemaChain(schemaID string) *ValidateSchemaChainResu gid, err := NewGtsID(schemaID) if err != nil { return &ValidateSchemaChainResult{ - SchemaID: schemaID, - OK: false, - Error: fmt.Sprintf("Invalid GTS ID: %v", err), + TypeID: schemaID, + OK: false, + Error: fmt.Sprintf("Invalid GTS ID: %v", err), } } if len(gid.Segments) < 2 { - return &ValidateSchemaChainResult{SchemaID: schemaID, OK: true} + return &ValidateSchemaChainResult{TypeID: schemaID, OK: true} } segments := gid.Segments @@ -59,9 +59,9 @@ func (s *GtsStore) ValidateSchemaChain(schemaID string) *ValidateSchemaChainResu if isFinal, ok := baseEntity.Content[KeyXGtsFinal]; ok { if final, isBool := isFinal.(bool); isBool && final { return &ValidateSchemaChainResult{ - SchemaID: schemaID, - OK: false, - Error: fmt.Sprintf("base type '%s' is final and cannot be extended", baseID), + TypeID: schemaID, + OK: false, + Error: fmt.Sprintf("base type '%s' is final and cannot be extended", baseID), } } } @@ -70,17 +70,17 @@ func (s *GtsStore) ValidateSchemaChain(schemaID string) *ValidateSchemaChainResu baseContent, err := s.resolveSchemaRefsChecked(baseID) if err != nil { return &ValidateSchemaChainResult{ - SchemaID: schemaID, - OK: false, - Error: fmt.Sprintf("Schema '%s' has %v", baseID, err), + TypeID: schemaID, + OK: false, + Error: fmt.Sprintf("Schema '%s' has %v", baseID, err), } } derivedContent, err := s.resolveSchemaRefsChecked(derivedID) if err != nil { return &ValidateSchemaChainResult{ - SchemaID: schemaID, - OK: false, - Error: fmt.Sprintf("Schema '%s' has %v", derivedID, err), + TypeID: schemaID, + OK: false, + Error: fmt.Sprintf("Schema '%s' has %v", derivedID, err), } } @@ -90,8 +90,8 @@ func (s *GtsStore) ValidateSchemaChain(schemaID string) *ValidateSchemaChainResu errs := validateSchemaCompatibility(baseEff, derivedEff, baseID, derivedID, false) if len(errs) > 0 { return &ValidateSchemaChainResult{ - SchemaID: schemaID, - OK: false, + TypeID: schemaID, + OK: false, Error: fmt.Sprintf( "Schema '%s' is not compatible with base '%s': %s", derivedID, baseID, strings.Join(errs, "; "), @@ -100,7 +100,7 @@ func (s *GtsStore) ValidateSchemaChain(schemaID string) *ValidateSchemaChainResu } } - return &ValidateSchemaChainResult{SchemaID: schemaID, OK: true} + return &ValidateSchemaChainResult{TypeID: schemaID, OK: true} } // buildIDFromSegments reconstructs a GTS ID string from a slice of segments. @@ -256,12 +256,22 @@ func validateSchemaCompatibility(base, derived *effectiveSchema, baseID, derived } } + // Derived "loosens" additionalProperties only when it *explicitly* declares a + // permissive value at its own root. Omitting the keyword is NOT loosening + // when derived is composed as allOf:[{$ref: closed_base}, overlay]: per JSON + // Schema draft-07, additionalProperties only inspects sibling properties at + // the same level, but the base's additionalProperties:false still applies to + // the same instance via $ref/allOf composition, so closedness is inherited. + // The per-property loop above already catches the only structurally dangerous + // case (derived adds a new top-level property the closed base forbids). if baseDisallowsAdditional { - derivedAlsoClosed := false - if b, ok := derived.additionalProperties.(bool); ok && !b { - derivedAlsoClosed = true + derivedExplicitlyAllows := false + if derived.additionalProperties != nil { + if b, ok := derived.additionalProperties.(bool); !ok || b { + derivedExplicitlyAllows = true + } } - if !derivedAlsoClosed { + if derivedExplicitlyAllows { errors = append(errors, fmt.Sprintf( "derived schema '%s' loosens additionalProperties from false in base '%s'", derivedID, baseID, @@ -807,24 +817,27 @@ func (s *GtsStore) resolveSchemaRefsChecked(schemaID string) (map[string]any, er if entity == nil { return nil, fmt.Errorf("schema '%s' not found", schemaID) } - if !entity.IsSchema { + if !entity.IsTypeSchema { return nil, fmt.Errorf("entity '%s' is not a schema", schemaID) } return s.resolveRefs(entity.Content) } -// resolveRefs resolves all $ref references in a schema map, detecting cycles and duplicates. +// resolveRefs resolves all $ref references in a schema map, detecting cycles. +// +// Uses DFS-path cycle detection: a $ref target is held in the visited set only +// while its resolution is in progress on the current DFS stack and removed once +// that subtree finishes. Re-entry into an in-progress target is a true cycle. +// Multiple independent occurrences of the same $ref (e.g. a duplicate ref in an +// allOf composition) are NOT flagged — redundant manual aggregation across an +// $id chain is allowed (ADR-0002). func (s *GtsStore) resolveRefs(schema map[string]any) (map[string]any, error) { visited := make(map[string]bool) cycleFound := false - dupFound := false - resolved := s.resolveRefsInner(schema, visited, &cycleFound, &dupFound) + resolved := s.resolveRefsInner(schema, visited, &cycleFound) if cycleFound { return nil, fmt.Errorf("circular $ref detected") } - if dupFound { - return nil, fmt.Errorf("duplicate sibling $ref in allOf") - } if m, ok := resolved.(map[string]any); ok { if ref := findUnresolvedRef(m); ref != "" { return nil, fmt.Errorf("unresolved $ref: %v", ref) @@ -856,7 +869,7 @@ func findUnresolvedRef(schema any) string { return "" } -func (s *GtsStore) resolveRefsInner(schema any, visited map[string]bool, cycleFound *bool, dupFound *bool) any { +func (s *GtsStore) resolveRefsInner(schema any, visited map[string]bool, cycleFound *bool) any { switch v := schema.(type) { case map[string]any: // Handle $ref @@ -864,7 +877,7 @@ func (s *GtsStore) resolveRefsInner(schema any, visited map[string]bool, cycleFo if strings.HasPrefix(refVal, "#") { // local refs kept as-is result := make(map[string]any) for k, val := range v { - result[k] = s.resolveRefsInner(val, visited, cycleFound, dupFound) + result[k] = s.resolveRefsInner(val, visited, cycleFound) } return result } @@ -875,7 +888,7 @@ func (s *GtsStore) resolveRefsInner(schema any, visited map[string]bool, cycleFo result := make(map[string]any) for k, val := range v { if k != "$ref" { - result[k] = s.resolveRefsInner(val, visited, cycleFound, dupFound) + result[k] = s.resolveRefsInner(val, visited, cycleFound) } } if len(result) == 0 { @@ -885,9 +898,9 @@ func (s *GtsStore) resolveRefsInner(schema any, visited map[string]bool, cycleFo } entity := s.Get(canonical) - if entity != nil && entity.IsSchema { + if entity != nil && entity.IsTypeSchema { visited[canonical] = true - resolved := s.resolveRefsInner(entity.Content, visited, cycleFound, dupFound) + resolved := s.resolveRefsInner(entity.Content, visited, cycleFound) delete(visited, canonical) if resolvedMap, ok := resolved.(map[string]any); ok { @@ -909,7 +922,7 @@ func (s *GtsStore) resolveRefsInner(schema any, visited map[string]bool, cycleFo } for k, val := range v { if k != "$ref" { - merged[k] = s.resolveRefsInner(val, visited, cycleFound, dupFound) + merged[k] = s.resolveRefsInner(val, visited, cycleFound) } } return merged @@ -922,7 +935,7 @@ func (s *GtsStore) resolveRefsInner(schema any, visited map[string]bool, cycleFo if k == "$ref" { result[k] = val } else { - result[k] = s.resolveRefsInner(val, visited, cycleFound, dupFound) + result[k] = s.resolveRefsInner(val, visited, cycleFound) } } return result @@ -938,27 +951,12 @@ func (s *GtsStore) resolveRefsInner(schema any, visited map[string]bool, cycleFo mergedOther := make(map[string]any) anyMerged := false - seenRefs := make(map[string]bool) for _, item := range allOf { - if itemMap, ok := item.(map[string]any); ok { - if refVal, ok := itemMap["$ref"].(string); ok { - canonical := strings.TrimPrefix(refVal, GtsURIPrefix) - if seenRefs[canonical] { - *dupFound = true - continue - } - seenRefs[canonical] = true - } - } - - itemWasRef := false - if itemMap, ok := item.(map[string]any); ok { - if _, hasRef := itemMap["$ref"].(string); hasRef { - itemWasRef = true - } - } - - resolved := s.resolveRefsInner(item, visited, cycleFound, dupFound) + // Duplicate $refs in an allOf (redundant manual aggregation + // along the chain) are allowed — re-merging the same base is + // idempotent. True cycles are caught by DFS-path detection in + // the $ref branch below. + resolved := s.resolveRefsInner(item, visited, cycleFound) if resolvedMap, ok := resolved.(map[string]any); ok { if _, stillHasRef := resolvedMap["$ref"]; stillHasRef { resolvedAllOf = append(resolvedAllOf, resolved) @@ -980,12 +978,20 @@ func (s *GtsStore) resolveRefsInner(schema any, visited map[string]bool, cycleFo } for k, val := range resolvedMap { switch k { - case "properties", "required", "$id", "$schema": + case "properties", "required", "$id", "$schema", "additionalProperties": + // additionalProperties is never hoisted out of an + // allOf branch: allOf is a conjunction, so a branch's + // additionalProperties applies only over that branch's + // own properties (draft-07 §6.5.6). Hoisting an + // overlay's `additionalProperties: true` to the merged + // root would wrongly read as the derived schema opening + // up, when the closed base branch still denies extras. + // Closedness is carried by the base's own + // additionalProperties:false via the $ref/allOf + // composition. Only the derived schema's *root-level* + // additionalProperties (a sibling of allOf, copied + // below) is honored for OP#12. continue - case "additionalProperties": - if itemWasRef { - continue - } } mergedOther[k] = val } @@ -999,7 +1005,14 @@ func (s *GtsStore) resolveRefsInner(schema any, visited map[string]bool, cycleFo merged := make(map[string]any) for k, val := range v { if k != "allOf" { - merged[k] = val + // Resolve nested $refs in sibling keys too. A derived + // schema may carry its own x-gts-traits-schema next to + // allOf, and that subschema may itself $ref a standalone + // trait-schema type. Copying it verbatim would leave that + // $ref unresolved and trip findUnresolvedRef, even though + // OP#12 ignores x-gts-* keys. (OP#13 still resolves trait + // schemas separately from the raw content.) + merged[k] = s.resolveRefsInner(val, visited, cycleFound) } } for k, val := range mergedOther { // parent keys take precedence @@ -1040,14 +1053,14 @@ func (s *GtsStore) resolveRefsInner(schema any, visited map[string]bool, cycleFo result := make(map[string]any) for k, val := range v { - result[k] = s.resolveRefsInner(val, visited, cycleFound, dupFound) + result[k] = s.resolveRefsInner(val, visited, cycleFound) } return result case []any: result := make([]any, len(v)) for i, item := range v { - result[i] = s.resolveRefsInner(item, visited, cycleFound, dupFound) + result[i] = s.resolveRefsInner(item, visited, cycleFound) } return result diff --git a/gts/schema_compat_test.go b/gts/schema_compat_test.go index bce5f4f..04a6993 100644 --- a/gts/schema_compat_test.go +++ b/gts/schema_compat_test.go @@ -580,7 +580,11 @@ func TestValidateSchemaCompatibility(t *testing.T) { wantError: false, }, { - name: "derived loosens additionalProperties from false", + // Omitting additionalProperties on derived is NOT loosening: the + // base's additionalProperties:false is inherited via $ref/allOf + // composition (ADR-0001 / draft-07 §6.5.6). Only an explicit + // permissive declaration at derived's root loosens. + name: "derived omits additionalProperties (inherits closedness)", base: &effectiveSchema{ properties: map[string]any{}, required: map[string]bool{}, @@ -593,6 +597,22 @@ func TestValidateSchemaCompatibility(t *testing.T) { requiredSet: false, additionalProperties: nil, }, + wantError: false, + }, + { + name: "derived explicitly sets additionalProperties:true (loosens)", + base: &effectiveSchema{ + properties: map[string]any{}, + required: map[string]bool{}, + requiredSet: false, + additionalProperties: false, + }, + derived: &effectiveSchema{ + properties: map[string]any{}, + required: map[string]bool{}, + requiredSet: false, + additionalProperties: true, + }, wantError: true, }, { @@ -746,7 +766,7 @@ func TestValidateSchemaChain_TwoLevel_TypeChange(t *testing.T) { } } -func TestValidateSchemaChain_TwoLevel_LoosensAdditionalProperties(t *testing.T) { +func TestValidateSchemaChain_TwoLevel_ExplicitlyLoosensAdditionalProperties(t *testing.T) { store := NewGtsStore(nil) mustRegister(t, store, map[string]any{ "$id": "gts.x.chain.ns.closed.v1~", @@ -756,17 +776,45 @@ func TestValidateSchemaChain_TwoLevel_LoosensAdditionalProperties(t *testing.T) "id": map[string]any{"type": "string"}, }, }) - // Derived omits additionalProperties:false — loosening + // Derived *explicitly* sets additionalProperties:true at its own root — this + // is the only case that loosens (ADR-0001). Merely omitting the keyword is + // not loosening, because closedness is inherited via $ref/allOf composition. mustRegister(t, store, map[string]any{ - "$id": "gts.x.chain.ns.closed.v1~x.chain.ns.open.v1~", - "type": "object", - "properties": map[string]any{ - "id": map[string]any{"type": "string"}, + "$id": "gts.x.chain.ns.closed.v1~x.chain.ns.open.v1~", + "type": "object", + "additionalProperties": true, + "allOf": []any{ + map[string]any{"$ref": "gts://gts.x.chain.ns.closed.v1~"}, }, }) result := store.ValidateSchemaChain("gts.x.chain.ns.closed.v1~x.chain.ns.open.v1~") if result.OK { - t.Error("expected failure when derived loosens additionalProperties") + t.Error("expected failure when derived explicitly loosens additionalProperties to true") + } +} + +func TestValidateSchemaChain_TwoLevel_OmitsAdditionalProperties_InheritsClosedness(t *testing.T) { + store := NewGtsStore(nil) + mustRegister(t, store, map[string]any{ + "$id": "gts.x.chain.ns.closed2.v1~", + "type": "object", + "additionalProperties": false, + "properties": map[string]any{ + "id": map[string]any{"type": "string"}, + }, + }) + // Derived as allOf:[{$ref: base}] omitting additionalProperties — closedness + // flows through the $ref; this is NOT loosening (ADR-0001 / 9a086c0). + mustRegister(t, store, map[string]any{ + "$id": "gts.x.chain.ns.closed2.v1~x.chain.ns.omit.v1~", + "type": "object", + "allOf": []any{ + map[string]any{"$ref": "gts://gts.x.chain.ns.closed2.v1~"}, + }, + }) + result := store.ValidateSchemaChain("gts.x.chain.ns.closed2.v1~x.chain.ns.omit.v1~") + if !result.OK { + t.Errorf("omitting additionalProperties (closedness inherited via $ref) must pass, got: %s", result.Error) } } diff --git a/gts/schema_modifiers.go b/gts/schema_modifiers.go index f28c7af..7a16b3a 100644 --- a/gts/schema_modifiers.go +++ b/gts/schema_modifiers.go @@ -41,37 +41,108 @@ func readBoolModifier(content map[string]any, key string) (bool, error) { return b, nil } +// validateModifierPlacement enforces that x-gts-final / x-gts-abstract appear +// only at the schema document top level (gts-spec §9.11). They are type-level +// keywords describing the GTS Type as a whole; nesting either inside ANY +// subschema (allOf, properties, $defs/definitions, items, combinators, …) is a +// misplacement and is rejected rather than silently ignored. func validateModifierPlacement(content map[string]any) error { - allOf, ok := content["allOf"].([]any) - if !ok { - return nil - } - for _, item := range allOf { - entry, ok := item.(map[string]any) - if !ok { + for k, v := range content { + if k == KeyXGtsFinal || k == KeyXGtsAbstract { continue } - if _, has := entry[KeyXGtsFinal]; has { - return fmt.Errorf("%s must be at the schema top level, not inside allOf", KeyXGtsFinal) + if containsKeyInValue(v, KeyXGtsFinal) { + return fmt.Errorf("%s must be at the schema top level", KeyXGtsFinal) + } + if containsKeyInValue(v, KeyXGtsAbstract) { + return fmt.Errorf("%s must be at the schema top level", KeyXGtsAbstract) + } + } + return nil +} + +// ValidateTraitPlacement enforces that x-gts-traits and x-gts-traits-schema +// appear only at the schema document top level (gts-spec §9.7.1/§9.11). Like +// the modifiers, these are type-level keywords; nesting either inside a +// subschema (allOf, properties, $defs/definitions, combinators, items, …) is a +// misplacement and is rejected (fail fast). +// +// The rule constrains only the *position* of the keyword, not the *contents* +// of its value: the top-level x-gts-traits-schema is an ordinary JSON Schema +// subschema whose body may legitimately carry x-gts-* members (e.g. when an +// existing GTS type is reused as a trait-schema source via $ref). The top-level +// keyword values are therefore not re-scanned. +func ValidateTraitPlacement(content map[string]any) error { + for k, v := range content { + // The four document-level keyword slots are allowed at the top level; + // their own values are not re-scanned (see doc comment). + if k == KeyXGtsFinal || k == KeyXGtsAbstract || k == KeyXGtsTraits || k == KeyXGtsTraitsSchema { + continue } - if _, has := entry[KeyXGtsAbstract]; has { - return fmt.Errorf("%s must be at the schema top level, not inside allOf", KeyXGtsAbstract) + if containsKeyInValue(v, KeyXGtsTraitsSchema) { + return fmt.Errorf("%s must be at the schema top level", KeyXGtsTraitsSchema) } - if err := validateModifierPlacement(entry); err != nil { - return err + if containsKeyInValue(v, KeyXGtsTraits) { + return fmt.Errorf("%s must be at the schema top level", KeyXGtsTraits) } } return nil } -// ValidateInstanceModifiers checks that schema-only keywords (x-gts-final, x-gts-abstract) -// do not appear in instance content. +// schemaOnlyInstanceKeywords lists the x-gts-* keywords that are valid only on +// type-schema documents and MUST be rejected when found inside instance +// documents (gts-spec §9.7.1, §9.11.1). +var schemaOnlyInstanceKeywords = []string{ + KeyXGtsFinal, + KeyXGtsAbstract, + KeyXGtsTraitsSchema, + KeyXGtsTraits, +} + +// ValidateInstanceModifiers checks that schema-only keywords (x-gts-final, +// x-gts-abstract, x-gts-traits-schema, x-gts-traits) do not appear anywhere +// in instance content. Per gts-spec §9.7.1 / §9.11.1 these annotations are +// only valid on JSON Schema (type-schema) documents and implementations MUST +// reject instances that contain them. +// +// The check is recursive over both objects and arrays so a stray keyword +// nested under any property is also flagged. func ValidateInstanceModifiers(content map[string]any) error { - if _, ok := content[KeyXGtsFinal]; ok { - return fmt.Errorf("%s is a schema-only keyword and must not appear in instances", KeyXGtsFinal) - } - if _, ok := content[KeyXGtsAbstract]; ok { - return fmt.Errorf("%s is a schema-only keyword and must not appear in instances", KeyXGtsAbstract) + for _, key := range schemaOnlyInstanceKeywords { + if containsKeyRecursive(content, key) { + return fmt.Errorf("%s is a schema-only keyword and must not appear in instances", key) + } } return nil } + +// containsKeyRecursive reports whether the given key appears as a key in the +// content or anywhere in its nested objects/arrays. +func containsKeyRecursive(content map[string]any, key string) bool { + if content == nil { + return false + } + if _, ok := content[key]; ok { + return true + } + for _, v := range content { + if containsKeyInValue(v, key) { + return true + } + } + return false +} + +func containsKeyInValue(v any, key string) bool { + switch vv := v.(type) { + case map[string]any: + return containsKeyRecursive(vv, key) + case []any: + for _, item := range vv { + if containsKeyInValue(item, key) { + return true + } + } + } + return false +} diff --git a/gts/schema_modifiers_test.go b/gts/schema_modifiers_test.go index ba3e94f..bdd3184 100644 --- a/gts/schema_modifiers_test.go +++ b/gts/schema_modifiers_test.go @@ -166,6 +166,48 @@ func TestValidateInstanceModifiers_HasAbstract(t *testing.T) { } } +func TestValidateInstanceModifiers_HasTraits(t *testing.T) { + err := ValidateInstanceModifiers(map[string]any{ + "id": "test", + "x-gts-traits": map[string]any{"retention": "P30D"}, + }) + if err == nil { + t.Fatal("expected error for x-gts-traits in instance") + } + if !strings.Contains(err.Error(), "x-gts-traits") || !strings.Contains(err.Error(), "schema-only") { + t.Errorf("expected schema-only x-gts-traits error, got: %v", err) + } +} + +func TestValidateInstanceModifiers_HasTraitsSchema(t *testing.T) { + err := ValidateInstanceModifiers(map[string]any{ + "id": "test", + "x-gts-traits-schema": map[string]any{ + "type": "object", + "properties": map[string]any{"retention": map[string]any{"type": "string"}}, + }, + }) + if err == nil { + t.Fatal("expected error for x-gts-traits-schema in instance") + } + if !strings.Contains(err.Error(), "x-gts-traits-schema") { + t.Errorf("expected schema-only x-gts-traits-schema error, got: %v", err) + } +} + +func TestValidateInstanceModifiers_NestedTraits(t *testing.T) { + err := ValidateInstanceModifiers(map[string]any{ + "id": "test", + "metadata": map[string]any{"x-gts-traits": map[string]any{"foo": "bar"}}, + }) + if err == nil { + t.Fatal("expected error for nested x-gts-traits") + } + if !strings.Contains(err.Error(), "x-gts-traits") { + t.Errorf("expected nested x-gts-traits error, got: %v", err) + } +} + // ============================================================================= // x-gts-final integration tests // ============================================================================= @@ -691,3 +733,81 @@ func TestAbstract_NonBooleanRejected(t *testing.T) { t.Error("non-boolean x-gts-abstract should fail entity validation") } } + +// ============================================================================= +// ValidateTraitPlacement (gts-spec §9.7.1/§9.11) +// ============================================================================= + +func TestValidateTraitPlacement_TopLevelOk(t *testing.T) { + err := ValidateTraitPlacement(map[string]any{ + "type": "object", + "x-gts-traits-schema": map[string]any{ + "type": "object", + "properties": map[string]any{"topicRef": map[string]any{"type": "string"}}, + }, + "x-gts-traits": map[string]any{"topicRef": "events.orders"}, + "allOf": []any{map[string]any{"$ref": "gts.x.foo.base.v1~"}}, + }) + if err != nil { + t.Errorf("top-level trait keywords must be accepted, got: %v", err) + } +} + +func TestValidateTraitPlacement_InsideAllOfRejected(t *testing.T) { + err := ValidateTraitPlacement(map[string]any{ + "type": "object", + "allOf": []any{ + map[string]any{"$ref": "gts.x.foo.base.v1~"}, + map[string]any{"type": "object", "x-gts-traits": map[string]any{"topicRef": "x"}}, + }, + }) + if err == nil { + t.Error("x-gts-traits nested inside allOf must be rejected") + } +} + +func TestValidateTraitPlacement_InsidePropertiesRejected(t *testing.T) { + err := ValidateTraitPlacement(map[string]any{ + "type": "object", + "properties": map[string]any{ + "nested": map[string]any{"type": "object", "x-gts-traits-schema": map[string]any{"type": "object"}}, + }, + }) + if err == nil { + t.Error("x-gts-traits-schema nested inside properties must be rejected") + } +} + +func TestValidateTraitPlacement_KeysInsideTraitSchemaValueTolerated(t *testing.T) { + // The contents of the top-level x-gts-traits-schema are an ordinary JSON + // Schema subschema and may carry x-gts-* members (e.g. a $ref-reused GTS + // type). The placement rule constrains only the keyword's position. + err := ValidateTraitPlacement(map[string]any{ + "type": "object", + "x-gts-traits-schema": map[string]any{ + "type": "object", + "x-gts-traits-schema": map[string]any{"type": "object"}, + "x-gts-traits": map[string]any{"foo": "bar"}, + "properties": map[string]any{"retention": map[string]any{"type": "string"}}, + }, + "x-gts-traits": map[string]any{"retention": "P30D"}, + }) + if err != nil { + t.Errorf("x-gts-* nested inside the trait-schema value must be tolerated, got: %v", err) + } +} + +func TestValidateTraitPlacement_KeysInsideTraitValuesTolerated(t *testing.T) { + // The top-level x-gts-traits holds trait *values* matched against the + // effective trait-schema, not a subschema. A member keyed `x-gts-traits` + // nested inside those values is ordinary data, not a misplaced keyword. + err := ValidateTraitPlacement(map[string]any{ + "type": "object", + "x-gts-traits": map[string]any{ + "nested": map[string]any{"x-gts-traits": map[string]any{}}, + }, + }) + if err != nil { + t.Errorf("x-gts-traits nested inside trait values must be tolerated, got: %v", err) + } +} diff --git a/gts/schema_traits.go b/gts/schema_traits.go index 7d47584..c71d595 100644 --- a/gts/schema_traits.go +++ b/gts/schema_traits.go @@ -26,6 +26,16 @@ import ( "github.com/santhosh-tekuri/jsonschema/v6" ) +// KeyXGtsTraitsSchema is the JSON Schema annotation keyword that defines the +// shape of trait properties available to a GTS type and its descendants. +// Schema-only — MUST NOT appear in instances (see gts-spec §9.7.1). +const KeyXGtsTraitsSchema = "x-gts-traits-schema" + +// KeyXGtsTraits is the JSON Schema annotation keyword that supplies concrete +// values for trait properties declared via KeyXGtsTraitsSchema. Schema-only — +// MUST NOT appear in instances (see gts-spec §9.7.1). +const KeyXGtsTraits = "x-gts-traits" + const maxTraitsRecursionDepth = 64 // walkAllOf calls fn on the given schema and recursively on every item inside its allOf array. @@ -46,23 +56,27 @@ func walkAllOf(value map[string]any, depth int, fn func(map[string]any)) { // collectTraitSchemaFromValue recursively searches a schema value for x-gts-traits-schema entries. // Handles both top-level and allOf-nested occurrences. -func collectTraitSchemaFromValue(value map[string]any, out *[]map[string]any, depth int) { +// +// Each x-gts-traits-schema is an ordinary JSON Schema subschema (ADR-0002): its +// value MAY be a JSON object, the boolean `true` (any traits pass), or the +// boolean `false` (no traits permitted). Objects are collected as map[string]any, +// booleans as bool; any other JSON type is collected verbatim so it can be +// rejected later as an invalid subschema form. +func collectTraitSchemaFromValue(value map[string]any, out *[]any, depth int) { walkAllOf(value, depth, func(node map[string]any) { - if ts, ok := node["x-gts-traits-schema"]; ok { - if tsMap, ok := ts.(map[string]any); ok { - *out = append(*out, tsMap) - } else { - // Non-object trait schema — still collect it as a sentinel (nil) to signal presence - *out = append(*out, nil) - } + if ts, ok := node[KeyXGtsTraitsSchema]; ok { + *out = append(*out, ts) } }) } // collectTraitsFromValue recursively searches a schema value for x-gts-traits entries and merges them. +// +// `null` values are preserved verbatim — they carry RFC 7396 "delete this key" +// semantics and must reach the cross-level merge step (mergeRFC7396Into) intact. func collectTraitsFromValue(value map[string]any, merged map[string]any, depth int) { walkAllOf(value, depth, func(node map[string]any) { - if traits, ok := node["x-gts-traits"].(map[string]any); ok { + if traits, ok := node[KeyXGtsTraits].(map[string]any); ok { for k, v := range traits { merged[k] = v } @@ -70,22 +84,82 @@ func collectTraitsFromValue(value map[string]any, merged map[string]any, depth i }) } -// buildEffectiveTraitSchema composes all collected trait schemas using allOf. -func buildEffectiveTraitSchema(schemas []map[string]any) map[string]any { - switch len(schemas) { +// mergeRFC7396Into merges patch into target per RFC 7396 (JSON Merge Patch), +// used to compose x-gts-traits along the $id chain (root → leaf). +// +// Semantics: +// - a `null` patch value deletes the corresponding key from target; +// - an object patch value merges recursively (keys not restated by patch are +// preserved), replacing wholesale when target holds a non-object; +// - any other value (scalar or array) replaces the existing value wholesale. +// +// Object patch values are deep-copied on insert so stored entity content is +// never mutated through shared map references. +// +// Recursion over nested objects is bounded by maxTraitsRecursionDepth to +// prevent stack overflow on deeply-nested (or maliciously crafted) trait +// values supplied via x-gts-traits. +func mergeRFC7396Into(target map[string]any, patch map[string]any) { + mergeRFC7396Recursive(target, patch, 0) +} + +func mergeRFC7396Recursive(target map[string]any, patch map[string]any, depth int) { + if depth >= maxTraitsRecursionDepth { + return + } + for k, v := range patch { + switch pv := v.(type) { + case nil: + delete(target, k) + case map[string]any: + if existing, ok := target[k].(map[string]any); ok { + mergeRFC7396Recursive(existing, pv, depth+1) + } else { + fresh := make(map[string]any) + mergeRFC7396Recursive(fresh, pv, depth+1) + target[k] = fresh + } + default: + target[k] = pv + } + } +} + +// buildEffectiveTraitSchema composes all collected trait schemas via allOf +// (ADR-0002 chain aggregation). Each element is a JSON Schema subschema — an +// object, the boolean `true`, or the boolean `false`. +// +// Returns: +// - the boolean `false` if any subschema along the chain is `false` +// (allOf(false, …) ≡ false — the effective schema is unsatisfiable); +// - otherwise an object schema: `true` subschemas are identity elements and +// dropped, object subschemas are composed via allOf (a single object is +// returned directly, none yields the empty/accept-all schema {}). +func buildEffectiveTraitSchema(schemas []any) any { + objs := make([]map[string]any, 0, len(schemas)) + for _, s := range schemas { + // A `false` anywhere in the chain (a bare boolean, or nested inside an + // allOf) makes the composed schema unsatisfiable — allOf(false, …) ≡ false. + if schemaIsFalse(s, 0) { + return false + } + switch v := s.(type) { + case bool: + // `true` is the identity element under allOf — contributes nothing. + case map[string]any: + objs = append(objs, v) + } + } + + switch len(objs) { case 0: return map[string]any{} case 1: - if schemas[0] == nil { - return map[string]any{} - } - return schemas[0] + return objs[0] default: - allOf := make([]any, 0, len(schemas)) - for _, s := range schemas { - if s != nil { - allOf = append(allOf, s) - } + allOf := make([]any, len(objs)) + for i, o := range objs { + allOf[i] = o } return map[string]any{ "type": "object", @@ -94,6 +168,30 @@ func buildEffectiveTraitSchema(schemas []map[string]any) map[string]any { } } +// schemaIsFalse reports whether a JSON Schema subschema is the unsatisfiable +// schema: the boolean `false`, or an object whose `allOf` contains (recursively) +// a `false`. Under allOf composition such a subschema rejects every value, which +// GTS treats as the "traits prohibited" signal (ADR-0002). Recursion is bounded +// by maxTraitsRecursionDepth. +func schemaIsFalse(v any, depth int) bool { + if depth >= maxTraitsRecursionDepth { + return false + } + switch s := v.(type) { + case bool: + return !s + case map[string]any: + if allOf, ok := s["allOf"].([]any); ok { + for _, item := range allOf { + if schemaIsFalse(item, depth+1) { + return true + } + } + } + } + return false +} + type namedProp struct { name string schema map[string]any @@ -142,6 +240,36 @@ func collectAllProperties(schema map[string]any, depth int) []namedProp { return result } +// collectAllRequired returns the union of `required` property names declared at +// the top level or within any allOf branch of the (effective) trait schema. +// Mirrors collectAllProperties so completeness enforcement matches JSON Schema's +// own `required` aggregation across the composed chain. +func collectAllRequired(schema map[string]any, depth int) map[string]bool { + req := make(map[string]bool) + var collect func(s map[string]any, d int) + collect = func(s map[string]any, d int) { + if d >= maxTraitsRecursionDepth { + return + } + if required, ok := s["required"].([]any); ok { + for _, item := range required { + if name, ok := item.(string); ok { + req[name] = true + } + } + } + if allOf, ok := s["allOf"].([]any); ok { + for _, item := range allOf { + if sub, ok := item.(map[string]any); ok { + collect(sub, d+1) + } + } + } + } + collect(schema, depth) + return req +} + // applyDefaults applies JSON Schema default values from the effective trait schema // to the merged traits object for any properties that are not yet present. func applyDefaults(traitSchema map[string]any, traits map[string]any, depth int) map[string]any { @@ -224,26 +352,26 @@ func validateTraitsAgainstSchema(traitSchema map[string]any, effectiveTraits map return errors } - // Check for unresolved (missing) trait properties that have no default, - // recursing into nested object sub-properties. - errors = append(errors, checkUnresolvedProps(traitSchema, effectiveTraits, "")...) + // Check that every *required* trait property is resolved. + errors = append(errors, checkUnresolvedProps(traitSchema, effectiveTraits)...) return errors } -// checkUnresolvedProps recursively checks that all trait properties have either a value -// in traits or a default in the schema. Per spec §9.7.5: "if a trait is required by the -// effective trait schema (i.e., not covered by a default) but is not provided by any -// x-gts-traits in the chain, schema validation MUST fail". A property without a default -// is implicitly required regardless of the JSON Schema 'required' array. -func checkUnresolvedProps(schema map[string]any, traits map[string]any, prefix string) []string { +// checkUnresolvedProps checks that every *required* trait property has either a +// value in traits or a `default` in the schema. Per ADR-0003 / README §9.7.5, +// completeness is keyed on the effective trait schema's `required` set: optional +// declared properties MAY be left unresolved. Standard JSON Schema validation +// (run by the caller) already reports missing required members against the +// materialized object; this loop adds a type-annotated, trait-specific message. +func checkUnresolvedProps(schema map[string]any, traits map[string]any) []string { var errors []string + required := collectAllRequired(schema, 0) for _, p := range collectAllProperties(schema, 0) { - fullName := p.name - if prefix != "" { - fullName = prefix + "." + p.name + if !required[p.name] { + continue } - val, hasValue := traits[p.name] + _, hasValue := traits[p.name] _, hasDefault := p.schema["default"] if !hasValue && !hasDefault { propType, _ := p.schema["type"].(string) @@ -252,35 +380,13 @@ func checkUnresolvedProps(schema map[string]any, traits map[string]any, prefix s } errors = append(errors, fmt.Sprintf( "trait property '%s' (type: %s) is not resolved: no value provided and no default defined in the trait schema", - fullName, propType, + p.name, propType, )) - } else if p.schema["type"] == "object" { - if _, hasProps := p.schema["properties"]; hasProps { - var subTraits map[string]any - if valMap, ok := val.(map[string]any); ok { - subTraits = valMap - } else { - subTraits = map[string]any{} - } - errors = append(errors, checkUnresolvedProps(p.schema, subTraits, fullName)...) - } } } return errors } -// containsXGtsTraits reports whether a schema map contains an 'x-gts-traits' key -// at its top level or nested inside any allOf items (recursively). -func containsXGtsTraits(schema map[string]any) bool { - found := false - walkAllOf(schema, 0, func(node map[string]any) { - if _, ok := node["x-gts-traits"]; ok { - found = true - } - }) - return found -} - // removeXGtsFields removes x-gts-* extension fields from a schema recursively. func removeXGtsFields(schema map[string]any) map[string]any { return walkSchema(schema, nil, func(k string) bool { @@ -290,9 +396,9 @@ func removeXGtsFields(schema map[string]any) map[string]any { // ValidateSchemaTraitsResult is the result of OP#13 schema traits validation. type ValidateSchemaTraitsResult struct { - SchemaID string `json:"schema_id"` - OK bool `json:"ok"` - Error string `json:"error,omitempty"` + TypeID string `json:"type_id"` + OK bool `json:"ok"` + Error string `json:"error,omitempty"` } // ValidateSchemaTraits validates schema traits across the inheritance chain (OP#13). @@ -302,24 +408,21 @@ func (s *GtsStore) ValidateSchemaTraits(schemaID string) *ValidateSchemaTraitsRe gid, err := NewGtsID(schemaID) if err != nil { return &ValidateSchemaTraitsResult{ - SchemaID: schemaID, - OK: false, - Error: fmt.Sprintf("Invalid GTS ID: %v", err), + TypeID: schemaID, + OK: false, + Error: fmt.Sprintf("Invalid GTS ID: %v", err), } } segments := gid.Segments - // levelInfo holds per-level data collected during the chain walk. - type levelInfo struct { - segSchemaID string - rawSchemas []map[string]any // raw (unresolved) trait schemas from this level - traits map[string]any // x-gts-traits collected from this level - } - - // Pass 1: walk the chain and collect raw trait schemas and trait values per level. - var allLevels []levelInfo - var traitSchemas []map[string]any + // Pass 1: walk the chain root → leaf, collecting x-gts-traits-schema + // subschemas (composed via allOf per ADR-0002) and merging x-gts-traits + // values per RFC 7396 JSON Merge Patch (ADR-0004). Publishers lock values + // via standard JSON Schema `const`; the registry carries no GTS-specific + // immutability rule. + var traitSchemas []any + mergedTraits := make(map[string]any) for i := range segments { segSchemaID := buildIDFromSegments(segments[:i+1]) @@ -327,200 +430,118 @@ func (s *GtsStore) ValidateSchemaTraits(schemaID string) *ValidateSchemaTraitsRe entity := s.Get(segSchemaID) if entity == nil { return &ValidateSchemaTraitsResult{ - SchemaID: schemaID, - OK: false, - Error: fmt.Sprintf("Schema '%s' not found for trait validation", segSchemaID), + TypeID: schemaID, + OK: false, + Error: fmt.Sprintf("Schema '%s' not found for trait validation", segSchemaID), } } content := entity.Content - var rawSchemas []map[string]any - collectTraitSchemaFromValue(content, &rawSchemas, 0) - traitSchemas = append(traitSchemas, rawSchemas...) + collectTraitSchemaFromValue(content, &traitSchemas, 0) levelTraits := make(map[string]any) collectTraitsFromValue(content, levelTraits, 0) - - allLevels = append(allLevels, levelInfo{ - segSchemaID: segSchemaID, - rawSchemas: rawSchemas, - traits: levelTraits, - }) + mergeRFC7396Into(mergedTraits, levelTraits) } - // Pass 2: normalize $$ref and resolve $ref in all collected trait schemas. + // Pass 2: normalize $$ref and resolve $ref in object-form trait schemas so + // external/standalone trait shapes are inlined. Boolean subschemas pass + // through untouched. for i, ts := range traitSchemas { - if ts == nil { + tsMap, ok := ts.(map[string]any) + if !ok { continue } - normalized := normalizeDollarRefs(ts) + normalized := normalizeDollarRefs(tsMap) resolved, err := s.resolveRefs(normalized) if err != nil { return &ValidateSchemaTraitsResult{ - SchemaID: schemaID, - OK: false, - Error: fmt.Sprintf("Schema '%s' trait schema has %v", schemaID, err), + TypeID: schemaID, + OK: false, + Error: fmt.Sprintf("Schema '%s' trait schema has %v", schemaID, err), } } traitSchemas[i] = resolved } - // Build a per-level slice of resolved schemas (parallel to allLevels). - resolvedSchemasByLevel := make([][]map[string]any, len(allLevels)) - idx := 0 - for li, lv := range allLevels { - resolvedSchemasByLevel[li] = traitSchemas[idx : idx+len(lv.rawSchemas)] - idx += len(lv.rawSchemas) - } - - // Pass 3: run cross-level checks against resolved schemas, then merge traits. - mergedTraits := make(map[string]any) - lockedTraits := make(map[string]bool) - knownDefaults := make(map[string]any) - - for li, lv := range allLevels { - // Track which properties this level's resolved trait schemas introduce. - levelSchemaProps := make(map[string]bool) - for _, ts := range resolvedSchemasByLevel[li] { - if ts == nil { - continue - } - for _, p := range collectAllProperties(ts, 0) { - levelSchemaProps[p.name] = true - if newDefault, ok := p.schema["default"]; ok { - if oldDefault, exists := knownDefaults[p.name]; exists { - if !jsonEqual(oldDefault, newDefault) { - return &ValidateSchemaTraitsResult{ - SchemaID: schemaID, - OK: false, - Error: fmt.Sprintf( - "Schema '%s' trait validation failed: trait schema default for '%s' in '%s' overrides default set by ancestor", - schemaID, p.name, lv.segSchemaID, - ), - } - } - } else { - knownDefaults[p.name] = newDefault - } - } - } - } - - // Check for locked trait overrides BEFORE merging this level's values. - for k, v := range lv.traits { - if existing, exists := mergedTraits[k]; exists { - if lockedTraits[k] && !jsonEqual(existing, v) { - return &ValidateSchemaTraitsResult{ - SchemaID: schemaID, - OK: false, - Error: fmt.Sprintf( - "Schema '%s' trait validation failed: trait '%s' in '%s' overrides value set by ancestor", - schemaID, k, lv.segSchemaID, - ), - } - } - } - } - - // Merge level traits (rightmost wins). - for k, v := range lv.traits { - mergedTraits[k] = v - } - - // Lock trait values set at this level only when this level does NOT introduce - // a schema property for the key (via the resolved schema). - for k := range lv.traits { - if !levelSchemaProps[k] { - lockedTraits[k] = true - } + // Abstract types are "incomplete waiting for descendants" (ADR-0003): the + // trait completeness check is skipped for them. As in gts-rust, abstract + // leaves skip trait validation entirely — concrete descendants still run the + // full check on their own effective state. Keyed on the validated type's own + // x-gts-abstract, never on a registry-state-dependent "leaf" notion. + if leafEntity := s.Get(schemaID); leafEntity != nil { + if ab, isBool := leafEntity.Content[KeyXGtsAbstract].(bool); isBool && ab { + return &ValidateSchemaTraitsResult{TypeID: schemaID, OK: true} } } - // Check for x-gts-traits-schema integrity: must not contain x-gts-traits anywhere - // (including nested inside allOf items). - for i, ts := range traitSchemas { - if ts == nil { - // Non-object trait schema — will fail validation below - continue - } - if containsXGtsTraits(ts) { - return &ValidateSchemaTraitsResult{ - SchemaID: schemaID, - OK: false, - Error: fmt.Sprintf( - "x-gts-traits-schema[%d] contains 'x-gts-traits' — trait values must not appear inside a trait schema definition", - i, - ), - } - } - } - - // Check: if no trait schemas, but trait values exist → error hasTraitValues := len(mergedTraits) > 0 + + // No x-gts-traits-schema anywhere in the chain: trait values are meaningless. if len(traitSchemas) == 0 { if hasTraitValues { return &ValidateSchemaTraitsResult{ - SchemaID: schemaID, - OK: false, - Error: "x-gts-traits values provided but no x-gts-traits-schema is defined in the inheritance chain", + TypeID: schemaID, + OK: false, + Error: "x-gts-traits values provided but no x-gts-traits-schema is defined in the inheritance chain", } } - return &ValidateSchemaTraitsResult{SchemaID: schemaID, OK: true} + return &ValidateSchemaTraitsResult{TypeID: schemaID, OK: true} } - // Check for nil (non-object) trait schemas and enforce type:object + // Each x-gts-traits-schema is a JSON Schema subschema (ADR-0002): an object, + // `true`, or `false`. Reject any other JSON type. x-gts-* members nested + // inside an object subschema (e.g. a $ref-reused GTS type) are tolerated — + // they are unknown JSON Schema keywords and inert at validation time. for i, ts := range traitSchemas { - if ts == nil { - return &ValidateSchemaTraitsResult{ - SchemaID: schemaID, - OK: false, - Error: fmt.Sprintf("x-gts-traits-schema[%d] is not a valid JSON Schema object", i), - } - } - if t, _ := ts["type"].(string); t != "object" { + switch ts.(type) { + case bool, map[string]any: + default: return &ValidateSchemaTraitsResult{ - SchemaID: schemaID, - OK: false, - Error: fmt.Sprintf("x-gts-traits-schema[%d] must have \"type\": \"object\"", i), + TypeID: schemaID, + OK: false, + Error: fmt.Sprintf("x-gts-traits-schema[%d] must be an object subschema or a boolean", i), } } } - // Build effective trait schema - effectiveTraitSchema := buildEffectiveTraitSchema(traitSchemas) - - // Apply defaults - effectiveTraits := applyDefaults(effectiveTraitSchema, mergedTraits, 0) + // Build the effective trait schema by composing the chain via allOf. + effAny := buildEffectiveTraitSchema(traitSchemas) - // Check if the leaf schema (rightmost segment) is abstract. - // Abstract schemas are not leaf schemas, so trait validation is skipped entirely — - // they are not required to provide trait values or satisfy required constraints. - leafEntity := s.Get(schemaID) - isAbstractLeaf := false - if leafEntity != nil { - if val, ok := leafEntity.Content[KeyXGtsAbstract]; ok { - if ab, isBool := val.(bool); isBool && ab { - isAbstractLeaf = true + // A `false` somewhere in the chain makes the effective schema unsatisfiable: + // traits are prohibited on this host and its whole subtree. A type carrying + // no traits is still valid; any trait value fails. + if b, ok := effAny.(bool); ok && !b { + if hasTraitValues { + return &ValidateSchemaTraitsResult{ + TypeID: schemaID, + OK: false, + Error: "x-gts-traits-schema resolves to `false` in the chain — x-gts-traits values are prohibited", } } + return &ValidateSchemaTraitsResult{TypeID: schemaID, OK: true} } - if isAbstractLeaf { - return &ValidateSchemaTraitsResult{SchemaID: schemaID, OK: true} - } + effectiveTraitSchema, _ := effAny.(map[string]any) + + // Materialize: apply defaults from the effective trait schema for any + // properties not present after the chain merge. + effectiveTraits := applyDefaults(effectiveTraitSchema, mergedTraits, 0) - // Validate + // Validate the materialized effective traits against the effective trait + // schema, including the required-trait completeness check (this type is + // non-abstract — abstract types returned OK above). errs := validateTraitsAgainstSchema(effectiveTraitSchema, effectiveTraits, true) if len(errs) > 0 { return &ValidateSchemaTraitsResult{ - SchemaID: schemaID, - OK: false, - Error: fmt.Sprintf("Schema '%s' trait validation failed: %s", schemaID, strings.Join(errs, "; ")), + TypeID: schemaID, + OK: false, + Error: fmt.Sprintf("Schema '%s' trait validation failed: %s", schemaID, strings.Join(errs, "; ")), } } - return &ValidateSchemaTraitsResult{SchemaID: schemaID, OK: true} + return &ValidateSchemaTraitsResult{TypeID: schemaID, OK: true} } // walkSchema applies a key transform and a recursive map transform to every node in a schema. @@ -565,9 +586,19 @@ func normalizeDollarRefs(m map[string]any) map[string]any { }, nil) } -// validateEntityLevelTraits checks entity-level trait constraints: -// - If a trait schema is defined, trait values must be provided. -// - Each trait schema must be closed (additionalProperties: false). +// validateEntityLevelTraits is the OP#13 entity-level check applied on the +// /validate-entity path (NOT on /validate-type-schema). For a schema to be a +// valid standalone *entity*: +// - if any x-gts-traits-schema is declared along the chain, x-gts-traits +// values must be provided somewhere in the chain (an open trait surface +// with no values is an incomplete entity); and +// - every object-form x-gts-traits-schema must be closed +// (additionalProperties: false) — an open trait schema signals a type +// designed to be extended, not a deployable entity. +// +// Boolean trait subschemas (true/false) carry no additionalProperties and are +// not subject to the closedness check. This mirrors the type-schema-validation +// relaxations (ADR-0002/0003) while preserving the stricter entity contract. func (s *GtsStore) validateEntityLevelTraits(schemaID string) error { gid, err := NewGtsID(schemaID) if err != nil { @@ -575,7 +606,7 @@ func (s *GtsStore) validateEntityLevelTraits(schemaID string) error { } segments := gid.Segments - var rawTraitSchemas []map[string]any + var traitSchemas []any hasTraitValues := false for i := range segments { @@ -585,7 +616,7 @@ func (s *GtsStore) validateEntityLevelTraits(schemaID string) error { return fmt.Errorf("schema '%s' not found", segSchemaID) } content := entity.Content - collectTraitSchemaFromValue(content, &rawTraitSchemas, 0) + collectTraitSchemaFromValue(content, &traitSchemas, 0) levelTraits := make(map[string]any) collectTraitsFromValue(content, levelTraits, 0) if len(levelTraits) > 0 { @@ -593,35 +624,24 @@ func (s *GtsStore) validateEntityLevelTraits(schemaID string) error { } } - if len(rawTraitSchemas) == 0 { + if len(traitSchemas) == 0 { return nil } if !hasTraitValues { - return fmt.Errorf("entity defines x-gts-traits-schema but no x-gts-traits values are provided") - } - - // Resolve $refs before checking additionalProperties - traitSchemas := make([]map[string]any, 0, len(rawTraitSchemas)) - for _, ts := range rawTraitSchemas { - if ts == nil { - continue - } - normalized := normalizeDollarRefs(ts) - resolved, err := s.resolveRefs(normalized) - if err != nil { - return fmt.Errorf("entity trait schema has %v", err) - } - traitSchemas = append(traitSchemas, resolved) + return fmt.Errorf("Entity defines x-gts-traits-schema but no x-gts-traits values are provided") } for _, ts := range traitSchemas { - ap, hasAP := ts["additionalProperties"] - if !hasAP { - return fmt.Errorf("entity trait schema must set additionalProperties: false to be a valid standalone entity") + obj, ok := ts.(map[string]any) + if !ok { + // Boolean subschema — no additionalProperties to check. + continue } - if b, ok := ap.(bool); !ok || b { - return fmt.Errorf("entity trait schema must set additionalProperties: false to be a valid standalone entity") + if ap, hasAP := obj["additionalProperties"]; !hasAP { + return fmt.Errorf("Entity trait schema must set additionalProperties: false to be a valid standalone entity") + } else if b, isBool := ap.(bool); !isBool || b { + return fmt.Errorf("Entity trait schema must set additionalProperties: false to be a valid standalone entity") } } @@ -648,7 +668,7 @@ func (s *GtsStore) ValidateEntity(entityID string) *ValidateEntityResult { } } - if entity.IsSchema { + if entity.IsTypeSchema { // Validate schema modifiers (x-gts-final, x-gts-abstract): type, mutual exclusion, placement. if err := ValidateSchemaModifiers(entity.Content); err != nil { return &ValidateEntityResult{ @@ -659,6 +679,18 @@ func (s *GtsStore) ValidateEntity(entityID string) *ValidateEntityResult { } } + // Validate trait keyword placement (x-gts-traits, x-gts-traits-schema): + // these are type-level keywords and MUST appear only at the schema top + // level (gts-spec §9.7.1/§9.11). + if err := ValidateTraitPlacement(entity.Content); err != nil { + return &ValidateEntityResult{ + EntityID: entityID, + EntityType: "schema", + OK: false, + Error: err.Error(), + } + } + // For schemas: run OP#12 chain validation + OP#13 traits validation chainResult := s.ValidateSchemaChain(entityID) if !chainResult.OK { @@ -680,8 +712,9 @@ func (s *GtsStore) ValidateEntity(entityID string) *ValidateEntityResult { } } - // Entity-level trait check: schema must have trait values if it defines a trait schema, - // and all trait schemas must be closed (additionalProperties: false). + // Entity-level trait check (stricter than type-schema validation): a + // deployable standalone entity must provide trait values when a trait + // schema is declared, and its trait schemas must be closed. if err := s.validateEntityLevelTraits(entityID); err != nil { return &ValidateEntityResult{ EntityID: entityID, @@ -715,9 +748,9 @@ func (s *GtsStore) ValidateEntity(entityID string) *ValidateEntityResult { } } - // Also run OP#12 chain validation and OP#13 traits validation on the schema - if entity.SchemaID != "" { - chainResult := s.ValidateSchemaChain(entity.SchemaID) + // Also run OP#12 chain validation and OP#13 traits validation on the type-schema + if entity.TypeID != "" { + chainResult := s.ValidateSchemaChain(entity.TypeID) if !chainResult.OK { return &ValidateEntityResult{ EntityID: entityID, @@ -727,7 +760,7 @@ func (s *GtsStore) ValidateEntity(entityID string) *ValidateEntityResult { } } - traitsResult := s.ValidateSchemaTraits(entity.SchemaID) + traitsResult := s.ValidateSchemaTraits(entity.TypeID) if !traitsResult.OK { return &ValidateEntityResult{ EntityID: entityID, diff --git a/gts/schema_traits_test.go b/gts/schema_traits_test.go index 81e1936..4ef4a2a 100644 --- a/gts/schema_traits_test.go +++ b/gts/schema_traits_test.go @@ -110,24 +110,38 @@ func TestCollectTraitSchemaFromValue(t *testing.T) { }, }, } - var out []map[string]any + var out []any collectTraitSchemaFromValue(schema, &out, 0) if len(out) != 1 { t.Fatalf("expected 1 trait schema, got %d", len(out)) } - if out[0] == nil { - t.Error("expected non-nil trait schema") + if _, ok := out[0].(map[string]any); !ok { + t.Error("expected object trait schema") } }) - t.Run("non-object x-gts-traits-schema appends nil sentinel", func(t *testing.T) { + t.Run("boolean x-gts-traits-schema collected verbatim", func(t *testing.T) { + schema := map[string]any{ + "x-gts-traits-schema": false, + } + var out []any + collectTraitSchemaFromValue(schema, &out, 0) + if len(out) != 1 { + t.Fatalf("expected 1 collected subschema, got %d", len(out)) + } + if b, ok := out[0].(bool); !ok || b { + t.Errorf("expected boolean false subschema, got %v", out[0]) + } + }) + + t.Run("non-object non-bool x-gts-traits-schema collected verbatim", func(t *testing.T) { schema := map[string]any{ "x-gts-traits-schema": "invalid", } - var out []map[string]any + var out []any collectTraitSchemaFromValue(schema, &out, 0) - if len(out) != 1 || out[0] != nil { - t.Error("expected one nil sentinel for non-object trait schema") + if len(out) != 1 || out[0] != "invalid" { + t.Errorf("expected the raw value to be collected for later rejection, got %v", out) } }) @@ -142,7 +156,7 @@ func TestCollectTraitSchemaFromValue(t *testing.T) { }, }, } - var out []map[string]any + var out []any collectTraitSchemaFromValue(schema, &out, 0) if len(out) != 2 { t.Errorf("expected 2 trait schemas from allOf, got %d", len(out)) @@ -150,7 +164,7 @@ func TestCollectTraitSchemaFromValue(t *testing.T) { }) t.Run("no x-gts-traits-schema", func(t *testing.T) { - var out []map[string]any + var out []any collectTraitSchemaFromValue(map[string]any{"type": "object"}, &out, 0) if len(out) != 0 { t.Errorf("expected 0 trait schemas, got %d", len(out)) @@ -158,7 +172,7 @@ func TestCollectTraitSchemaFromValue(t *testing.T) { }) t.Run("depth limit stops recursion", func(t *testing.T) { - var out []map[string]any + var out []any collectTraitSchemaFromValue(map[string]any{"x-gts-traits-schema": map[string]any{}}, &out, maxTraitsRecursionDepth) if len(out) != 0 { t.Error("should not collect at max depth") @@ -223,62 +237,87 @@ func TestCollectTraitsFromValue(t *testing.T) { func TestBuildEffectiveTraitSchema(t *testing.T) { tests := []struct { name string - schemas []map[string]any + schemas []any wantEmpty bool wantAllOf bool wantDirect bool + wantFalse bool }{ { name: "empty input returns empty map", - schemas: []map[string]any{}, + schemas: []any{}, wantEmpty: true, }, { - name: "single nil returns empty map", - schemas: []map[string]any{nil}, + name: "single `true` (identity) returns empty map", + schemas: []any{true}, wantEmpty: true, }, { - name: "single non-nil returns it directly", - schemas: []map[string]any{{"type": "object"}}, + name: "single object returns it directly", + schemas: []any{map[string]any{"type": "object"}}, wantDirect: true, }, { name: "two schemas wrapped in allOf", - schemas: []map[string]any{{"type": "object"}, {"type": "object"}}, + schemas: []any{map[string]any{"type": "object"}, map[string]any{"type": "object"}}, wantAllOf: true, }, + { + name: "any `false` makes the effective schema unsatisfiable", + schemas: []any{map[string]any{"type": "object"}, false}, + wantFalse: true, + }, + { + name: "`true` is dropped, leaving a single object direct", + schemas: []any{true, map[string]any{"type": "object"}}, + wantDirect: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := buildEffectiveTraitSchema(tt.schemas) - if tt.wantEmpty && len(result) != 0 { - t.Errorf("expected empty map, got %v", result) + if tt.wantFalse { + if b, ok := result.(bool); !ok || b { + t.Errorf("expected boolean false, got %v", result) + } + return + } + m, ok := result.(map[string]any) + if !ok { + t.Fatalf("expected object schema, got %v", result) + } + if tt.wantEmpty && len(m) != 0 { + t.Errorf("expected empty map, got %v", m) } if tt.wantAllOf { - if _, ok := result["allOf"]; !ok { - t.Errorf("expected allOf key in result, got %v", result) + if _, ok := m["allOf"]; !ok { + t.Errorf("expected allOf key in result, got %v", m) } } if tt.wantDirect { - if _, ok := result["type"]; !ok { - t.Errorf("expected direct schema pass-through, got %v", result) + if _, ok := m["type"]; !ok { + t.Errorf("expected direct schema pass-through, got %v", m) } } }) } } -func TestBuildEffectiveTraitSchema_NilsInMultiple(t *testing.T) { - // Nils mixed with valid schemas: nils should be filtered out of allOf - schemas := []map[string]any{nil, {"type": "object"}, nil, {"properties": map[string]any{}}} +func TestBuildEffectiveTraitSchema_TrueDropsFromMultiple(t *testing.T) { + // `true` subschemas (identity under allOf) are dropped; objects compose. + schemas := []any{true, map[string]any{"type": "object"}, true, map[string]any{"properties": map[string]any{}}} result := buildEffectiveTraitSchema(schemas) - allOf, ok := result["allOf"].([]any) + m, ok := result.(map[string]any) if !ok { - t.Fatalf("expected allOf, got %v", result) + t.Fatalf("expected object schema, got %v", result) + } + allOf, ok := m["allOf"].([]any) + if !ok { + t.Fatalf("expected allOf, got %v", m) } if len(allOf) != 2 { - t.Errorf("expected 2 items in allOf (nils filtered), got %d", len(allOf)) + t.Errorf("expected 2 items in allOf (`true`s dropped), got %d", len(allOf)) } } @@ -488,29 +527,48 @@ func TestValidateTraitsAgainstSchema(t *testing.T) { } }) - t.Run("checkUnresolved flags unresolved properties without defaults", func(t *testing.T) { + t.Run("checkUnresolved flags unresolved REQUIRED properties without defaults", func(t *testing.T) { traitSchema := map[string]any{ "type": "object", "properties": map[string]any{ "unresolved": map[string]any{"type": "string"}, }, + "required": []any{"unresolved"}, } errs := validateTraitsAgainstSchema(traitSchema, map[string]any{}, true) if len(errs) == 0 { - t.Error("expected error for unresolved trait without default") + t.Error("expected error for unresolved required trait without default") } }) - t.Run("unresolved property with default is ok", func(t *testing.T) { + t.Run("unresolved OPTIONAL property passes completeness", func(t *testing.T) { + // Per ADR-0003 / README §9.7.5, completeness is keyed on `required`. + // An optional declared property left unresolved is spec-valid. traitSchema := map[string]any{ "type": "object", "properties": map[string]any{ - "color": map[string]any{"type": "string", "default": "blue"}, + "note": map[string]any{"type": "string"}, }, } errs := validateTraitsAgainstSchema(traitSchema, map[string]any{}, true) if len(errs) != 0 { - t.Errorf("property with default should not be flagged as unresolved, got %v", errs) + t.Errorf("optional unresolved property must not fail completeness, got %v", errs) + } + }) + + t.Run("required property satisfied by materialized default is ok", func(t *testing.T) { + // In the full pipeline applyDefaults materializes the default before this + // check runs; here we pass the materialized value to mirror that. + traitSchema := map[string]any{ + "type": "object", + "properties": map[string]any{ + "color": map[string]any{"type": "string", "default": "blue"}, + }, + "required": []any{"color"}, + } + errs := validateTraitsAgainstSchema(traitSchema, map[string]any{"color": "blue"}, true) + if len(errs) != 0 { + t.Errorf("required property satisfied by default should not fail, got %v", errs) } }) } @@ -697,48 +755,90 @@ func TestValidateSchemaTraits_InheritedTraitSchema_TwoLevel(t *testing.T) { } } -func TestValidateSchemaTraits_TraitSchemaContainsTraitValues_Fails(t *testing.T) { +func TestValidateSchemaTraits_XGtsKeysInsideTraitSchema_Tolerated(t *testing.T) { + // A trait-schema may carry x-gts-* members as ordinary JSON Schema keywords + // (e.g. when an existing GTS type is reused as a trait-schema source via + // $ref). These are unknown keywords to a JSON Schema validator and must NOT + // cause registration to fail (ADR-0002). The trait-schema below declares an + // optional property and the chain supplies a matching value. store := NewGtsStore(nil) mustRegisterTraits(t, store, map[string]any{ "$id": "gts.x.traits.ns.selfcontained.v1~", "type": "object", "x-gts-traits-schema": map[string]any{ - "type": "object", - "x-gts-traits": map[string]any{"color": "red"}, + "type": "object", + "x-gts-traits-schema": map[string]any{"type": "object"}, + "x-gts-traits": map[string]any{"foo": "bar"}, "properties": map[string]any{ "color": map[string]any{"type": "string"}, }, - "additionalProperties": false, }, "x-gts-traits": map[string]any{"color": "red"}, }) result := store.ValidateSchemaTraits("gts.x.traits.ns.selfcontained.v1~") - if result.OK { - t.Error("expected failure: x-gts-traits inside x-gts-traits-schema is not allowed") + if !result.OK { + t.Errorf("x-gts-* nested inside a trait-schema body should be tolerated, got: %s", result.Error) } } -func TestValidateSchemaTraits_LockedTraitOverride_Fails(t *testing.T) { +func TestValidateSchemaTraits_DescendantOverride_LastWins(t *testing.T) { + // RFC 7396 last-wins (ADR-0004): a descendant freely overrides an ancestor's + // trait value when the trait-schema does not lock it via `const`. store := NewGtsStore(nil) - // Base defines trait value without a schema property — trait becomes locked mustRegisterTraits(t, store, map[string]any{ - "$id": "gts.x.traits3.ns.base.v1~", - "type": "object", + "$id": "gts.x.traits3.ns.base.v1~", + "type": "object", + "x-gts-traits-schema": map[string]any{ + "type": "object", + "properties": map[string]any{ + "color": map[string]any{"type": "string"}, + }, + }, "x-gts-traits": map[string]any{"color": "red"}, }) - // Child tries to override with a different value — must fail mustRegisterTraits(t, store, map[string]any{ "$id": "gts.x.traits3.ns.base.v1~x.traits3.ns.child.v1~", "type": "object", "x-gts-traits": map[string]any{"color": "blue"}, }) result := store.ValidateSchemaTraits("gts.x.traits3.ns.base.v1~x.traits3.ns.child.v1~") + if !result.OK { + t.Errorf("descendant override (last-wins) should be allowed, got: %s", result.Error) + } +} + +func TestValidateSchemaTraits_ConstLock_OverrideFails(t *testing.T) { + // Publishers lock a value via standard JSON Schema `const` in the + // trait-schema (ADR-0004). A descendant that sets a different value fails + // standard validation against the effective trait-schema. + store := NewGtsStore(nil) + mustRegisterTraits(t, store, map[string]any{ + "$id": "gts.x.traits3c.ns.base.v1~", + "type": "object", + "x-gts-traits-schema": map[string]any{ + "type": "object", + "properties": map[string]any{ + "indexed": map[string]any{"type": "boolean", "const": true}, + }, + }, + "x-gts-traits": map[string]any{"indexed": true}, + }) + mustRegisterTraits(t, store, map[string]any{ + "$id": "gts.x.traits3c.ns.base.v1~x.traits3c.ns.child.v1~", + "type": "object", + "x-gts-traits": map[string]any{"indexed": false}, + }) + result := store.ValidateSchemaTraits("gts.x.traits3c.ns.base.v1~x.traits3c.ns.child.v1~") if result.OK { - t.Error("expected failure: child overrides locked trait value") + t.Error("expected failure: descendant violates const-locked trait value") } } -func TestValidateSchemaTraits_DuplicateDefaultAcrossChain_Fails(t *testing.T) { +func TestValidateSchemaTraits_RedeclaredDefaultAcrossChain_Allowed(t *testing.T) { + // With chain aggregation via allOf and RFC 7396 merge (no GTS-specific + // immutability), a descendant may redeclare a property's `default`. It + // simply doesn't take effect for a property already declared upstream — the + // aggregated allOf retains both declarations. store := NewGtsStore(nil) mustRegisterTraits(t, store, map[string]any{ "$id": "gts.x.traits4.ns.base.v1~", @@ -748,10 +848,8 @@ func TestValidateSchemaTraits_DuplicateDefaultAcrossChain_Fails(t *testing.T) { "properties": map[string]any{ "color": map[string]any{"type": "string", "default": "red"}, }, - "additionalProperties": false, }, }) - // Child redefines default for the same property with a different value — must fail mustRegisterTraits(t, store, map[string]any{ "$id": "gts.x.traits4.ns.base.v1~x.traits4.ns.child.v1~", "type": "object", @@ -760,12 +858,86 @@ func TestValidateSchemaTraits_DuplicateDefaultAcrossChain_Fails(t *testing.T) { "properties": map[string]any{ "color": map[string]any{"type": "string", "default": "blue"}, }, - "additionalProperties": false, }, }) result := store.ValidateSchemaTraits("gts.x.traits4.ns.base.v1~x.traits4.ns.child.v1~") - if result.OK { - t.Error("expected failure: child overrides ancestor's trait schema default") + if !result.OK { + t.Errorf("redeclared default in descendant should be allowed, got: %s", result.Error) + } +} + +func TestValidateSchemaTraits_BooleanFalse_ProhibitsTraits(t *testing.T) { + // x-gts-traits-schema: false prohibits trait values on the host and its + // subtree (ADR-0002). A trait-less type still validates; any trait fails. + store := NewGtsStore(nil) + mustRegisterTraits(t, store, map[string]any{ + "$id": "gts.x.traitsf.ns.notraits.v1~", + "type": "object", + "x-gts-traits-schema": false, + }) + if r := store.ValidateSchemaTraits("gts.x.traitsf.ns.notraits.v1~"); !r.OK { + t.Errorf("false trait-schema with no trait values should pass, got: %s", r.Error) + } + + mustRegisterTraits(t, store, map[string]any{ + "$id": "gts.x.traitsf.ns.withtraits.v1~", + "type": "object", + "x-gts-traits-schema": false, + "x-gts-traits": map[string]any{"anything": "x"}, + }) + if r := store.ValidateSchemaTraits("gts.x.traitsf.ns.withtraits.v1~"); r.OK { + t.Error("expected failure: false trait-schema prohibits trait values") + } +} + +func TestValidateSchemaTraits_NestedFalseInAllOf_ProhibitsTraits(t *testing.T) { + // A `false` nested inside an allOf makes the effective trait schema + // unsatisfiable — allOf(false, …) ≡ false — so traits are prohibited even + // when the `false` is not a bare top-level subschema (ADR-0002). + store := NewGtsStore(nil) + mustRegisterTraits(t, store, map[string]any{ + "$id": "gts.x.traitsnf.ns.withtraits.v1~", + "type": "object", + "x-gts-traits-schema": map[string]any{ + "allOf": []any{false}, + }, + "x-gts-traits": map[string]any{"anything": "x"}, + }) + if r := store.ValidateSchemaTraits("gts.x.traitsnf.ns.withtraits.v1~"); r.OK { + t.Error("expected failure: allOf containing false prohibits trait values") + } +} + +func TestValidateSchemaTraits_BooleanTrue_AllowsAnyTraits(t *testing.T) { + // x-gts-traits-schema: true permits arbitrary trait values (ADR-0002). + store := NewGtsStore(nil) + mustRegisterTraits(t, store, map[string]any{ + "$id": "gts.x.traitst.ns.anytraits.v1~", + "type": "object", + "x-gts-traits-schema": true, + "x-gts-traits": map[string]any{"retention": "P90D", "anything": 7.0}, + }) + if r := store.ValidateSchemaTraits("gts.x.traitst.ns.anytraits.v1~"); !r.OK { + t.Errorf("true trait-schema should accept any traits, got: %s", r.Error) + } +} + +func TestValidateSchemaTraits_AbstractSkipsCompleteness(t *testing.T) { + // Abstract types skip the required-trait completeness check (ADR-0003). + store := NewGtsStore(nil) + mustRegisterTraits(t, store, map[string]any{ + "$id": "gts.x.traitsa.ns.base.v1~", + "type": "object", + "x-gts-abstract": true, + "x-gts-traits-schema": map[string]any{ + "type": "object", + "properties": map[string]any{"retention": map[string]any{"type": "string"}}, + "required": []any{"retention"}, + }, + // No x-gts-traits and no default — would fail completeness if non-abstract. + }) + if r := store.ValidateSchemaTraits("gts.x.traitsa.ns.base.v1~"); !r.OK { + t.Errorf("abstract type should skip completeness, got: %s", r.Error) } } @@ -835,6 +1007,9 @@ func TestValidateEntity_Schema_ChainIncompatible(t *testing.T) { } func TestValidateEntity_Schema_TraitSchemaWithoutValues_Fails(t *testing.T) { + // The entity-level check (/validate-entity) is stricter than type-schema + // validation: a deployable entity that declares a trait schema must provide + // trait values somewhere in the chain. store := NewGtsStore(nil) mustRegisterTraits(t, store, map[string]any{ "$id": "gts.x.entity.ns.novals.v1~", @@ -844,7 +1019,7 @@ func TestValidateEntity_Schema_TraitSchemaWithoutValues_Fails(t *testing.T) { "properties": map[string]any{"color": map[string]any{"type": "string"}}, "additionalProperties": false, }, - // No x-gts-traits — entity-level check must fail + // No x-gts-traits — entity-level check must fail. }) result := store.ValidateEntity("gts.x.entity.ns.novals.v1~") if result.OK { @@ -853,6 +1028,8 @@ func TestValidateEntity_Schema_TraitSchemaWithoutValues_Fails(t *testing.T) { } func TestValidateEntity_Schema_TraitSchemaNotClosed_Fails(t *testing.T) { + // The entity-level check requires every (object-form) trait schema to be + // closed (additionalProperties:false) to be a deployable standalone entity. store := NewGtsStore(nil) mustRegisterTraits(t, store, map[string]any{ "$id": "gts.x.entity.ns.notclosed.v1~", @@ -860,7 +1037,7 @@ func TestValidateEntity_Schema_TraitSchemaNotClosed_Fails(t *testing.T) { "x-gts-traits-schema": map[string]any{ "type": "object", "properties": map[string]any{"color": map[string]any{"type": "string"}}, - // additionalProperties intentionally absent — must fail entity-level check + // additionalProperties intentionally absent — entity-level check fails. }, "x-gts-traits": map[string]any{"color": "red"}, }) diff --git a/gts/store.go b/gts/store.go index 7eb468f..99e3edf 100644 --- a/gts/store.go +++ b/gts/store.go @@ -139,7 +139,7 @@ func (s *GtsStore) registerLocked(entity *JsonEntity) error { } s.byID[key] = entity - log.Printf("Registered entity: %s (schema: %v, refs: %d)", key, entity.IsSchema, len(entity.GtsRefs)) + log.Printf("Registered entity: %s (type-schema: %v, refs: %d)", key, entity.IsTypeSchema, len(entity.GtsRefs)) return nil } @@ -185,9 +185,9 @@ func (s *GtsStore) RegisterSchema(typeID string, schema map[string]any) error { } entity := &JsonEntity{ - GtsID: gtsID, - Content: schema, - IsSchema: true, + GtsID: gtsID, + Content: schema, + IsTypeSchema: true, } s.mu.Lock() @@ -222,8 +222,8 @@ func (s *GtsStore) GetSchemaContent(typeID string) (map[string]any, error) { if entity == nil { return nil, fmt.Errorf("schema not found: %s", typeID) } - if !entity.IsSchema { - return nil, fmt.Errorf("entity is not a schema: %s", typeID) + if !entity.IsTypeSchema { + return nil, fmt.Errorf("entity is not a type-schema: %s", typeID) } return entity.Content, nil } @@ -248,9 +248,9 @@ func (s *GtsStore) Count() int { // EntityInfo represents basic information about an entity type EntityInfo struct { - ID string `json:"id"` - SchemaID string `json:"schema_id"` - IsSchema bool `json:"is_schema"` + ID string `json:"id"` + TypeID string `json:"type_id"` + IsTypeSchema bool `json:"is_type_schema"` } // ListResult represents the result of listing entities @@ -271,9 +271,9 @@ func (s *GtsStore) List(limit int) *ListResult { break } entities = append(entities, EntityInfo{ - ID: id, - SchemaID: entity.SchemaID, - IsSchema: entity.IsSchema, + ID: id, + TypeID: entity.TypeID, + IsTypeSchema: entity.IsTypeSchema, }) count++ } @@ -312,12 +312,12 @@ func (s *GtsStore) validateEntityGtsReferences(entity *JsonEntity) error { continue } - // Additional validation for schema references - if entity.IsSchema { + // Additional validation for type-schema references + if entity.IsTypeSchema { if strings.Contains(ref.SourcePath, "$ref") { - // This is a schema reference - the referenced entity should be a schema - if !referencedEntity.IsSchema { - errors = append(errors, fmt.Sprintf("schema reference points to non-schema entity: %s (at %s)", ref.ID, ref.SourcePath)) + // This is a type-schema reference - the referenced entity should be a type-schema + if !referencedEntity.IsTypeSchema { + errors = append(errors, fmt.Sprintf("type-schema reference points to non-type-schema entity: %s (at %s)", ref.ID, ref.SourcePath)) } } } @@ -333,7 +333,7 @@ func (s *GtsStore) validateEntityGtsReferences(entity *JsonEntity) error { // ValidateSchema validates a schema including JSON Schema meta-schema and GTS reference validation func (s *GtsStore) ValidateSchema(gtsID string) error { if !strings.HasSuffix(gtsID, "~") { - return fmt.Errorf("ID '%s' is not a schema (must end with '~')", gtsID) + return fmt.Errorf("ID '%s' is not a type-schema ID (must end with '~')", gtsID) } entity := s.Get(gtsID) @@ -341,8 +341,8 @@ func (s *GtsStore) ValidateSchema(gtsID string) error { return &StoreGtsSchemaNotFoundError{EntityID: gtsID} } - if !entity.IsSchema { - return fmt.Errorf("entity '%s' is not a schema", gtsID) + if !entity.IsTypeSchema { + return fmt.Errorf("entity '%s' is not a type-schema", gtsID) } log.Printf("Validating schema %s", gtsID) @@ -390,25 +390,25 @@ func (s *GtsStore) ValidateInstanceWithXGtsRef(instanceID string) error { return &StoreGtsObjectNotFoundError{EntityID: instanceID} } - if instance.IsSchema { - return fmt.Errorf("entity '%s' is a schema, not an instance", instanceID) + if instance.IsTypeSchema { + return fmt.Errorf("entity '%s' is a type-schema, not an instance", instanceID) } - // Get the schema for this instance - if instance.SchemaID == "" { + // Get the type-schema for this instance + if instance.TypeID == "" { return &StoreGtsSchemaForInstanceNotFoundError{EntityID: instanceID} } - schema := s.Get(instance.SchemaID) + schema := s.Get(instance.TypeID) if schema == nil { - return &StoreGtsSchemaNotFoundError{EntityID: instance.SchemaID} + return &StoreGtsSchemaNotFoundError{EntityID: instance.TypeID} } - if !schema.IsSchema { - return fmt.Errorf("schema entity '%s' is not marked as schema", instance.SchemaID) + if !schema.IsTypeSchema { + return fmt.Errorf("type-schema entity '%s' is not marked as type-schema", instance.TypeID) } - log.Printf("Validating instance %s against schema %s", instanceID, instance.SchemaID) + log.Printf("Validating instance %s against type-schema %s", instanceID, instance.TypeID) // Validate x-gts-ref constraints xGtsRefValidator := NewXGtsRefValidator(s) diff --git a/gts/validate.go b/gts/validate.go index 89c3d9e..e2b38a6 100644 --- a/gts/validate.go +++ b/gts/validate.go @@ -30,8 +30,8 @@ func (l *gtsURLLoader) Load(url string) (any, error) { if entity == nil { return nil, fmt.Errorf("unresolvable GTS reference: %s", url) } - if !entity.IsSchema { - return nil, fmt.Errorf("GTS reference is not a schema: %s", url) + if !entity.IsTypeSchema { + return nil, fmt.Errorf("GTS reference is not a type-schema: %s", url) } return entity.Content, nil } @@ -85,8 +85,8 @@ func (s *GtsStore) ValidateInstance(instanceID string) *ValidationResult { } } - // Check if instance has a schema ID - if obj.SchemaID == "" { + // Check if instance has a type ID + if obj.TypeID == "" { return &ValidationResult{ ID: instanceID, OK: false, @@ -94,21 +94,21 @@ func (s *GtsStore) ValidateInstance(instanceID string) *ValidationResult { } } - // Get the schema from store - schemaEntity := s.Get(obj.SchemaID) + // Get the type-schema from store + schemaEntity := s.Get(obj.TypeID) if schemaEntity == nil { return &ValidationResult{ ID: instanceID, OK: false, - Error: (&StoreGtsSchemaNotFoundError{EntityID: obj.SchemaID}).Error(), + Error: (&StoreGtsSchemaNotFoundError{EntityID: obj.TypeID}).Error(), } } - if !schemaEntity.IsSchema { + if !schemaEntity.IsTypeSchema { return &ValidationResult{ ID: instanceID, OK: false, - Error: fmt.Sprintf("entity '%s' is not a schema", obj.SchemaID), + Error: fmt.Sprintf("entity '%s' is not a type-schema", obj.TypeID), } } @@ -118,7 +118,7 @@ func (s *GtsStore) ValidateInstance(instanceID string) *ValidationResult { return &ValidationResult{ ID: instanceID, OK: false, - Error: fmt.Sprintf("type '%s' is abstract and cannot have direct instances", obj.SchemaID), + Error: fmt.Sprintf("type '%s' is abstract and cannot have direct instances", obj.TypeID), } } } @@ -270,7 +270,7 @@ func (s *GtsStore) validateWithSchema(instance map[string]any, schema map[string // Pre-load all schemas from the store (matches Python's store dict pre-population) // Note: Store IDs are already normalized (without gts:// prefix) for id, entity := range s.byID { - if entity.IsSchema && id != normalizedSchemaID { + if entity.IsTypeSchema && id != normalizedSchemaID { if err := compiler.AddResource(id, entity.Content); err != nil { // Ignore errors - gtsURLLoader will handle dynamic resolution continue diff --git a/server/handlers.go b/server/handlers.go index 0bb9f0a..3ef4056 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -148,8 +148,8 @@ func (s *Server) handleAddEntity(w http.ResponseWriter, r *http.Request) { return } - // Always validate schema constraints for schemas - if entity.IsSchema { + // Always validate schema constraints for type-schemas + if entity.IsTypeSchema { // Validate $id field for GTS schemas - check for specific invalid patterns if idField, exists := entity.Content["$id"]; exists { if idStr, ok := idField.(string); ok { @@ -209,8 +209,11 @@ func (s *Server) handleAddEntity(w http.ResponseWriter, r *http.Request) { } } - // Validate schema modifiers (x-gts-final, x-gts-abstract) for schemas. - if entity.IsSchema { + // Validate schema modifiers (x-gts-final, x-gts-abstract) and trait keyword + // placement (x-gts-traits, x-gts-traits-schema) for type-schemas. These are + // type-level keywords and MUST appear only at the schema top level + // (gts-spec §9.7.1/§9.11). + if entity.IsTypeSchema { if err := gts.ValidateSchemaModifiers(entity.Content); err != nil { s.writeJSON(w, http.StatusUnprocessableEntity, map[string]any{ "ok": false, @@ -218,6 +221,13 @@ func (s *Server) handleAddEntity(w http.ResponseWriter, r *http.Request) { }) return } + if err := gts.ValidateTraitPlacement(entity.Content); err != nil { + s.writeJSON(w, http.StatusUnprocessableEntity, map[string]any{ + "ok": false, + "error": err.Error(), + }) + return + } } // Check if validation is requested via query parameter @@ -227,7 +237,7 @@ func (s *Server) handleAddEntity(w http.ResponseWriter, r *http.Request) { } if validation == "true" { err := s.store.RegisterWithValidation(entity, func(id string) error { - if entity.IsSchema { + if entity.IsTypeSchema { if r := s.store.ValidateSchemaChain(id); !r.OK { return errors.New(r.Error) } @@ -433,15 +443,15 @@ func (s *Server) handleResolveRelationships(w http.ResponseWriter, r *http.Reque // OP#8 - Compatibility func (s *Server) handleCompatibility(w http.ResponseWriter, r *http.Request) { - oldSchemaID := s.getQueryParam(r, "old_schema_id") - newSchemaID := s.getQueryParam(r, "new_schema_id") + oldTypeID := s.getQueryParam(r, "old_type_id") + newTypeID := s.getQueryParam(r, "new_type_id") - if oldSchemaID == "" || newSchemaID == "" { - s.writeError(w, http.StatusBadRequest, "Missing old_schema_id or new_schema_id parameter") + if oldTypeID == "" || newTypeID == "" { + s.writeError(w, http.StatusBadRequest, "Missing old_type_id or new_type_id parameter") return } - result := s.store.CheckCompatibility(oldSchemaID, newSchemaID) + result := s.store.CheckCompatibility(oldTypeID, newTypeID) s.writeJSON(w, http.StatusOK, result) } @@ -449,14 +459,14 @@ func (s *Server) handleCompatibility(w http.ResponseWriter, r *http.Request) { func (s *Server) handleCast(w http.ResponseWriter, r *http.Request) { var req struct { InstanceID string `json:"instance_id"` - ToSchemaID string `json:"to_schema_id"` + ToTypeID string `json:"to_type_id"` } if err := s.readJSON(r, &req); err != nil { s.writeError(w, http.StatusBadRequest, "Invalid JSON") return } - result, err := s.store.Cast(req.InstanceID, req.ToSchemaID) + result, err := s.store.Cast(req.InstanceID, req.ToTypeID) if err != nil { s.writeJSON(w, http.StatusOK, map[string]any{ "error": err.Error(), @@ -487,24 +497,24 @@ func (s *Server) handleQuery(w http.ResponseWriter, r *http.Request) { s.writeJSON(w, http.StatusOK, result) } -// OP#12 - Validate Schema (schema-vs-schema chain validation) +// OP#12 - Validate Type-Schema (schema-vs-schema chain validation) func (s *Server) handleValidateSchema(w http.ResponseWriter, r *http.Request) { var req struct { - SchemaID string `json:"schema_id"` + TypeID string `json:"type_id"` } if err := s.readJSON(r, &req); err != nil { s.writeError(w, http.StatusBadRequest, "Invalid JSON") return } - if req.SchemaID == "" { - s.writeError(w, http.StatusBadRequest, "Missing schema_id") + if req.TypeID == "" { + s.writeError(w, http.StatusBadRequest, "Missing type_id") return } - result := s.store.ValidateSchemaChain(req.SchemaID) + result := s.store.ValidateSchemaChain(req.TypeID) if result.OK { // Also run OP#13 traits validation - traitsResult := s.store.ValidateSchemaTraits(req.SchemaID) + traitsResult := s.store.ValidateSchemaTraits(req.TypeID) if !traitsResult.OK { s.writeJSON(w, http.StatusOK, map[string]any{ "ok": false, diff --git a/server/server.go b/server/server.go index 23496d2..856c429 100644 --- a/server/server.go +++ b/server/server.go @@ -44,7 +44,7 @@ func (s *Server) registerRoutes() { s.mux.HandleFunc("GET /entities/{id}", s.handleGetEntity) s.mux.HandleFunc("POST /entities", s.handleAddEntity) s.mux.HandleFunc("POST /entities/bulk", s.handleAddEntities) - s.mux.HandleFunc("POST /schemas", s.handleAddSchema) + s.mux.HandleFunc("POST /type-schemas", s.handleAddSchema) // OP#1 - Validate ID s.mux.HandleFunc("GET /validate-id", s.handleValidateID) @@ -79,8 +79,8 @@ func (s *Server) registerRoutes() { // OP#11 - Attribute Access s.mux.HandleFunc("GET /attr", s.handleAttribute) - // OP#12 - Validate Schema (schema-vs-schema chain validation) - s.mux.HandleFunc("POST /validate-schema", s.handleValidateSchema) + // OP#12 - Validate Type-Schema (schema-vs-schema chain validation) + s.mux.HandleFunc("POST /validate-type-schema", s.handleValidateSchema) // OP#13 - Validate Entity (schema chain + traits validation) s.mux.HandleFunc("POST /validate-entity", s.handleValidateEntity) @@ -241,10 +241,10 @@ func (s *Server) GetOpenAPISpec() map[string]any { "operationId": "attr", }, }, - "/validate-schema": map[string]any{ + "/validate-type-schema": map[string]any{ "post": map[string]any{ - "summary": "Validate a derived schema against its chain", - "operationId": "validateSchema", + "summary": "Validate a derived type-schema against its chain", + "operationId": "validateTypeSchema", }, }, "/validate-entity": map[string]any{