notrab / dumbo

A lightweight, friendly PHP framework for HTTP.
MIT License
158 stars 16 forks source link

fix: app config setget #39

Closed darkterminal closed 2 months ago

darkterminal commented 2 months ago

Fix related issue: #28

darkterminal commented 2 months ago

@notrab What about using Trait and used in Dumbo and Context class, so we don't have duplicated methods.

notrab commented 2 months ago

@darkterminal I'd be open to the Trait, I've not used those yet, so would be good learning exercise reading your code. What would it look like?

darkterminal commented 2 months ago

Dance!

darkterminal commented 2 months ago

@notrab Here we goo... I use setVar and getVar method in HasConfig trait so it's not duplicates with other method.

notrab commented 2 months ago

Hey @darkterminal, I took a step back and revisited the Router/Dumbo class context to see if we could modify what is there now to achieve what we need.

I remember that when I first added get and set for the route context, I always wanted this to work via middleware. Then when route groups were added, I thought the current behaviour would just work. It turns out, from our discussions above this is not the case.

So, I propose a new API that is more flexible but retains the current behaviour for getting and setting context. Here is the new API:

<?php

require "vendor/autoload.php";

use Dumbo\Dumbo;

$app = new Dumbo();

// Context middleware for all routes
$app->use(function ($context, $next) {
    $context->set("message", "Hello Dumbo!");

    return $next($context);
});

$app->get("/", function ($context) {
    $message = $context->get("message");

    return $context->json([
        "message" => $message,
    ]);
});

// Route specific context via middleware
$app->use("/api", function ($context, $next) {
    $context->set("databaseUrl", "something-connection-uri");
    $context->set("authToken", "my-secret-auth-token");

    return $next($context);
});

$app->get("/api", function ($context) {
    $databaseUrl = $context->get("databaseUrl");
    $authToken = $context->get("authToken");
    $message = $context->get("message");

    return $context->json([
        "message" => $message,
        "databaseUrl" => $databaseUrl,
        "authToken" => $authToken,
    ]);
});

$app->run();

I've pushed the changes to this PR. I would like to bring back the Trait, but maybe in another PR as I think we could use more Traits elsewhere to make things more pluggable.

darkterminal commented 2 months ago

What a great decision! You the idea is; keep the set and get in the same context scope. I didn't realize that. :rocket: