We have moved to Github. Please open tickets there.

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#3495 closed Defect (worksforme)

Inconsistent attribute handling by ElggEntity

Reported by: step70 Owned by:
Priority: normal Milestone: Needs Review
Component: Core Version: 1.8 Beta
Severity: major Keywords: set, ElggEntity
Cc: brett@…, bart@… Difficulty:

Description

The inconsistency causes that attributes of an object are not stored. The inconsistent behaviour is between set() and save().

When setting the attributes for an object, the function set() is called (class ElggEntity). This function checks whether the attribute exists in the attributes array, defined by initializeAttributes(), which it is. My object is extended from ElggObject and has its own attributes ($this->attributesnlevel? = NULL;). If the attribute exists it will be kept in the attributes array, otherwise in the metadata array.

public function set($name, $value) {

if (array_key_exists($name, $this->attributes)) {

When saving the object the save() function from ElggEntity only saves known attributes from the array. And the metadata from the temp_metadata array. This means that attributes defined in the extended object are not saved.

ElggObject circumvents this problem by using inconsistent names between the initialized attributes and the actual used attributes, but this is very confusing (as a workaround).

Suggested solution would be to store all attributes defined by extended objects in the metadata (which I understand is the basic idea from Elgg). Which basically means removing the array key check from the setter. Adding attributes to objects would then be as simple as defining the attribute with initializeAttributes(). No extra code would be needed.

Downside is that objects, for example ElggGroup, which have attributes stored in a separate entity need custimization, but that is already the case.

My workaround for now is override the setter and make sure the attributes becomes metadata before object->save(). To avoid later migration issues keep the same attribute name in the metadata array.

Change History (6)

comment:1 Changed 2 years ago by step70

  • Cc bart@… added

comment:2 follow-up: Changed 2 years ago by ewinslow

  • Resolution set to worksforme
  • Status changed from new to closed

All custom attributes should be stored as metadata. You should not initialize custom properties in the attributes array. Instead, just do $this->property = $default_value.

comment:3 Changed 2 years ago by cash

The only way to do custom attributes in the attributes array for a subclass of ElggObject would be to create your own table and implement the load/save methods. You would lose the ability to query based on those attributes though the API like you can with metadata (though you could use a custom join clause). Regardless, metadata is the recommended mechanism for adding stuff to an ElggObject.

The use of $object->title and $object->tags should work and feel the same to a developer. It doesn't right now because doing $object->title requires $object->save() to be called. It's one of those things that bites you early on with Elgg and you learn to work around it. I don't know of a simple solution to that. My assumption is that ElggEntity, ElggObject, and so on all have "metadata" like columns for performance reasons.

comment:4 Changed 2 years ago by step70

Thank you for the support. I now implemented an override of the getter and setter. Works for me now and I might change it later.

comment:5 Changed 2 years ago by ewinslow

One way to remove the inconsistency would simply be to require that writing entity fields to the database should always require an explicit "save" call, whether metadata our native.

comment:6 in reply to: ↑ 2 Changed 2 years ago by step70

Replying to ewinslow:

All custom attributes should be stored as metadata. You should not initialize custom properties in the attributes array. Instead, just do $this->property = $default_value.

Hi,

I have tested with the suggested approach and found the following issue. When the parent:save() is called for a new ElggObject it does not return the additional metadata fields, but it does when saving an existing ElggObject.

The metadata is saved by ElggEntity:save() and loaded by MyObject:load(). They seem to get lost by ElggObject:save() upon return to my save().

My workaround for this now is to load() after parent:save().

Grt!

Note: See TracTickets for help on using tickets.