Skip to content

Commit f200d7e

Browse files
rubenvdlinderubenvdlinde
committed
fix(gitignore): stop substring globs from swallowing source files
The previous commit's contract tests were NOT in it. `git add` dropped tests/unit/Controller/PreferencesControllerTest.php silently — no error, no mention in `git status` — because .gitignore carried **/*references* and "PReferencesController" contains "references". The same rule was already ignoring lib/Controller/PreferencesController.php itself. Six more substring globs sat beside it and are the identical trap waiting: **/*Analysis* **/*encoding* **/clearCache* **/update*Settings* **/rebase* **/setup* `**/update*Settings*` swallows an UpdateSettingsCommand, `**/clearCache*` a ClearCacheTest, `**/setup*` a setup.ts. All removed. What the block was FOR is kept: the patterns that remain all contain a SPACE, which is what makes them safe — they catch stray files accidentally named with a sentence ("PR something"), and no PHP class or spec file can contain a space, so they cannot reach source. This matters beyond one repo. This file is copied into every app scaffolded from this template, so each of them inherited a .gitignore that discards plausibly-named source files without saying so. A file that was never added looks exactly like a file that was. Verified: the test file is no longer ignored, it is committed, and the 26 unit tests still pass.
1 parent 820cd58 commit f200d7e

2 files changed

Lines changed: 232 additions & 8 deletions

File tree

‎.gitignore‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,36 @@ phpqa_output.log
4040
*.xls
4141
*.xlsx
4242

43-
# Files with unusual extensions or no extensions that could be mistakes
43+
# Files with unusual extensions or no extensions that could be mistakes.
44+
#
45+
# THE PATTERNS BELOW ALL CONTAIN A SPACE, AND THAT IS LOAD-BEARING. They exist
46+
# to catch stray files accidentally created with a sentence for a name ("PR
47+
# something", "adds something"). A PHP class or a spec file cannot contain a
48+
# space, so these cannot reach source.
49+
#
50+
# Seven SUBSTRING globs used to sit here and could:
51+
#
52+
# **/*Analysis* **/*references* **/*encoding*
53+
# **/clearCache* **/update*Settings* **/rebase* **/setup*
54+
#
55+
# `**/*references*` matched lib/Controller/PreferencesController.php — the word
56+
# is inside "PReferencesController" — so a new PreferencesControllerTest.php was
57+
# silently NOT added by `git add`, with no error and no mention in git status.
58+
# The commit that was supposed to introduce it simply did not contain it.
59+
#
60+
# The others are the same trap waiting: `**/update*Settings*` swallows an
61+
# UpdateSettingsCommand, `**/clearCache*` a ClearCacheTest, `**/setup*` a
62+
# setup.ts. This file is copied into every app scaffolded from this template,
63+
# so each of them inherited a .gitignore that drops plausibly-named source
64+
# files without saying so.
65+
#
66+
# If a stray file needs ignoring, name it — do not reach for a substring.
4467
**/PR *
4568
**/adds *
4669
**/implements *
4770
**/ALL *
4871
**/endpoints *
49-
**/*Analysis*
50-
**/*references*
51-
**/*encoding*
5272
**/ter
53-
**/clearCache*
54-
**/update*Settings*
55-
**/rebase*
56-
**/setup*
5773

5874
# Temporary test files that shouldn't be committed
5975
simple-solr-test.php

