preaction / Yancy

The Best Web Framework Deserves the Best Content Management System
http://preaction.me/yancy/
Other
54 stars 21 forks source link

using mojolicious reserved stash values as database column names causes issues #142

Open rmallah opened 1 year ago

rmallah commented 1 year ago

Hi ,

if we have a table as:

CREATE TABLE foo  (
        id integer primary key,
        status  varchar(20),
        UNIQUE ( status)
);

and we want to use it with Yancy CMS the mojo application will fail to start and will abort with following message

Can't load application from file "/home/rmallah/work/yancy_case/script/yancy_case": 
Route pattern "/foo/<*status>" contains a reserved stash value at 
/opt/perlbrew/perls/perl-5.32.1/lib/site_perl/5.32.1/Mojolicious/Plugin/OpenAPI.pm 
line 85. Compilation failed in require at (eval 140) line 1.

this seems to be too prohibitive and limiting.

An working example to replicate the issue is attached in the file.

yancy_case.zip

lib/YancyCase.pm

package YancyCase;
use Mojo::Base 'Mojolicious', -signatures;

use DBIx::Migration;

# This method will run once at server start
sub startup ($self) {

  # Load configuration from config file
  my $config = $self->plugin('NotYAMLConfig');

  # Configure the application
  $self->secrets($config->{secrets});

  # Router
  my $r = $self->routes;

  # Normal route to controller
  $r->get('/')->to('Example#welcome');

  my $home = $self->home;
  my $dbfile = "$home/myapp.db";
  my $dsn = "dbi:SQLite:$dbfile" ;

  my $m = DBIx::Migration->new( { dsn => $dsn , dir => "$home/sqlite" });
  $m -> migrate(1);

  $self->plugin ( 'Yancy' => { backend => "sqlite:$dbfile" });

  $self->log->info ( "application is ready");

}

1;
preaction commented 1 year ago

Ah, I think the problem is more that Yancy is trying to use the status field as the ID for URLs. Setting x-id-field to explicitly use the id column for that table could be one way to fix this. A more robust way might be for Yancy's schema reader to not try to choose those reserved stash values as ID fields. If neither of those end up working, we might have to start adding prefixes to the stash names for ID fields (which I hope it does not come to that, as I think that may break some existing sites).

rmallah commented 1 year ago

thanks for clearly , explaining the root-cause and hinting the fix ( to use x-id-field feature) . I will try to see the code and create PR based on the suggestion which sounds reasonable.