nova-framework / framework

Demo application using Nova - this is an extensive playground, use "app" repository for real applications.
http://novaframework.com/
MIT License
418 stars 211 forks source link

SMVC restructure #222

Closed dcblogdev closed 8 years ago

dcblogdev commented 8 years ago

I've given the framework a bit of a restructure so it main files are outside of the web root by default and the only files that should be inside the web root are the templates, .htaccess and index.php.

so the structure is now like this:

app
    Controllers
    Core
    Helpers
    Language    
    logs
        error.log
    Models
    Modules
    vendor
    views
    composer.json
public_html
    templates
        default
    .gitignore
    .htaccess
    index.php
    licence.txt

From the index.php the starting path would be:

$smvc = '../';

The vendor path would be

if (file_exists(SMVC.'app/vendor/autoload.php')) {
    require SMVC.'app/vendor/autoload.php';
}

I've also adjusted the view class and url helper to make the paths more flexible.

This would mean all public files (templates) would be in the web root all classes would be outside the web root.

The logger class has been changes since it would no longer be able to serve html errors they would be stored in app/logs/error.log above the document root.

This kind of setup needs to happen, it's a basic security practice. I'm very late in changing this.

First off is there any objections to this?

Additionally should this be a sub part update 2.2.1 or warrant a full version increase? personally I think 2.2.1 would be sufficient no new functionality has been added.

Another area I'm not 100% on is Modules. They may need to be in the web root as well so any modules containing css can be loaded but at the same time they will contain classes so should be above the root! will require some thought on this one.

I haven't pushed this up yet I'm waiting to get your thoughts on this.

LuckyCyborg commented 8 years ago

@jimgwhit If you talk about my proposed Router ability to work without Apache's ModRewrite support, then yes, it can works fine without it, using URLs similar with those accepted by CodeIgniter. For example, the following URLs are working by default:

http://www.mywebsite.org/admin/users/edit/3

http://www.mywebsite.org/index.php/admin/users/edit/3

The second URL do not need the Apache's ModRewrite to be active. In other hand, my proposed Router, and, very important, even the SVMC's Macaw based Router, both do not handle the QUERY_STRING on URIs processing. Even some Big Houses do not do, i.e. CodeIgniter.

Stil, if the passed URI contains a QUERY_STRING, i.e. 'admin/users/list?page=4&search=jim', our Router (to be more precise, on Route::match()) automatically capture the query part, if exists, as the last parameter. So, if you have the Admin/Users Controller's method something like:

public function list($query = '')
{
...
}

On the method, the $query parameter will contains page=4&search=jim . One can explode this string, then map the parts as an array keys/values, eventually applying some XSS protection. Finally, guess what? You will obtain something similar with what already exists in $_GET.

So, assuming that you manually handle the QUERY parameters in your Controllers, will work by default the URL's like:

http://www.mywebsite.org/admin/users/list?page=4&search=jim

http://www.mywebsite.org/index.php/admin/users/list?page=4&search=jim

And, if we talk about Laravel, it's Routing complexity is about several magnitudes greater than the proposed code, so is absolutely normal to not have all Laravel's Routing abilities.

But, you know what is the best part, if Dave accept our Router? If you can't live without handling the QUERYSTRING via Routing, you can easily extend the (base) \Core\Router_ and to create and use one which automatically handle those things.

Again, we propose a simple, yet improved Routing, which can be easily extended to support new facilities, not the final Routing which we use on the e-Learning Portal, which is much more complex, especially on Files handling.

I.e. our production Router can and condition the User access to some files by: the User is logged in, the User taken a specified Course, he is in a 3 hours time limit, and have given access for the current IP only.

To respond to your last question, there is no public repository for our code. In fact, the Portal for what we have interest on SVMC, is written on request for the final owner, and it will not be published as source code and/or for downloading, and the SMVC license permit well that way. Yet, eventually we muse about publishing the Core Framework as add-on on top of SMVC, but this is another story.

Also, I'm there as job task, being instructed to contact the SVMC community and to start the communication, eventually, to propose diverse enhancements.

@tomvlk As performance, compared with the SVMC's orthodox Router, our own tests say that our Routing is similar as speed, behaving slightly better in some situations. For example, to support something equivalent with a Route with optional parameters, like:

blog/archive(/:num(/:num(/:num)))

using the actual Routing, you need to define Four Routes, and the Router will loop over them. Instead, our Routing will get it on first shot.

PS. We have also, a (more complex) Routing variant which support also so called named parameters. Think about something like:

