Upon model retrieval the model's query object forgets the selected columns

[dobiadi]

• Laravel Version: 6
• Adldap2-Laravel Version: 6.0.9
• PHP Version: 7.3
• LDAP Type: OpenLDAP

Description:

Hi.

We are using OpenLDAP which means we need to explicitly select the entryuuid on users when running an LDAP query. It works fine as the UserResolver automatically inserts it to the queries, but the resulting Adldap\Model model's "query" object loses this information so upon calling $model->fresh(), the entryuuid is missing from the fresh model.

Steps To Reproduce:

$resolver->query()->where('uid', 'grant')->first()

The returned model has the "entryuuid" attribute, but the model's "query" object has only "*" as the selected columns.

Calling fresh() right after the query:

$resolver->query()->where('uid', 'grant')->first()->fresh()

The returned model does not have the "entryuuid" attribute and same as above the "query" object is missing the entryuuid column.

Proposed solution

I traced back the source of this behaviour to Adldap\Query\Builder@newInstance method which creates a new Builder object from the previous one, but does not care at all about the previous selects.

Original code:

    /**
     * Returns a new Query Builder instance.
     *
     * @param string $baseDn
     *
     * @return Builder
     */
    public function newInstance($baseDn = null)
    {
        // We'll set the base DN of the new Builder so
        // developers don't need to do this manually.
        $dn = is_null($baseDn) ? $this->getDn() : $baseDn;

        return (new static($this->connection, $this->grammar, $this->schema))
            ->setDn($dn);
    }

Proposed solution:

    /**
     * Returns a new Query Builder instance.
     *
     * @param string $baseDn
     *
     * @return Builder
     */
    public function newInstance($baseDn = null)
    {
        // We'll set the base DN of the new Builder so
        // developers don't need to do this manually.
        $dn = is_null($baseDn) ? $this->getDn() : $baseDn;

        return (new static($this->connection, $this->grammar, $this->schema))
            ->setDn($dn)->select($this->getSelects());
    }

[stevebauman]

Hi @dobiadi,

I'm not sure if placing this in the newInstance method in the query builder would be the best place -- as it may have unintended consequence when creating two or more queries and expecting them to be separate and not effect each other. For instance:

$query = Adldap::search();

$users = $query->users()->select('samaccountname')->get();

// The 'samaccountname' select will now be carried over to this
// query, when it shouldn't since it's a "brand new" instance:
$computers = $query->newInstance()->computers()->get();

I've actually added this in on the successor package LdapRecord as it has a Model Scopes feature allowing us to add specific requirements to individual LDAP distributions, but Adldap2 does not have this feature.

It may be better to check the schema in use (in this case OpenLDAP) and then add the select there. Thoughts?

[dobiadi]

You are right. I might check out the other package, it seems great.

Maybe adding this extra select on the getSelects() method (similarly how it is done in UserResolver) would solve this without breaking anything? Or adding it somewhere in the search Factory?