Просмотр исходного кода

Write the config where the config is read from, and finish the postData ids

Trog::Config::get() hardcoded the 'config/' prefix inside the sub, so
$home_cfg was the bare basename "main.cfg".  Everything that *wrote* the
configuration used that variable, and therefore wrote to the instance root --
somewhere get() would never look.  That is why a main.cfg with a hostname in it
has been sitting in the repo root doing nothing.  Both package vars are now
full paths relative to the tCMS root and get() uses them as-is.

bin/tcms-hostname was worse: it called Config::Simple::save(), which writes
back to whichever file get() loaded.  With no instance config that is
config/default.cfg -- the tracked, shipped defaults.  Setting a hostname
rewrote a file in git.  It writes to $home_cfg now.

Also finishes what the previous commit started: series.tx and profile.tx were
still emitting a bare id="postData", shared by every post of that type on the
page, where the other forms suffix it with the post id.

t/Trog-Config.t covers the round trip -- write where the writers write, read
where the readers read, and check they agree -- and fails against the old
arrangement.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U3k2hhSZpcsoe6uBmhF9nB
George Baugh 2 месяцев назад
Родитель
Сommit
1c148657a6

+ 5 - 1
bin/tcms-hostname

@@ -18,7 +18,11 @@ my $hostname = $ARGV[0];
 my $conf = Trog::Config->get();
 my $conf = Trog::Config->get();
 if ($hostname) {
 if ($hostname) {
     $conf->param('general.hostname', $hostname);
     $conf->param('general.hostname', $hostname);
-    $conf->save();
+
+    # write(), not save().  save() writes back to whatever file get() loaded,
+    # which is the tracked config/default.cfg until the instance has saved a
+    # configuration of its own.
+    $conf->write($Trog::Config::home_cfg) or die "Could not write $Trog::Config::home_cfg";
 }
 }
 
 
 my $domain = $conf->param('general.hostname');
 my $domain = $conf->param('general.hostname');

+ 13 - 4
lib/Trog/Config.pm

@@ -17,17 +17,26 @@ A thin wrapper around Config::Simple which reads the configuration from the appr
 Returns a configuration object that will be used by server.psgi, the data model and Routing modules.
 Returns a configuration object that will be used by server.psgi, the data model and Routing modules.
 Memoized, so you will need to HUP the children on config changes.
 Memoized, so you will need to HUP the children on config changes.
 
 
+Reads $home_cfg, falling back to $default when the instance has never saved a
+configuration of its own.  Both are full paths relative to the tCMS root, so
+that writers can use them too -- an earlier version had the 'config/' prefix
+hardcoded here and nowhere else, which meant everything that *wrote* the
+configuration put it somewhere get() would never look.
+
 =cut
 =cut
 
 
