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

Defining operations without reflection #751

Open
HoldYourWaffle opened this issue Nov 22, 2023 · 6 comments
Open

Defining operations without reflection #751

HoldYourWaffle opened this issue Nov 22, 2023 · 6 comments

Comments

@HoldYourWaffle
Copy link
Contributor

Operations use reflection to determine the parameters and return type of their callable.
This works great, but only if your operations can be defined statically, as in "at the time of writing code".

I'm trying to write a "proxy" that exposes some archaic esoteric legacy system as an OData API.
Long story short, it's impossible to statically define this system's operations, they're derived from all sorts of places including user configuration.

Therefore I need a way to dynamically specify operations, for example something along these lines:

$archaicOperations = array(
    'foo' => array(
        'arguments' => array(
            'a' => array('type' => 'string', 'nullable' => false),
            'b' => array('type' => 'string', 'nullable' => false),
        ),
        'returnType' => 'string',
    ),
    'bar' => array(/*...*/)
);

foreach ($archaicOperations as $name => $operation) {
    $function = new Operation\Function_($name);
    
    $function->setCallable(function (...$arguments) use ($name) {
        // ...validate arguments...
        return fetchFromArchaicSystem($name, $arguments);
    });

    $function->setArguments(array_map(function ($argument) {
        $a = new Operation\Argument();
        $a->setName($argument['name']);
        $a->setType($argument['type']);
        $a->setNullable($argument['nullable']);
        return $a;
    }, $operation['arguments']));
    $function->setReturnType($operation['returnType']);
    
    Lodata::add($function);
}

I took a shot at decoupling Operation from reflection, but unfortunately I ran head-first into a whole lot of trouble, including but certainly not limited to:

  1. Operation::returnsCollection relies on resolving the return type to a class name for is_a, but Operation::returnType is of type Type. Type::factory should contain a class name, but for some reason this doesn't seem to always be the case.
  2. What is the intended usage of the existing Operation::setReturnType?
    Seems to be used for callbacks labeled with Entity or EntitySet, but it's not respected everywhere. Operation's returnsCollection and isNullable always use the reflection-based getCallableReturnType.

I'm going to keep investigating this, but given my extremely limited knowledge of Lodata's internals I figured I might as well get some input from those in the know :)
What are your thoughts on this kind of feature? Any suggestions on how this could be implemented?

@HoldYourWaffle HoldYourWaffle changed the title Magic callable operations Dynamically define parameters and return types for operations with a generic callable Nov 22, 2023
@HoldYourWaffle HoldYourWaffle changed the title Dynamically define parameters and return types for operations with a generic callable Dynamically define parameters and return type for operations with a generic callable Nov 22, 2023
@27pchrisl
Copy link
Contributor

Yes, OData has quite a statically defined outlook : )

Some of what you're running into is for the OData metadata, for example Operation::setReturnType is used so that the metadata can describe the type of Entity or EntitySet that will be produced, and put the right return type in the API response.

To work with the problem you've described, you could implement something a bit like JSON-RPC in OData, where you have a single 'invoke' function, you'd pass the actual method as a string parameter, and an set of arguments as an optional untyped array, and the response is just an open entity type or array. But in this case the OData client would need to understand how to handle this flow.

@HoldYourWaffle
Copy link
Contributor Author

First of all: thank you so much for your help!

A generic invoke is a very interesting idea, but I doubt Excel (the main client I have to work with) would work well with that. One of the main reasons I'd love an OData layer is discoverability, such a setup would counteract that.

It sounds like Operation::setReturnType is used to "override" whatever was found using reflection, so why is it not respected by Operation::returnsCollection and Operation::isNullable then?

Perhaps I'm missing something obvious here, but it seems odd that metadata is so tightly coupled to (static) reflection. Intuitively I'd consider "describing an operation" and "automatically loading a description with reflection" as two separate things. In principle other parts of the code aren't interested in how an operation was described, only what the description ended up being.
Is there a reason for this that I'm missing, or is this just a design choice that only becomes a problem when you need to do weird stuff like me? :P

@27pchrisl
Copy link
Contributor

I wrote this code quite a while ago, so my memory might be incorrect!

Operation::setReturnType isn't exactly an override, it adds additional type information. If your operation returns an Entity object, then Lodata wouldn't know the actual entity type you intend to return to include in the metadata, it needs you to provide that.

Based on your example it seems like you have a fixed operation name, but variable arguments.

While it's not pretty, if you truly have variable arguments for a specific function name can you do:

$f = new Operation\Function_('add');
$f->setCallable(function (?string $arg0, ?string $arg1, ?string $arg2, ?string $arg2): string {
// ...
});
Lodata::add($add);

@HoldYourWaffle
Copy link
Contributor Author

Hmmm, another interesting idea, but unfortunately the "set of available operations" is dynamic too in my case.
In the context of the example in my original post: $archaicOperations is fully dynamic and fetched from the archaic system.

I was working on decoupling the operation system from reflection a couple months ago, until I was rudely interrupted by other things.
I'd like to continue this work now, but I figured I might as well ask you upfront: is there anything preventing this from working, other than it not being implemented yet?
Earlier you mentioned that "OData has quite a statically defined outlook", but can't the OData metadata be generated dynamically?

@27pchrisl
Copy link
Contributor

Hiya, I was referring to this: https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html#sec_ModelVersioning where as long as you're only adding things then that's safe.

Of course there's no reason Lodata has to stick to that, if the developer knows what they're doing. So yes I'd be fine with the ability to add operations in ways other than via reflection.

@HoldYourWaffle
Copy link
Contributor Author

Oooh interesting, thanks for the reference and your lightning fast response!
I'll get to work on this as soon as I can, but it might be a while until I can actually share something useful.

For completeness: in principle, defining operations 'non-reflectively' doesn't necessarily mean they're 'dynamic'; they could for example be based on a static configuration file.
In my case it happens to be dynamic, but luckily this squarely falls into the 'user's responsibility' category :)

@HoldYourWaffle HoldYourWaffle changed the title Dynamically define parameters and return type for operations with a generic callable Defining operations without reflection Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants