hkulekci/qdrant-php

Add getters for structs.

gregpriday opened this issue · 6 comments

I've found a few instances where I needed to get variable values from structs. Namely MultiVectorStruct, PointsStruct, PointStruct, VectorStruct. A very simple solution would be to make these variables public, but a more maintainable solution might be to have a single trait for all structs that allows get access to all protected variables.

I haven't checked this implementation, but something like this:

<?php
namespace Qdrant\Traits;

use InvalidArgumentException;
use ReflectionProperty;

/**
 * Trait ProtectedPropertyAccessor
 *
 * Allows access to protected properties through the magic __get method.
 */
trait ProtectedPropertyAccessor
{
    /**
     * Magic method to implement generic getter functionality for protected properties.
     *
     * @param string $property The name of the property to get.
     * @return mixed The value of the property.
     * @throws InvalidArgumentException if the property doesn't exist or is not protected.
     */
    public function __get(string $property)
    {
        if (property_exists($this, $property)) {
            $reflection = new ReflectionProperty($this, $property);
            if ($reflection->isProtected()) {
                return $this->$property;
            } else {
                throw new InvalidArgumentException("Access to property '$property' is not allowed");
            }
        }

        throw new InvalidArgumentException("Property '$property' does not exist");
    }
}

What do you think @hkulekci?

Here's the PR if you're interested in adding this: #22

Could you please share a little snippet to see the problem to see why we need this type of trick?

It is a good opinion to be able to reach protected fields but I could not get the real point of what we are trying to solve exactly. And also, we need to think about testability. We just try to remove getter methods from the Struct classes, right?

Sorry, I just spent some time looking through my code and I remembered I actually needed this in SearchRequest, not the Structs. I can change the PR I opened.

I needed it to implement pagination. I need to know a bit about the SearchRequest to get the total results count, here https://github.com/gregpriday/laravel-scout-qdrant/blob/develop/src/Scout/QdrantScoutEngine.php#L268 - I still need to finish that function so it uses Qdrant's core count feature when possible.

My workaround was to pass these variables here https://github.com/gregpriday/laravel-scout-qdrant/blob/develop/src/Scout/QdrantScoutEngine.php#L205-L209, but I'd much rather rely on the SearchRequest as the single source of truth.

Overall though, making these variables accessible can only help other developers integrate with Qdrant. To quote the Python guidelines, "we are all consenting adults here".

In terms of testing, we can add a simple test for the trait that it allows access to protected variables and blocks access to private variables.

I think we are done with this, right? Please feel free to close it if it is. @gregpriday

All done, yes. Sorry, just saw this now.