‎tests/unit/Controller/PreferencesControllerTest.php‎

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
<?php
2+
3+
/**
4+
* Contract tests for PreferencesController.
5+
*
6+
* @category Test
7+
* @package OCA\AppTemplate\Tests\Unit\Controller
8+
*
9+
* @author Conduction Development Team <info@conduction.nl>
10+
* @copyright 2026 Conduction B.V.
11+
* @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12
12+
*
13+
* SPDX-FileCopyrightText: 2026 Conduction B.V. <info@conduction.nl>
14+
* SPDX-License-Identifier: EUPL-1.2
15+
*
16+
* @version GIT: <git-id>
17+
*
18+
* @link https://conduction.nl
19+
*/
20+
21+
declare(strict_types=1);
22+
23+
namespace OCA\AppTemplate\Tests\Unit\Controller;
24+
25+
use OCA\AppTemplate\AppInfo\Application;
26+
use OCA\AppTemplate\Controller\PreferencesController;
27+
use OCP\AppFramework\Http;
28+
use OCP\AppFramework\Http\JSONResponse;
29+
use OCP\IConfig;
30+
use OCP\IRequest;
31+
use OCP\IUser;
32+
use OCP\IUserSession;
33+
use PHPUnit\Framework\MockObject\MockObject;
34+
use PHPUnit\Framework\TestCase;
35+
36+
/**
37+
* Contract tests for the per-user preference endpoints (gate-25).
38+
*
39+
* These are the two endpoints in this template that carry an actual JSON
40+
* contract — `{value: string|null}` — as opposed to the dashboard routes,
41+
* which render the SPA shell and are excluded with a stated reason. What is
42+
* pinned here is the CONTRACT: the status code and the response shape a caller
43+
* designs against, including the unauthenticated and invalid-key paths, which
44+
* are the ones most likely to change by accident.
45+
*/
46+
class PreferencesControllerTest extends TestCase {
47+
48+
/**
49+
* The controller under test.
50+
*
51+
* @var PreferencesController
52+
*/
53+
private PreferencesController $controller;
54+
55+
/**
56+
* Mock IConfig.
57+
*
58+
* @var IConfig&MockObject
59+
*/
60+
private IConfig&MockObject $config;
61+
62+
/**
63+
* Mock IUserSession.
64+
*
65+
* @var IUserSession&MockObject
66+
*/
67+
private IUserSession&MockObject $userSession;
68+
69+
/**
70+
* Set up test fixtures.
71+
*
72+
* @return void
73+
*/
74+
protected function setUp(): void {
75+
parent::setUp();
76+
77+
$request = $this->createMock(IRequest::class);
78+
$this->config = $this->createMock(IConfig::class);
79+
$this->userSession = $this->createMock(IUserSession::class);
80+
81+
$this->controller = new PreferencesController(
82+
$request,
83+
$this->config,
84+
$this->userSession
85+
);
86+
87+
}//end setUp()
88+
89+
/**
90+
* Point the session at a logged-in user.
91+
*
92+
* @param string $uid The user id to report.
93+
*
94+
* @return void
95+
*/
96+
private function withUser(string $uid = 'alice'): void {
97+
$user = $this->createMock(IUser::class);
98+
$user->method('getUID')->willReturn($uid);
99+
$this->userSession->method('getUser')->willReturn($user);
100+
101+
}//end withUser()
102+
103+
/**
104+
* A stored preference is returned under `value`.
105+
*
106+
* @return void
107+
*/
108+
public function testGetPreferenceReturnsStoredValue(): void {
109+
$this->withUser();
110+
$this->config->expects($this->once())
111+
->method('getUserValue')
112+
->with('alice', Application::APP_ID, 'pref_theme', '')
113+
->willReturn('dark');
114+
115+
$response = $this->controller->getPreference('theme');
116+
117+
$this->assertInstanceOf(JSONResponse::class, $response);
118+
$this->assertSame(Http::STATUS_OK, $response->getStatus());
119+
$this->assertSame(['value' => 'dark'], $response->getData());
120+
121+
}//end testGetPreferenceReturnsStoredValue()
122+
123+
/**
124+
* An unset preference reads as null, not as an empty string.
125+
*
126+
* The controller stores "cleared" as '' and must translate that back to
127+
* null on the way out — a caller distinguishing "unset" from "set to empty"
128+
* depends on it.
129+
*
130+
* @return void
131+
*/
132+
public function testGetPreferenceReturnsNullWhenUnset(): void {
133+
$this->withUser();
134+
$this->config->method('getUserValue')->willReturn('');
135+
136+
$response = $this->controller->getPreference('theme');
137+
138+
$this->assertSame(['value' => null], $response->getData());
139+
140+
}//end testGetPreferenceReturnsNullWhenUnset()
141+
142+
/**
143+
* No session means 401, and no config read is attempted.
144+
*
145+
* @return void
146+
*/
147+
public function testGetPreferenceRequiresALoggedInUser(): void {
148+
$this->userSession->method('getUser')->willReturn(null);
149+
$this->config->expects($this->never())->method('getUserValue');
150+
151+
$response = $this->controller->getPreference('theme');
152+
153+
$this->assertSame(Http::STATUS_UNAUTHORIZED, $response->getStatus());
154+
155+
}//end testGetPreferenceRequiresALoggedInUser()
156+
157+
/**
158+
* A key that sanitises to nothing is refused with 400 rather than read.
159+
*
160+
* This is the guard that stops a caller reaching arbitrary config keys by
161+
* smuggling separators through the key, so it is pinned deliberately.
162+
*
163+
* @return void
164+
*/
165+
public function testGetPreferenceRejectsAnUnusableKey(): void {
166+
$this->withUser();
167+
$this->config->expects($this->never())->method('getUserValue');
168+
169+
$response = $this->controller->getPreference('///');
170+
171+
$this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus());
172+
173+
}//end testGetPreferenceRejectsAnUnusableKey()
174+
175+
/**
176+
* A write stores the value under the prefixed key and echoes it back.
177+
*
178+
* @return void
179+
*/
180+
public function testSetPreferenceStoresTheValue(): void {
181+
$this->withUser();
182+
$this->config->expects($this->once())
183+
->method('setUserValue')
184+
->with('alice', Application::APP_ID, 'pref_theme', 'dark');
185+
186+
$response = $this->controller->setPreference('theme', 'dark');
187+
188+
$this->assertSame(Http::STATUS_OK, $response->getStatus());
189+
$this->assertSame(['value' => 'dark'], $response->getData());
190+
191+
}//end testSetPreferenceStoresTheValue()
192+
193+
/**
194+
* Writing without a session is refused with 401 and stores nothing.
195+
*
196+
* @return void
197+
*/
198+
public function testSetPreferenceRequiresALoggedInUser(): void {
199+
$this->userSession->method('getUser')->willReturn(null);
200+
$this->config->expects($this->never())->method('setUserValue');
201+
202+
$response = $this->controller->setPreference('theme', 'dark');
203+
204+
$this->assertSame(Http::STATUS_UNAUTHORIZED, $response->getStatus());
205+
206+
}//end testSetPreferenceRequiresALoggedInUser()
207+
208+
}//end class

0 commit comments

Comments
 (0)