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

Add support for "include" #20

Closed
nanawel opened this issue Jul 10, 2023 · 17 comments
Closed

Add support for "include" #20

nanawel opened this issue Jul 10, 2023 · 17 comments

Comments

@nanawel
Copy link

nanawel commented Jul 10, 2023

As the code base grows and the number of pages increases, do you think this would be possible to add an "include" syntax that could help factorize some common elements in the pages?

Typically, I'd like to include shell.sql on top of each page to keep a common structure and the customizations I need accross all pages.

I suspect this could quickly complexify the structure so it might not be the right way to meet this need.

@nanawel
Copy link
Author

nanawel commented Jul 10, 2023

Aaand... right after posting I find

https://github.com/lovasoa/SQLpage/blob/3e9987971ec993b6f7d1c0cd9489f6eb98f3011d/examples/corporate-conundrum/game.sql#L1-L3

which simply solves the problem by storing the configuration in the database

https://github.com/lovasoa/SQLpage/blob/3e9987971ec993b6f7d1c0cd9489f6eb98f3011d/examples/corporate-conundrum/sqlpage/migrations/00_base_structure.sql#L52-L62

Simple but efficient. I guess this should be enough for all common cases.

@lovasoa
Copy link
Collaborator

lovasoa commented Jul 10, 2023

Indeed, I am not a fan on adding non-standard or non-SQL syntax to SQLPage. The whole mantra of the project is that It's just good old SQL.

But I'll rework the examples to make more consistent use of the kind of "page configuration database tables" like the one you highlighted that allows you to make a simple SELECT * FROM shell. Another advantage of the technique is that once you have that, you can make interface elements configurable by the site's users if you want, using a simple form.

@nanawel nanawel closed this as completed Jul 10, 2023
@mnesarco
Copy link

Hi Friends, is it possible to use a similar approach for protected pages preface? I want to add the following code to all protected pages but with a DRY approach:

SET username = (SELECT username FROM login_session WHERE id = sqlpage.cookie('session'));

SELECT 'redirect' AS component,
        'signin.sql?error' AS link
WHERE $username IS NULL;

@lovasoa
Copy link
Collaborator

lovasoa commented Dec 28, 2023

Hi @mnesarco !

This is on my roadmap, but is currently not available. What you can do at the moment to avoid repetition, if you are using a database that supports table-valued stored procedures, is

select * from my_auth_function(sqlpage.cookie('session'))

@mnesarco
Copy link

Hi @lovasoa !
Thank you. I have created a framework for Authentication and Authorization using stored procedures in postgresql for this. I will share it on github soon.

@lovasoa
Copy link
Collaborator

lovasoa commented Jan 4, 2024

I'd like to re-open this, since there is still no good general feature to allow code re-use in SQLPage.

What I am currently thinking of is a sqlpage.run_sql(sql_file, parameters) function that returns json that can in turn be used by the dynamic component. That would be powerful, because it would be composable with existing features.

One could easily do things such as

set personal_data = json_object('person_id', $id);
select 'dynamic' as component, sqlpage.run_sql('personal_info.sql', $personal_data) as properties;

@lovasoa lovasoa reopened this Jan 4, 2024
@matthewlarkin
Copy link
Contributor

Hi @lovasoa, thanks for re-opening. Your sqlpage.run_sql() function looks promising!

Though, for better readability, I wonder if something like this would be possible:

select 'card' from component
'You are logged in' as title
where sqlpage.run_sql('filters/logged-in.sql');

With filters/logged-in.sql being something like:

select coalesce((select 1 from sessions where token = sqlpage.cookie('session_token')), 0);

Here, maybe sqlpage.run_sql() returns the given sql statement's boolean result, which could then be piped into where and case statements. Would that be feasible?

That said, I imagine we'd need to be careful with those sql files, perhaps making them forbidden to the public but visible to SQLPage (in case the results are useful to our program but too sensitive for public consumption).


In the least, I do +1 the 'dynamic' component approach you mentioned.

@lovasoa
Copy link
Collaborator

lovasoa commented Feb 2, 2024

Returning a boolean is a little bit limiting. What I was thinking is having it return json, in a way that is compatible with the dynamic component. So, in your example, you would do