-our $home_cfg = "main.cfg";
-our $default  = "default.cfg";
+# Where an instance's own configuration lives, once it has saved one.
+our $home_cfg = "config/main.cfg";
+
+# Shipped defaults.  Tracked in git -- never write to this.
+our $default = "config/default.cfg";
 
 
 sub get {
 sub get {
     state $cf;
     state $cf;
     return $cf if $cf;
     return $cf if $cf;
     foreach my $cfg2try ($home_cfg, $default) {
     foreach my $cfg2try ($home_cfg, $default) {
-        next unless -f "config/$cfg2try";
-        $cf = Config::Simple->new("config/$cfg2try");
+        next unless -f $cfg2try;
+        $cf = Config::Simple->new($cfg2try);
         last;
         last;
     }
     }
     die "Could not find config file!" unless $cf;
     die "Could not find config file!" unless $cf;

+ 1 - 1
lib/Trog/Routes/HTML.pm

@@ -799,7 +799,7 @@ sub _get_data_models {
 
 
 =head2 config_save
 =head2 config_save
 
 
-Implements /config/save route.  Saves what little configuration we actually use to ~/.tcms/tcms.conf
+Implements /config/save route.  Saves what little configuration we actually use to config/main.cfg
 
 
 =cut
 =cut
 
 

+ 74 - 0
t/Trog-Config.t

@@ -0,0 +1,74 @@
+use strict;
+use warnings;
+
+use Test::More;
+use Cwd();
+use File::Path();
+use File::Temp();
+use Path::Tiny();
+use FindBin;
+
+use lib "$FindBin::Bin/../lib";
+
+# $Trog::Config::default is read exactly once below, which is enough for perl
+# to suspect a typo.
+no warnings qw{once};
+
+require_ok('Trog::Config') or BAIL_OUT("Can't find SUT");
+
+# Every path in this app is cwd-relative, so exercise it the way the app sees
+# it: from inside an instance root.  CLEANUP => 0 plus an explicit END, because
+# a tempdir cannot tidy itself up while we are standing inside it.
+our $ROOT   = File::Temp::tempdir( 'tcms-config-XXXXXX', TMPDIR => 1, CLEANUP => 0 );
+our $OLDCWD = Cwd::getcwd();
+
+END {
+    chdir($OLDCWD) if $OLDCWD;
+    File::Path::remove_tree($ROOT) if $ROOT && -d $ROOT;
+}
+
+File::Path::make_path("$ROOT/config");
+chdir($ROOT) or die $!;
+
+subtest 'the defaults are where the app actually keeps them' => sub {
+
+    # The whole bug this test exists for: get() used to hardcode the 'config/'
+    # prefix internally, so $home_cfg was a bare basename and every writer that
+    # used it put the file somewhere get() would never look.
+    is( $Trog::Config::home_cfg, 'config/main.cfg',    'home_cfg is a usable path' );
+    is( $Trog::Config::default,  'config/default.cfg', 'and so is the default' );
+
+    like( $Trog::Config::home_cfg, qr{^config/}, 'home_cfg is under config/' );
+    unlike( $Trog::Config::home_cfg, qr{^/}, 'and is relative, so it follows the instance root' );
+};
+
+subtest 'a config written to home_cfg is the one that gets read back' => sub {
+    Path::Tiny->new('config/default.cfg')->spew_utf8("[general]\ndata_model=FlatFile\ntitle=Stock\n");
+
+    # This is the round trip that was broken: write where the writers write,
+    # read where the readers read, and check they agree.
+    require Config::Simple;
+    my $written = Config::Simple->new( syntax => 'ini' );
+    $written->param( 'general.data_model', 'FlatFile' );
+    $written->param( 'general.title',      'Instance' );
+    $written->param( 'general.hostname',   'spec.example.com' );
+    ok( $written->write($Trog::Config::home_cfg), 'wrote to home_cfg' );
+
+    ok( -f 'config/main.cfg', 'which landed under config/, not the instance root' );
+    ok( !-f 'main.cfg',       'and not loose in the root where nothing would read it' );
+
+    my $read = Trog::Config::get();
+    is( $read->param('general.title'),    'Instance',         'get() picked up the written config' );
+    is( $read->param('general.hostname'), 'spec.example.com', 'including a key the defaults never had' );
+};
+
+subtest 'the shipped defaults are left alone' => sub {
+    # get() is memoized, so this is the same object as above -- which is the
+    # point: bin/tcms-hostname used to call save() on it, and save() writes back
+    # to whichever file it was constructed from.
+    is( Path::Tiny->new('config/default.cfg')->slurp_utf8,
+        "[general]\ndata_model=FlatFile\ntitle=Stock\n",
+        'default.cfg is untouched' );
+};
+
+done_testing();

+ 1 - 1
www/templates/html/components/forms/profile.tx

@@ -10,7 +10,7 @@
             <div style="background-image:url(<: $post.wallpaper :>);" class="banner profile">
             <div style="background-image:url(<: $post.wallpaper :>);" class="banner profile">
                 <div style="background-image:url(<: $post.preview :>);" class="circle portrait"></div>
                 <div style="background-image:url(<: $post.preview :>);" class="circle portrait"></div>
                 : if( $post.data ) {
                 : if( $post.data ) {
-                    <div id="postData" class="nameBadge">
+                    <div id="postData-<: $post.id :>" class="nameBadge">
                         <big><: $post.title :></big><br /><br />
                         <big><: $post.title :></big><br /><br />
                         <: render_it($post.data) | mark_raw  :>
                         <: render_it($post.data) | mark_raw  :>
                     </div>
                     </div>

+ 1 - 1
www/templates/html/components/forms/series.tx

@@ -20,7 +20,7 @@
         : }
         : }
 
 
         : if( $post.data ) {
         : if( $post.data ) {
-            <div class="postData" id="postData">
+            <div class="postData" id="postData-<: $post.id :>">
                 <: render_it($post.data) | mark_raw  :>
                 <: render_it($post.data) | mark_raw  :>
             </div>
             </div>
         : }
         : }