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

Fix collections property name conflict in Collections class #79

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tharropoulos
Copy link

Change Summary

What is this?

This change addresses a critical naming conflict in the Typesense PHP library where having a collection named "collections" causes an unhandled exception. The issue occurs because the Collections class has a property named collections that conflicts with the magic getter method (__get), causing it to return an empty array instead of the expected Collection object when accessing a collection named "collections".

This fix is particularly important for users who might use "collections" as a legitimate collection name (e.g., in e-commerce systems), preventing unexpected behavior and improving the robustness of the library.

Changes

Code Changes:

  1. In src/Collections.php:
    • Renamed the private property $collections to $typesenseCollections to avoid naming conflicts
    • Updated all internal references to use the new property name:
      • In __get() method
      • In offsetExists() method
      • In offsetGet() method
      • In offsetSet() method
      • In offsetUnset() method
    • Behavior remains unchanged for all collection names, including "collections"

Context

  • Issue reported in Laravel Scout repository (#896) by @Thiritin where using "collections" as a collection name causes an unhandled exception
  • The bug occurs in Laravel Scout's TypesenseEngine when calling $this->typesense->getCollections()->{$collectionName}
  • Previous implementation would return an empty array instead of a Collection object when $collectionName is "collections"
  • This change ensures consistent behavior regardless of collection names

The fix maintains backward compatibility while resolving the naming conflict, allowing developers to use "collections" as a valid collection name in their applications.

PR Checklist

* rename array variable to avoid reserving the `collections` name for
collections
* the get magic method checks if the name passed is set, and returns
that
* if the name matches the term `collections`, the array would be
returned instead
* rename array variable to avoid reserving the `aliases` name for
aliases
* the get magic method checks if the name passed is set, and returns
that
* if the name matches the term `aliases`, the array would be
returned instead
@tharropoulos
Copy link
Author

Added more commits to tackle the same issues in other parts of the codebase, where the name of a feature may collide with the user's name of said instance of a feature.

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

Successfully merging this pull request may close these issues.

1 participant