select 'card' from component, 'You are logged in' as title
where sqlpage.run_sql('filters/logged-in.sql')->>0->>'authorized' -- access the 'authorized' column of the first row returned by 'filters/logged-in.sql' 

With filters/logged-in.sql being something like:

select whatever as authorized;

@matthewlarkin
Copy link
Contributor

Yes, I see the JSON approach is quite flexible. And in SQLite, we'd do the following:

select 'card' from component, 'You are an editor' as title
where json_extract(sqlpage.run_sql('filters/editor.sql'), '$[0].access_level') = 4;

where sqlpage.run_sql('filters/editor.sql') returns a JSON array (not an object since we're returning rows):

[
    {
        "username" : "myusername",
        "access_level" : 4
    }
]

and filters/editor.sql is something like:

select username, access_level from users
where username = (
    select username from sessions where token = sqlpage.cookie('session_token')
);

Truly flexible! And would avoid a lot of repeated code 🙂

@lovasoa
Copy link
Collaborator

lovasoa commented Mar 8, 2024

I'm working on this in #253

Currently, the behavior is the following:

  1. Variable values are passed from the including file to the included file:
set x=1;
select 'dynamic' as component, sqlpage.run_sql('included.sql') as properties;

($x will be 1 inside included.sql)
2. Variable values are not passed back from included to including (forcing the including file to extract the result value of sqlpage.run_sql

What do you think ?

Another solution would be to force passing all values explicitly, probably in the form of

set x=1
select 'dynamic' as component, sqlpage.run_sql('included.sql', 'x', $x) as properties;

@matthewlarkin
Copy link
Contributor

Awesome to hear @lovasoa!

I like the simplicity of automatic passing and blocking included variable values 👏

I do see a benefit to forcing the values explicitly -- preventing unintended side effects if other variables are not intended to be shared with included.sql -- but I think side effects could be mitigated with good naming conventions.

@mnesarco
Copy link

I prefer the explicit args, that prevents shooting yourself in the foot. Implicit passing (aka gobal vars) can generate name collisions hard to debug.

@lovasoa
Copy link
Collaborator

lovasoa commented Mar 11, 2024

I'm not really sure about that... The included file simply has access to the current request, that is, the URL parameters (like ?x=1) through $x, as well as other request-specific parameters such as the current path (through sqlpage.path()).

The variables are not global, their scope is limited to the current request being executed. Do you see an example where this could be problematic ?

I don't want to think about SQLPage as a traditional programming framework, made to accommodate large and complex applications. SQLPage tries to give priority to simplicity, accessibility, and maintainability. I think this "variables can go only one way" rule works towards these goals.

I think most of the use of this feature will be something like

select 'dynamic' as component, sqlpage.run_sql('sqlpage/header.sql') as properties;

and then header.sql can implement some global rules for the application, handling authentication, generating the shell, etc... And it can use URL parameters for that.

What do you think ?

@mnesarco
Copy link

I think that people will use this feature in all possible expected and unexpected ways. Multiple levels of includes increases the risk of name collisions in such scenarios. Implicit argument passing is a kind of magical behavior. A little magic is good but too much magic is evil in the long run IMO.

@lovasoa
Copy link
Collaborator

lovasoa commented Mar 11, 2024

But is this "argument passing" ? $x allows accessing the URL parameter named x, wouldn't it be weird if it stopped working when in an included file.

Currently, visiting a sql file containing just

select 'dynamic' as component, sqlpage.run_sql('page.sql') as properties;

is the same as visiting page.sql directly. Not passing URL parameters to the file would break that...

@gourk
Copy link

gourk commented Mar 11, 2024

Hi,

This is awesome ! Thanks for your amazing work.

In my usage, I don't see any scenario where parameters should be useful. For sure, it can add a level of dynamism. Maybe you should release this first and add parameters later if it is needed.

@lovasoa lovasoa closed this as completed Mar 12, 2024
@lovasoa
Copy link
Collaborator

lovasoa commented Mar 12, 2024

Implemented in v0.20! Please test it and report any issue! 😁

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

5 participants