/blog/@controller/@method(/@param:num(/:num(/:num)))

if there is interest, we can give also the code of this variant.

tomvlk commented 8 years ago

@LuckyCyborg Thanks for the fast reply on the performance. And indeed you don't need multiple route definitions if you have optional paramters. This will reduce the number of routes defined and will reduce time to do the for loop. I really think that this will improve the router and should be in the new version, as the current router lacks on some features you have implemented.

jimgwhit commented 8 years ago

@BryanYeh in case you didn't see it, @daveismyname did a video on this:
http://simplemvcframework.com/screencasts/v2.1/build-a-login-authentication-system
Also an example I do is I made an admin controller and I

$username = $_POST['username'];
            $password = $_POST['password'];
            $darray = $this->Admin->getHash($username);

In model

public function getHash($username)
    {
        $data = $this->db->select("SELECT ownerid, password FROM " . PREFIX . "users WHERE
        username = :username", array(':username' => $username));
        return $data;
   }

and back in controller

if(password_verify($password, $darray[0]->password) == false){
              die('wrong username or password');  //put a redirect here
            } else {
                Session::set('loggin', true);
                Session::set('owner', $darray[0]->ownerid);
               Url::redirect('pet');
            }

In users table you could add fields for roles, sub roles, etc
For user to see there own data only:

public function getPets($offset = "", $rowsperpage = "", $petsearch = "")
    {
        $pagingQuery = "LIMIT {$offset}, {$rowsperpage}";
        $petsearch = $petsearch . "%";
       if (Session::get('owner') == '1') {
            return $this->db->select("SELECT * FROM " . PREFIX . "pets WHERE petname like :search ORDER BY petname " . $pagingQuery, array('search' => $petsearch));
        } else {
            return $this->db->select("SELECT * FROM " . PREFIX . "pets WHERE petname like :search AND ownerid = ownerid ORDER BY petname " . $pagingQuery, array('search' => $petsearch, 'ownerid' => Session::get('owner')));
        }
}

There are several ways to implement it. In example I have 1 as admin, but it could be whatever it needs to be for your requirements. Like

 if (Session::get('role') == 'admin') {

A lot of how authentication is done is customised anyway, Look at the docs for laravel, they have authentication, but just a starter with email and password, it's assumed if the programmer needs roles, sub roles, etc that the individual programmer is smart enough to program that added stuff in themselves.
That's not to say there are also packages on github that deal with authentication, but even in laravel you have to pull it in if you want it. In other words if you want a specialized authentication package you have to pull it in yourself in composer.
That's the way I was truely hoping this framework would stay, have the basic framework, but also have available various packages you can pull in with composer, (but only if wanted) not forced to use.
Getting back to authentication, I prefer to write my own, I would hate if someone was trying force me to use an authentication I didn't want. However if it was offered as an option, then I'd be more open to at least trying it.
To close, let me add if someone is using a framework, prior to using it, they should already have an understanding of basic password usage, authentication, and stuff like that. Just the basics, I don't mean be an expert.
There, I thought I was rather nice on this comment, I hope.

jimgwhit commented 8 years ago

@LuckyCyborg you mentioned exploding and mapping a querystring. In Laravel I don't have to explode and map nothing, I can have a route like

Route::get('pets', ['middleware' => 'auth', 'uses' => 'PetsController@petlist']);

pass a querystring the regular way and in controller

if (isset($_REQUEST['search'])) {
            $t1 = $_REQUEST['search'];
        } else {
            $t1 = "";
        }

Notice I am getting the querystring the normal way. I am not having to

public function petlist()  // nothing has to be passed in the parenthesis since regular querystring.
    {
      // more code
   } 

If the router has to change and can't use querystring like it does now, then I plead with @daveismyname let's use the routing system Laravel is using. I don't want to have to explode and map or do some special thing with a querystring. I want the querystring to work as always.
If this querystring thing is going to be an issue, someone let me know right now. I have real production sites, I use querystring, and that's that. I need to know how to proceed.
AND NO I am not being rude, I promise, but you are scaring the C**P out of me with that having to explode and map, rather than dealing with the querystring the regular way. The big THREE still use querystring the regular way, ZEND, SYMPHONY, LARAVEL.
@daveismyname I am not scraed to see the framework evolve, but evolve where still usable in a regular way. I hope you use regular querystring also, maybe you will feel the way I do about querystring usage.

LuckyCyborg commented 8 years ago

@jimgwhit Long story short, both Routing systems, the actual one and the one proposed by us, literally do not manage, change or modify the QUERY_STRING elements, which you can access via as always, via $_REQUEST and $_GET. Nothing is changed or will be changed, if you massive use the QUERY_STRING in your applications, via PHP's default ways. OKay?

In other hand, our Routing do not have the complexity of the Laravel Routing and do not want to be a Routing similar with the Laravel ones. It is just a enhanced version of the existent one, even compatible as routes configuration, nothing more.

Also, if your typical routing looks like:

Route::get('pets', ['middleware' => 'auth', 'uses' => 'PetsController@petlist']);

permit me to ask, on which framework are written your applications? SVMC or Laravel?

jimgwhit commented 8 years ago

@LuckyCyborg I have one using laravel, others are SMVC. Please explain in greater detail what you meant about having to explode and map the querystring, please.

LuckyCyborg commented 8 years ago

@jimgwhit Nothing more than that our Routing have also the ability to parse the URI and to offer the non-processed QUERY_STRING part as last parameter to a Controller Method, IF the passed URI contains it. That's all.

To note that the URI used by our Routing is per-processed by \Helpers\Url::detectUri(), which ignore/strip the QUERY_STRING, just like do also Dave's Router based on Macaw. There is nothing changed. Both Routers gracious ignore the QUERY_STRING.

BUT, YOU DO NOT NEED to do something with this supplementary parameter, even you can ignore it. You can just use as always your $_REQUEST and $_GET friends, which are untouched, and let's be happy. :)

jimgwhit commented 8 years ago

IN smvc I have

Router::any('pet/index', 'Controllers\Pet@index');

after a successful edit I can pass a regular querystring in url

Url::redirect('pet?p=' . Session::get('petpage') . '&search=' . Session::get('petsearch'));

which would look like

site/pet?p=2&search=ro  //just example here

I don't use what's call friendly URL's so I dont mess with route filters.


    (:any) any - can use characters or numbers
    (:num) num - can only use numbers
    (:all) all - will accept everything including any slash paths

I pass data with the regular querystring in the url.
When passing data with a querystring, you don't need or use route filters.
the regular route such as

Router::any('pet', 'Controllers\Pet@index');

will work.
And of course in the controller you can use
$_GET or $_REQUEST or the helper class like

$t1 = Request::get('search');  //using request helper

But I do not have to do a special thing to explode and map nothing.

LuckyCyborg commented 8 years ago

@jimgwhit Everything which you describe is not changed a bit using rather the today SMVC Routing, even you have to use the proposed one, which is punctually compatible with the existent one as Routes configuration. Again, absolutely nothing will be changed from your POV.

jimgwhit commented 8 years ago

@LuckyCyborg you said non-processed QUERY_STRING part as last parameter to a Controlle Method
Show an example route ofhow this would work please and possible the top part of controller where you are getting the data. for an example can we say that we are passing in the url:

site/pet?p=2&search=b&color=black

Please give example.

LuckyCyborg commented 8 years ago

@jimgwhit If you want to see, what you have additionally, if you use our Routing, please give me the actually SVMC Route which you use to map this URI, and your complete Controller method to process it.

In other hand, for those who want to test the performance or even the new introduced abilities of our proposed Routing, I prepared a repository and a branch where are added the classes and modifications discussed there.

https://github.com/LuckyCyborg/simple-mvc-framework/tree/3.0-improved-routing

jimgwhit commented 8 years ago

My routes for this

Router::any('pet', 'Controllers\Pet@index');
Router::any('pet/index', 'Controllers\Pet@index');
Router::any('pet/add', 'Controllers\Pet@add');
Router::any('pet/edit', 'Controllers\Pet@edit');

controller // not cleaned up, this is test data

public function index()
    {
        echo 'made it here';
        Session::set('owner', '1');
        echo Session::get('owner');
        //$this->_pet = new Models\pet_model();
        //$this->_contacts = new \Models\contacts();
        //Session::set('petsearch','b');
        //echo "ps".Session::get('petsearch');
        $petsearch = (isset($_REQUEST['psch']) <> '' ? $_REQUEST['psch'] : "");
        //echo "ps".Session::get('petsearch');
        Session::set('petsearch', $petsearch);
        //$_SESSION['petsearch'] = "b";
        //$_SESSION['petsearch'] = $petsearch;
        //$pm = new \Models\PetModel();
        $petrows = $this->Pet->petCount($petsearch);
        echo "pr" . $petrows . "pr";
        print_r($this->Pet->petCount($petsearch));
        //$_SESSION['petrows'] = $petrows;
        Session::set('petrows', $petrows);
        $pages = new HelpersPaginator('5', 'p');
        //$pages = new \helpers\paginator('5','');
        //$_SESSION['petpage'] = $pages->get_instance();
        Session::set('petpage', $pages->getInstance());

        $pages->setTotal($petrows);
        //$data['page_links'] = $pages->page_links();
        $data['pageLinks'] = $pages->pageLinks('?', '&psch=' . $petsearch);
        $data['title'] = 'pet';
        $data['pets'] = $this->Pet->getPets($pages->getLimit2(), $pages->getPerpage(), $petsearch);
        //good $data['pets'] = $this->_pet->get_pets($pages->get_limit(), $petsearch);
        $this->view->renderTemplate('header', $data);
        $this->view->render('pet/index', $data);
        $this->view->renderTemplate('footer', $data);
    }

I added the &color=black to see what an extra querystring parameter would do.

jimgwhit commented 8 years ago

@LuckyCyborg I just wanted to see how a route in your routes would change for this

site/pet?p=2&search=b&color=black

Surley you can show example and using the requesrt helper to retrieve the data please. Not for real, just the code it would look like.

LuckyCyborg commented 8 years ago

@jimgwhit Sorry for delayed response, but I haven some things to do.

So, there is NO CHANGE in your Routes. Your Controller's Method will work with no problem.

Looks like you use:

Router::any('pet', 'Controllers\Pet@index');
Router::any('pet/index', 'Controllers\Pet@index');
Router::any('pet/add', 'Controllers\Pet@add');
Router::any('pet/edit', 'Controllers\Pet@edit');

Those Routes are perfectly compatible with the proposed Router. NOTHING to change in your part. Eventually, if you talk about SMVC 3.0, because @daveismyname decided this way, you should modify the Controllers namespace. Something like:

Router::any('pet', 'App\Controllers\Pet@index');
Router::any('pet/index', 'App\Controllers\Pet@index');
Router::any('pet/add', 'App\Controllers\Pet@add');
Router::any('pet/edit', 'App\Controllers\Pet@edit');

Note that this change is required your Routing to work on the default SMVC 3.0, not by our Routing.

Like I said, this Route compatibility is obtained by Design.

Still, if you want to test the "mysterious" query feature, and the QUERY string part to arrive on Routing, you should modify on \Helpers\Url:

    public static function detectUri()
    {
        $requestUri = $_SERVER['REQUEST_URI'];
        $scriptName = $_SERVER['SCRIPT_NAME'];

        $pathName = dirname($scriptName);

        if (strpos($requestUri, $scriptName) === 0) {
            $requestUri = substr($requestUri, strlen($scriptName));
        }
        else if (strpos($requestUri, $pathName) === 0) {
            $requestUri = substr($requestUri, strlen($pathName));
        }

        if (($requestUri == '/') || empty($requestUri)) {
            return '/';
        }

        $uri = parse_url($requestUri, PHP_URL_PATH);

        return str_replace(array('//', '../'), '/', ltrim($uri, '/'));
    }

AS:

    public static function detectUri()
    {
        $requestUri = $_SERVER['REQUEST_URI'];
        $scriptName = $_SERVER['SCRIPT_NAME'];

        $pathName = dirname($scriptName);

        if (strpos($requestUri, $scriptName) === 0) {
            $requestUri = substr($requestUri, strlen($scriptName));
        }
        else if (strpos($requestUri, $pathName) === 0) {
            $requestUri = substr($requestUri, strlen($pathName));
        }

        if (($requestUri == '/') || empty($requestUri)) {
            return '/';
        }

        return str_replace(array('//', '../'), '/', ltrim($requestUri, '/'));
    }

Note that I have eliminated the parse_url() at the end of method. After those modifications, is supposed to have a copy of the current query_string as last parameter for method. For example, if you change:

public function index($str = '')

when using the base Core\Router, not the ClassicRouter, the $str variable should contains p=2&search=b&color=black.

NOTE that this is an additional feature, used by us on some extensions of this (proposed) Routing, then you can completely ignore this ability and use the normal style, as always.

In fact, while I published this proposed Routing code assembled in a functional branch tree, you are free to take it and play as you like to see yourself if there are differences on behavior between our Routing and the actual one. There you want to go:

https://github.com/LuckyCyborg/simple-mvc-framework/tree/3.0-improved-routing

All the best!

jimgwhit commented 8 years ago

@LuckyCyborg I don't understand

public function index($str = '')

And you didn't show how to use the request helper to get the two values

p=2&search=b&color=black

Search and color. The p is pagination. In controller would I still


$t1 = Request::get('search');

$mycolor = Request::get('color');

If this isn't correct, what is the way using your proposal? Example please of retrieving this in a controller. And how is p passed to paginate?
And why would I need

($str = '')
jimgwhit commented 8 years ago

Currently in Laravel nothing extra is needed in the controller method for a querystring to work.
Still looks like

public function index()

So why in yours do we need str=''. As stated if @daveismyname is going to drastically change routing, why not Just use the one Laravel is using so no special technique has to be accomplished to get a querystring passed.

LuckyCyborg commented 8 years ago

@jimgwhit Literally, like I said, you should ignore this feature of passing the raw query as last parameter to a Controller Method, if you have no intention to extend the Routing in special ways, or to do special processing in your Controllers.

To note also that this thing is disabled/ignored by default by Url::detectUri() and there are no difference of behaviour on QUERY processing, compared to actual Router. Read: the QUERY_STRING is ignored and you should retrieve the query_string by standard methods.

woodsy1 commented 8 years ago

@jimgwhit perhaps you should take this off the thread with @luckycyborg Your questions aren't making sense he has answered you a number of times and you have just clogging the thread with the same question over and over On Dec 12, 2015 6:25 AM, "jimgwhit" notifications@github.com wrote:

@LuckyCyborg https://github.com/LuckyCyborg I don't understand

public function index($str = '')

And you didn't show how to use the request helper to get the two values

p=2&search=b&color=black

Search and color. The p is pagination. In controller would I still

$t1 = Request::get('search');

$mycolor = Request::get('color');

If this isn't correct, what is the way using your proposal? Example please of retrieving this in a controller. And how is p passed to paginate?

And why would I need

($str = '')

— Reply to this email directly or view it on GitHub https://github.com/simple-mvc-framework/framework/issues/222#issuecomment-164025930 .

jimgwhit commented 8 years ago

He still hasn't show controller example.

woodsy1 commented 8 years ago

Get his email talk with him directly. The answer is dead simple he has stated it many times YOU CAN DO EXACTLY WHAT YOUR DOING NOW On Dec 12, 2015 6:38 AM, "jimgwhit" notifications@github.com wrote:

He still hasn't show controller example.

— Reply to this email directly or view it on GitHub https://github.com/simple-mvc-framework/framework/issues/222#issuecomment-164030057 .

jimgwhit commented 8 years ago

@woodsy1 having to put str='' is not the same, and frankly I don't care if that's no big deal to you but it is a real big deal to me yes it is. Remember I also have a right to voice my opinion I'd rather have the Laravel router. Now that's my honest to goodness opinion if you want something else that is your opinion which you are entitled to. But until he shows the controller example like I asked the complete question has not been answered.

tomvlk commented 8 years ago

Maybe a good idea to create a seperate issue for the discussions for the different components in 3.0. Because this isn't anymore about the folder restructure but more like feature requests etc.

Btw, @jimgwhit you said before: "Couldn't they install the framework, and custom write their own classes and customize in those features they want themselves?" :hushed: . Already regret saying it as I can see. Lol.

woodsy1 commented 8 years ago

He has answered you a number of times!!!!! This answers your question "public function index($str = '')

when using the base Core\Router, not the ClassicRouter, the $str variable should contains p=2&search=b&color=black.

NOTE that this is an additional feature, used by us on some extensions of this (proposed) Routing, then you can completely ignore this ability and use the normal style, as always."

THEN YOU CAN COMPLETELY IGNORE THIS ABILITY AND USE THE NORMAL STYLE AS ALWAYS On Dec 12, 2015 6:46 AM, "jimgwhit" notifications@github.com wrote:

@woodsy1 https://github.com/woodsy1 having to put str='' is not the same, and frankly I don't care if that's no big deal to you but it is a real big deal to me yes it is. Remember I also have a right to voice my opinion I'd rather have the Laravel router. Now that's my honest to goodness opinion if you want something else that is your opinion which you are entitled to. But until he shows the controller example like I asked the complete question has not been answered.

— Reply to this email directly or view it on GitHub https://github.com/simple-mvc-framework/framework/issues/222#issuecomment-164031979 .

jimgwhit commented 8 years ago

And @woodsy1 rather than take the stabs at me, how come you don't put in some ideas, suggestions, code fragments, or whatever to help version 3. You're welcomed to skip over the comments above you do not want to read. But just remember a router change is a major thing not some little tiny thing to be taken lightly. But if I could get the controller example I would drop it.

jimgwhit commented 8 years ago

That's a good idea I will start a new issue.

woodsy1 commented 8 years ago

The answer is there it will be exactly the same as you have now. Exactly. Nothing will change.

I am very interested in reading the comments and helping where I am needed but in this case he has already answered you a number times but you are not seeing it. Please use @tom suggestion and take it to a new thread or message him privately. Or take hour on advice and customise it yourself. On Dec 12, 2015 6:53 AM, "jimgwhit" notifications@github.com wrote:

And @woodsy1 https://github.com/woodsy1 rather than take the stabs at me, how come you don't put in some ideas, suggestions, code fragments, or whatever to help version 3. You're welcomed to skip over the comments above you do not want to read. But just remember a router change is a major thing not some little tiny thing to be taken lightly. But if I could get the controller example I would drop it.

— Reply to this email directly or view it on GitHub https://github.com/simple-mvc-framework/framework/issues/222#issuecomment-164033465 .

LuckyCyborg commented 8 years ago

@tomvlk I dare to say that the thread title is indeed very generous: SMVC restructure So let's restructure it! :)

In other hand, how about to improve the configuration a bit? And to give to SVMC 3.0 a Inflector?

For configuration, I propose the very simple class mentioned in the first post:

namespace Core;

class Config {

    protected static $settings = array();

    public static function get($key) {
        return isset(self::$settings[$key]) ? self::$settings[$key] : null;
    }

    public static function set($key, $value) {
        self::$settings[$key] = $value;
    }

}

It's usage is very simple, and it is very versatile. For example, if you want to do a active Modules list, you can do:

Config::set('modules', array(
    'System',
    'Cron',
    'Center',
    'User',
    'Menu',
    'Page',
    'Template',
    'MediaManager',
    'Contact',
    'WebAudio',
    'WebChat',
    'CDN',
    'Exam',
    'Course',
));

Yep, we can store in it even arrays, in fact, we can store everything that can be stored in a array, and the built-in behavior is global read access. Of course, this is not a replacement of the constants (aka defines), but a companion. For example, we can have the following files:

app/config/bootstrap.php
app/config/config.php
app/config/constants.php
app/config/routes.php

and in public/index.php, instead of standard Config instantiate, just:

require SMVC.'config/bootstrap.php';
require SMVC.'config/routes.php';

The SMVC/config/bootstrap.php can contains some global useful functions, to include the config.php and constants.php and to do the initialization stage. For example, can contains:

use Helpers\Session;

        /**
         * Turn on output buffering.
         */
        ob_start();

        require SMVC.'config/constants.php';
        require SMVC.'config/config.php';

        /**
         * Turn on custom error handling.
         */
        set_exception_handler('Core\Logger::ExceptionHandler');
        set_error_handler('Core\Logger::ErrorHandler');

        /**
         * Set timezone.
         */
        date_default_timezone_set('Europe/Rome');

        /**
         * Start sessions.
         */
        Session::init();

The constants and config file content is oblivious.

That's all! We have a very simple configuration, which can be extended easily, dedicated per (every) application(s).

The second class, the Inflector, is glorious simple and bugs prone:

<?
php namespace Helpers;

class Inflector extends \Doctrine\Common\Inflector\Inflector
{
}

Yep, I propose to use the Doctrine's Inflector, and the integration is trivial. A new line in composer.json and this class.

"doctrine/inflector": "1.0.*",

And a last suggestion about PHPMailer. Why is needed to ship it? It is available in the Composer repositories. For example, we can add in the composer.json file:

"phpmailer/phpmailer": "~5.2"

and to use a simple wrapper class to integrate it in SVMC.

All the best!

mickacz commented 8 years ago

What do you think about the possibility of grouping routes?

Router::group('admin', function() {

    Router::get('/', 'App\Controllers\Admin\AdminController@index');
    Router::get('/some', 'App\Controllers\Admin\AdminController@some');
    (...)

});

or the optional second callback

Router::get('some', 'App\Controllers\Auth@isLogin', 'App\Controllers\Some@some');
Router::get('news/(:num)', 'App\Controllers\News@addVisit', 'App\Controllers\News@showNews');
LuckyCyborg commented 8 years ago

@mickacz First of all, we preferred to show up with a minimal but very efficient and extensible Routing structure, hopping to not be considered too fat (as code) or complicated, to be accepted as replacement for the ol'good Macaw Son.

But yes, we considered the Route Grouping and we believe that our (proposed) Routing can be easily extended to support that ability.

Eventually, in future, if this Route Grouping presents a huge interest from SVMC community, we can implement it native in the base Router.

In other hand, what you describe in the optional second callback is something rather simple to be implemented via a good Auth Service and the via mentioned Events and EventManager.

First one is a typically Controller's Authorization guarding case, easily to be implemented via an Authorize helper or service, the second one is rather a typical case of Event propagation, IF there is an EventManager, which not exist.

Yet, we can create, with no problems, some simple EventManager for SVMC, if there is enough interest...

All the Best!

jimgwhit commented 8 years ago

@woodsy1 me and @LuckyCyborg worked it out, didn't mean any harm, but I like minute detail, I just needed more detail. Are you going to help test some of this stuff and report your findings? Believe me I know I rant, but with real sites out there, coupled with lots of testing I've done in the past, I have stood by this framework through thick and thin. Yes major changes makes be very nervous.

LuckyCyborg commented 8 years ago

Hello,

as response to @mickacz, I made an ExtendedRouter class, who support the Route Groups.

namespace App\Core;

class ExtendedRouter extends \Core\Router
{
    private static $routeGroup = '';

    // Constructor
    public function __construct()
    {
        parent::__construct();
    }

    public static function group($group, $callback)
    {
        // Set the current Routes Group 
        self::$routeGroup = trim($group, '/');

        call_user_func($callback);
        // Reset the Routes Group to default (none).
        self::$routeGroup = '';
    }

    public function addRoute($method, $route, $callback)
    {
        $method = strtoupper($method);
        $pattern = ltrim(self::$routeGroup.'/'.$route, '/');

        $route = new Route($method, $pattern, $callback);

        array_push($this->routes, $route);
    }

}

This simple Router extension supports the routes group commands, just like was requested:

ExtendedRouter::group('admin', function() {
    Router::get('/', 'App\Controllers\Admin\AdminController@index');
    Router::get('/some', 'App\Controllers\Admin\AdminController@some');
});

Alternatively, if we accept to use the $router instance, there is no need to use a static method, and the Router class can configure itself the right Router:

$router = Router::getInstance();

$router->group('admin', function() {
    Router::get('/', 'App\Controllers\Admin\AdminController@index');
    Router::get('/some', 'App\Controllers\Admin\AdminController@some');
});

To activate the ExtendedRouter, we need on Config:

/**
 * Set the Application Router.
 */
define('APPROUTER', '\App\Core\ExtendedRouter');

Like everyone can see, the design of our (proposed) Routing permit an easily and punctually customization and extension of the Routing behavior, without even touching the SVMC's System files.

All the best!

jimgwhit commented 8 years ago

@LuckyCyborg you have

$router = Router::getInstance();

$router->group('admin', function() {
    Router::get('/', 'App\Controllers\Admin\AdminController@index');
    Router::get('/some', 'App\Controllers\Admin\AdminController@some');
});

But what if admin controller is underneath controllers like

$router = Router::getInstance();

$router->group('admin', function() {
    Router::get('whatever/', 'App\Controllers\AdminController@whatEver');
    });

Also the same define is being used couldn't the define for the extended router be named something else that way regular routing and grouping could it still be done.
Another question should the pull request be closed for now while you tweak this and then you could put in a new pull request after testing all.

jimgwhit commented 8 years ago

@woodsy1 speaking of routers how is your exhaustive test of the new routing system coming along?

dcblogdev commented 8 years ago

@jimgwhit have you tried it? it looks like it's called by its namespace.

I'm having a few problems with @LuckyCyborg's routes I've commented on the PR for it.

jimgwhit commented 8 years ago

I tried it last night with a querystring, haven't tried it with grouping, I'm waiting til all tweaks are finished. @LuckyCyborg let me know in issue #232 when you have all tweaks completed and I will retest. But I will only be basic testing querystring I'll leave it to @daveismyname to test friendly URL also.

LuckyCyborg commented 8 years ago

@jimgwhit OK, I will talk with you in #232 and with @daveismyname in #231

woodsy1 commented 8 years ago

@jimgwhit do not belittle me with a comment like that. I am getting very tired of the comments you are making. On Dec 13, 2015 4:37 AM, "LuckyCyborg" notifications@github.com wrote:

@jimgwhit https://github.com/jimgwhit OK, I will talk with you in #232 https://github.com/simple-mvc-framework/framework/issues/232 and with @daveismyname https://github.com/daveismyname in #231 https://github.com/simple-mvc-framework/framework/pull/231

— Reply to this email directly or view it on GitHub https://github.com/simple-mvc-framework/framework/issues/222#issuecomment-164170726 .

jimgwhit commented 8 years ago

No @woodsy1 any help testing new stuff would be good. You might use the framework in a way I don't, as stated I only tested for querystring. If you are using friendly URLs how about give it a test and let us know if it works. @LuckyCyborg has the latest changes on their forked site now.

jimgwhit commented 8 years ago

And @woodsy1 I would never want to belittle you, I worry about major changes, may sound argumentative at times, but I do not like that belittle thing. Let's be all in this together. And perhaps Before we jump all over a good sounding change let's test, test, and when we think it's good test again.

woodsy1 commented 8 years ago

I have done some testing but mostly around the usability with git. It seems a lot people don't use git so I have been spending my time preparing some material for a tutorial on that which will hopefully massively help people with deployments! On Dec 13, 2015 10:33 AM, "jimgwhit" notifications@github.com wrote:

And @woodsy1 https://github.com/woodsy1 I would never want to belittle you, I worry about major changes, may sound argumentative at times, but I do not like that belittle thing. Let's be all in this together.

— Reply to this email directly or view it on GitHub https://github.com/simple-mvc-framework/framework/issues/222#issuecomment-164201862 .

dcblogdev commented 8 years ago

I've accepted the PR for the new router works a lot better than the previous version.

Also I've added a paths.php file under public that define absolute paths:

/** Define the absolute paths for configured directories */
define('APP', dirname(__DIR__).'/app/');
define('SYSTEM', dirname(__DIR__).'/system/');
define('PUBLICDIR', dirname(__FILE__).'/');
define('ROOT', dirname(__DIR__).'/');

the paths inside index.php have been removed and replaced with a require

require('paths.php');

The SMVC constant has been renamed to APP since it maps to the app directory. All referenced to SMVC have been updated.

Some items discussed previously not yet implemented is group routes and assets serving, I think both are good suggestions and would like to implement them with @LuckyCyborg's help.

Also @tomvlk I'd like to look at your proposal too I haven't forgot about the services, correct me if I'm wrong but this appears it would work as an additional classes so getting it setup should be a fairly quick process.

@woodsy1 thanks that's great I'll get the tutorial thread setup on the forum.

jimgwhit commented 8 years ago

@daveismyname with the router on your thoughts, did you remember to put the logger-memory PR in the ver 3 beta yet? Going to do new test now.

dcblogdev commented 8 years ago

Yes that was accepted shortly after it was opened

jimgwhit commented 8 years ago

@daveismyname see issue #233 please, but get some more rest. You had a hard day with that router thing. Now sleep.

jimgwhit commented 8 years ago

@woodsy1 you mentioned
I have done some testing but mostly around the usability with git.
Does git also work when uploading to a shared host like Godaddy? That would be nice.

woodsy1 commented 8 years ago

That is exactly what I have been testing :) It will change your life!! On Dec 13, 2015 3:50 PM, "jimgwhit" notifications@github.com wrote:

@woodsy1 https://github.com/woodsy1 you mentioned

I have done some testing but mostly around the usability with git. Does git also work when uploading to a shared host like Godaddy?

— Reply to this email directly or view it on GitHub https://github.com/simple-mvc-framework/framework/issues/222#issuecomment-164223291 .

jimgwhit commented 8 years ago

Let us know where the tutorial is once complete please.

woodsy1 commented 8 years ago

It will be on the forums in the new tutorial section Dave has setup. I have started helping Dave out with the forums so he can focus on the framework On Dec 13, 2015 3:59 PM, "jimgwhit" notifications@github.com wrote:

Let us know where the tutorial is once complete please.

— Reply to this email directly or view it on GitHub https://github.com/simple-mvc-framework/framework/issues/222#issuecomment-164223493 .

jimgwhit commented 8 years ago

Thanks, I am better now, that router thing was driving me to madness. But all worked out. It's great you're helping with the forums.

woodsy1 commented 8 years ago

No worries all good On Dec 13, 2015 4:02 PM, "jimgwhit" notifications@github.com wrote:

Thanks, I am better now, that router thing was driving me to madness. But all worked out.

— Reply to this email directly or view it on GitHub https://github.com/simple-mvc-framework/framework/issues/222#issuecomment-164223553 .