fix(knowledgegraph): fall back to configured default for invalid number/int params

applyDefaultParams() documented that an invalid number/int value falls back
to the configured default, matching the bool branch, but settype(null, ...)
silently produced 0.0/0 instead, discarding the default entirely.

Author: gesinn-it-gea

CHANGELOG.md

@@ -17,6 +17,7 @@ This project adheres to [Semantic Versioning](https://semver.org/) and
 - `KnowledgeGraph::setSemanticDataFromApi()` no longer reads/writes the public static `KnowledgeGraph::$data` or the private static `KnowledgeGraph::$relationsSeen` as an implicit communication channel with its callers; it now takes `$data`/`$relationsSeen` as by-reference parameters (accumulating across its own recursion) and also returns `$data`, so a call's result is explicit rather than something callers had to read back out of shared static state afterwards. `KnowledgeGraphApiLoadNodes`, `KnowledgeGraphApiLoadProperties`, and `KnowledgeGraphApiLoadCategories` (as well as `KnowledgeGraph::parserFunctionKnowledgeGraph()`) now each thread their own local `$data`/`$relationsSeen` through the call instead of relying on `\KnowledgeGraph::$data`/external reset discipline; this also fixes `KnowledgeGraphApiLoadCategories::execute()`'s pre-existing `self::$data` reference (a class with no such declared property, working only because the read is guarded by `isset()`) and removes the class of fatal ("Access to undeclared static property") this pattern had already caused twice before (see #68, #69 above). The now-unused `KnowledgeGraph::resetSeenRelations()` test-support method (and its dedicated test) was removed, since resetting shared state between calls/tests is no longer necessary ([#92](https://github.com/SemanticMediaWiki/KnowledgeGraph/issues/92))
 
 ### Fixed
+- `KnowledgeGraph::applyDefaultParams()`: for the `number`/`int`/`integer` types, an invalid value (one that fails `filter_var( ..., FILTER_NULL_ON_FAILURE )`) fell through to `settype( null, 'float' | 'integer' )`, which silently produced `0.0`/`0` instead of the type's documented behavior of falling back to the configured default — already the `bool`/`boolean` branch's behavior. Both branches now re-validate `$defaultValue` through the same filter on failure, matching `bool`/`boolean`. Updated `KnowledgeGraphApplyDefaultParamsTest::testNumberInvalidValueBecomesZeroNotDefault()` / `testIntegerInvalidValueBecomesZeroNotDefault()` (which had locked in the old, buggy behavior per #74) to `testNumberInvalidValueFallsBackToDefault()` / `testIntegerInvalidValueFallsBackToDefault()`, asserting the corrected default-fallback behavior. The other silent-fallback call sites reviewed alongside this fix (`getAllPropertiesForNode()`, `setSemanticDataFromApi()`, and the three `KnowledgeGraphApi*` endpoints, which only relay whatever `setSemanticDataFromApi()` returns) were left as `wfDebugLog()`-only intentionally: they exist inside a recursive graph-building traversal where one title/API failure should degrade to a partial graph rather than aborting or exception-ing the whole request ([#95](https://github.com/SemanticMediaWiki/KnowledgeGraph/issues/95))
 - `KnowledgeGraphApiLoadNodesExecuteTest::testDepthZeroCreatesRootNodeOnly`, `KnowledgeGraphApiLoadPropertiesExecuteTest::testDepthZeroCreatesRootNodeOnly`, `KnowledgeGraphApiLoadPropertiesExecuteTest::testInversePropsIncludedDoesNotAffectRootNodeCreation`: these three assertions still expected the root-node-only (`depth === 0`) API result shape from before the DISPLAYTITLE feature (`[ 'properties' => [], 'categories' => [] ]`), left stale when that feature added a `displayTitle` key to both branches of `KnowledgeGraph::setSemanticDataFromApi()`'s output but only updated `KnowledgeGraphSetSemanticDataFromApiTest.php` to match — breaking `main`'s CI on every run since. Updated all three to expect `'displayTitle' => null` (no `{{DISPLAYTITLE:...}}` set on the inserted test pages) ([#101](https://github.com/SemanticMediaWiki/KnowledgeGraph/issues/101))
 - `KnowledgeGraph.js`'s `attachContextMenuListener()`'s property-click-to-toggle handler: clicking a property in a node's right-click context menu that was already present in the graph (e.g. hidden via the by-article property-visibility selection from #16) did not reliably toggle it, instead sometimes creating a second, parallel node/edge for the same logical value. Root cause: the handler fetched a node's available properties via its own client-side `action=smwbrowse` call and re-derived node/edge ids independently from the server-side logic `KnowledgeGraph::setSemanticDataFromApi()` already uses for the initial graph build -- most notably resolving `_wpg` (wikipage-type) values via the *localized* namespace name (`mw.config.get('wgFormattedNamespaces')`) where the server resolves the same values via the *canonical* (English) namespace name (`NamespaceInfo::getCanonicalName()`), so on a non-English wiki the two code paths computed different ids for the same value (e.g. `Category:Site` vs. `Kategorie:Site`) and the toggle's `self.Edges.get(id)` lookup missed the already-existing edge. Replaced the client-side `smwbrowse` re-derivation with the existing `loadNodes()` helper (the same `action=knowledgegraph-load-nodes` endpoint the initial graph load already uses), so the context menu now renders/toggles using the exact server-canonical property/value shape and ids `createNodes()` was built from; toggling now flips the existing edge's/node's `hidden` flag (matching the visibility model used elsewhere) instead of removing and recreating them. A new `buildNodeAndEdgeFromValue()` helper factors the per-value node/edge-building logic out of `createNodes()` so both call sites stay in sync. Removed `fetchSemanticDataForNode()`, `parseProperties()`, `getPropertyValueForNode()`, `fetchNamespaceNameForNode()`, and `self.nodePropertiesCache`, which existed solely to support the old, independently-re-derived path ([#100](https://github.com/SemanticMediaWiki/KnowledgeGraph/issues/100))
 - `KnowledgeGraph.js`'s `attachContextMenuListener()`: the right-click "open article" link entry in a node's context menu always showed the node's raw title text, even for a node labeled with a DISPLAYTITLE-derived display name; it now shows the node's actual rendered label, while the link itself still resolves to the real page title

includes/KnowledgeGraph.php

@@ -620,12 +620,18 @@ public static function applyDefaultParams( $defaultParams, $params ) {
 
 				case 'number':
 					$val = filter_var( $val, FILTER_VALIDATE_FLOAT, FILTER_NULL_ON_FAILURE );
+					if ( $val === null ) {
+						$val = filter_var( $defaultValue, FILTER_VALIDATE_FLOAT, FILTER_NULL_ON_FAILURE );
+					}
 					settype( $val, "float" );
 					break;
 
 				case 'int':
 				case 'integer':
 					$val = filter_var( $val, FILTER_VALIDATE_INT, FILTER_NULL_ON_FAILURE );
+					if ( $val === null ) {
+						$val = filter_var( $defaultValue, FILTER_VALIDATE_INT, FILTER_NULL_ON_FAILURE );
+					}
 					settype( $val, "integer" );
 					break;
 

tests/phpunit/Unit/KnowledgeGraphApplyDefaultParamsTest.php

@@ -77,20 +77,13 @@ public function testNumberValidValueIsConvertedToFloat() {
 		$this->assertSame( [ 'ratio' => 3.5 ], $result );
 	}
 
-	/**
-	 * filter_var( ..., FILTER_VALIDATE_FLOAT, FILTER_NULL_ON_FAILURE ) returns
-	 * null for an invalid value, and settype( null, 'float' ) yields 0.0 —
-	 * NOT the configured default value. This locks in that (surprising)
-	 * current behavior rather than the documented "falls back to default"
-	 * intent; see the reported findings for details.
-	 */
-	public function testNumberInvalidValueBecomesZeroNotDefault() {
+	public function testNumberInvalidValueFallsBackToDefault() {
 		$result = KnowledgeGraph::applyDefaultParams(
 			[ 'ratio' => [ 9.9, 'number' ] ],
 			[ 'ratio' => 'not-a-number' ]
 		);
 
-		$this->assertSame( [ 'ratio' => 0.0 ], $result );
+		$this->assertSame( [ 'ratio' => 9.9 ], $result );
 	}
 
 	public function testIntegerValidValueIsConvertedToInt() {
@@ -102,17 +95,13 @@ public function testIntegerValidValueIsConvertedToInt() {
 		$this->assertSame( [ 'count' => 42 ], $result );
 	}
 
-	/**
-	 * Same fallback-to-zero behavior as the 'number' type: an invalid int
-	 * silently becomes 0 rather than falling back to the configured default.
-	 */
-	public function testIntegerInvalidValueBecomesZeroNotDefault() {
+	public function testIntegerInvalidValueFallsBackToDefault() {
 		$result = KnowledgeGraph::applyDefaultParams(
 			[ 'count' => [ 7, 'int' ] ],
 			[ 'count' => 'not-a-number' ]
 		);
 
-		$this->assertSame( [ 'count' => 0 ], $result );
+		$this->assertSame( [ 'count' => 7 ], $result );
 	}
 
 	public function testUnknownTypeValueIsPassedThroughUnchanged() {