Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] What about making it RESTful? #488

Closed
sopitz opened this issue Aug 22, 2014 · 10 comments
Closed

[4.0] What about making it RESTful? #488

sopitz opened this issue Aug 22, 2014 · 10 comments

Comments

@sopitz
Copy link

sopitz commented Aug 22, 2014

I think this project is a pretty damn good way to secure an application. And it easily works out of the box with only ONE disadvantage: You can only use it in PHP projects.

One of the major benefits of a PHP project in my eyes is the ease of getting up and running. Why dont we use this advantage and port that script to another version. Let's have an restfull api that you can use in ANY project, regardless of platform or technology, which is still super easy to set up and get running.

What do you think? Bullshit idea or something worth to be discussed?

@panique
Copy link
Owner

panique commented Aug 22, 2014

Definitly a fantastic idea, but to be honest I don't have the skills to build a really professional thing. It's also a little bit out of scope of the php-application approach, so we might create a new repo for this.

@sopitz
Copy link
Author

sopitz commented Aug 22, 2014

I think the logic itself would be the same. No changes to the core would be necessary. In my mind (I actually never converted a MVC to REST yet) we would only have to change the way people can reach the application and the way we answer. We wouldnt send HTML to a browser but xml/json to the calling application. The same would go for the calls external applications could perform.

Repository wise: DEFINITELY a new repo. The only things these two would have in common would be the "core" logic. (Maybe it would be necessary to decouple the pure login functionality from the MVC framework during the transition and split repos as well. But that should be discussed later as it has nothing to do with neither of the repos.)

@atdotslashdot
Copy link
Contributor

I'm using my own fork but have been experimenting with some restful bits in my app - under certain conditions (if the view url is preceded by "/api/") I spit json data out.

I know the code is a little clunky but the top of my View->render function looks like this

...
if($GLOBALS['app']->output_json)
{
 if(!headers_sent()){
  header('Content-type: application/json');
        }

        if(isset($this->data))
        {
            $this->data=(object)$this->data;
        }
        if(isset($_SESSION["feedback_negative"]) && !isset($this->view->data->errors)){
            $this->view->data->errors=$_SESSION["feedback_negative"];
        }
         echo(json_encode($this->data));
         Session::set('feedback_positive', null);
         Session::set('feedback_negative', null);

        return;
 }
 ...

the view->data contains data specific to a particular view, usually an array of stuff extracted from a database in the controller.
I'm not suggesting this should be implemented as is but I'm wondering if it really is a new repo or just a tweak to View->render?

@sopitz
Copy link
Author

sopitz commented Aug 28, 2014

I will have a look into your implementation soon. But I think it should definitely get its own repo as its a whole different way of how you cope with that login stuff. Its not something you should/could use stand alone then but rather embedded it into an application pool.

@atdotslashdot
Copy link
Contributor

OK, but in my head it's not a whole different way . The server receives requests - it can either render html or json back to the client. The server logic should be identical in both cases. It's also useful way to keep model data separate in view/controller code....

1.Server receives requests
2.appropriate controller is called which extracts data from the model
3.the data is passed to the view which renders the data back to the client either in simple json arrays or wrapping some html round it.

As I said, I spit json out if the url is prepended with '/api/' but I'm sure there are other ways to swing the cat :-)

@sopitz
Copy link
Author

sopitz commented Aug 28, 2014

I know. The difference regarding is identical. But still it's a different purpose and you should make sure only either of the ways is possible to allow authentication (single point if responsibility). Otherwise you see yourself trapped in issues trying to figure out if you want our code to behave in a platforms manner or if it's your own app. The rest-login would serve the purpose to be a platform. Hence it should be designed and not render HTML or any other frontend related things. (At least that's what I learned from bigger restful java projects I worked in!)
I think the interface should be the same for all calls (either HTML or json) but the implementation of the response layer would have to differ.

@panique
Copy link
Owner

panique commented Oct 30, 2014

New ticket for that: #519 (and then realized that this thread already exists) :)

@panique panique changed the title What about making it RESTfull? [3.0] What about making it RESTful? Dec 1, 2014
@panique panique added this to the 3.0 milestone Dec 1, 2014
@panique panique added the v3.0 label Dec 1, 2014
@panique panique removed the v3.0 label Jan 25, 2015
@panique panique changed the title [3.0] What about making it RESTful? [4.0] What about making it RESTful? Jan 25, 2015
@panique
Copy link
Owner

panique commented Jan 25, 2015

The new version will be 3.0, not 2.1 (as the changes are too big), so let's move this to 4.0!

@panique panique removed this from the 3.0 milestone Jan 25, 2015
@sopitz sopitz removed this from the 3.0 milestone Jan 25, 2015
@panique panique added the v4.0 label Jan 25, 2015
@panique
Copy link
Owner

panique commented Oct 11, 2015

Hey, I'm currently "cleaning" the project a little bit and moving feature-requests like this to an own list inside the readme file (find it under the "future features" point). I hope you are okay with it, as most tickets here are new features and not really bugs or so.

My idea is just to avoid this project from getting oversized by too many features, so I'm closing the ticket, but for sure linking it from the readme in case somebody wants to implement this.

I hope you are all okay with this. :)

@panique panique closed this as completed Oct 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants