Closed (duplicate)
Project:
Drupal core
Version:
8.0.x-dev
Component:
rdf.module
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
18 Feb 2013 at 19:08 UTC
Updated:
29 Jul 2014 at 21:55 UTC
Jump to comment: Most recent file
Comments
Comment #1
yched commentedPatch
Comment #3
dawehner#1: rdf-renderable_entities-1920498.patch queued for re-testing.
Comment #5
dawehnerWhat about using a typed exception and catch that one?
Comment #6
yched commentedYeah, getRenderController() and the other getController() methods are not totally ready for what we want to do here.
Raising and catching exceptions in regular request flow (like, here, on every ConfigEntity load...) is something we typically try to avoid.
- We should be able to call getController() and receive NULL/FALSE without having to catch an exception
- Besides, we don't actually need to instanciate the controller here, but merely check that one is defined. So getControllerClass() should have this "NULL / no exception please" ability.
[edit : pinged folks in #1807042: Reorganizie entity storage/list/form/render controller annotation keys about this]
Comment #7
sunExceptions are for actual error conditions only. We should not throw exceptions for perfectly valid runtime conditions/behavior.
Instead of instantiating the controller, why don't you retrieve the entity info and check whether there's a render controller defined?
Comment #8
yched commentedSure, I have a patch that does this - but that is not ideal encapsulation IMO. Dealing with this should be the job of the EntityManager (like, #1807042: Reorganizie entity storage/list/form/render controller annotation keys is about to reshuffle the whole structure of the 'controller classes' within the info).
getControllerClass() *almost* does what we need, we just need a "no exceptions" behavior.
Comment #9
catchWhy is RDF module even implementing hook_entity_load(), this would make more sense in hook_entity_prepare_view() no?
Comment #10
sun+1 to #9. Stellar. :)
Comment #11
yched commentedYup, rdf_entity_load() populates $entity->rdf_mapping, but the data is only actually used in implementations of hook_preprocess_[entity]() or hook_field_attach_view_alter(). So loading it in hook_entity_prepare_view() would make sense.
One exception though : rdf_comment_load() uses the 'rdf_mapping' property set in rdf_entity_load() :
"set it here to optimize performance for websites that implement an entity cache" :-)
Thoughts ?
Comment #12
dawehnerWe maybe have to think whether it actually makes sense to "require" a certain controller, as they should be maybe optional if possible. An exception kind of requires it, and you really don't want to use the plugin definition directly.
What about using the exception and let the client code, check whether (if ($render_controller instanceof EntityRenderControllerInterface))?
Comment #13
yched commentedAlso, even though rdf_entity_load() will be better suited as hook_entity_prepare_view() (and thus will only fire on the right entities), we still need to stop rdf_entity_bundle_info_alter() from loading stuff for non-rendereable entity types.
Comment #14
catchFor caching of the RDFa processing we can do #1605290: Enable entity render caching with cache tag support.
Comment #15
scor commentedWe're refactoring the RDF mappings to be loaded "on demand" at #1869600: Refactor RDF mappings to be inline with the new Entity Field API, so I would not worry about fixing them here. I'd suggest to mark this issue as dup of #1869600: Refactor RDF mappings to be inline with the new Entity Field API.
Comment #16
dawehnerStarted a small patch to get exactly just this information out: #1945680: Provide a method that checks whether a certain entity type provides a certain controller
Comment #17
dcam commentedhttp://drupal.org/node/1427826 contains instructions for updating the issue summary with the summary template.
The summary may need to be updated with information from comments.
Comment #18
scor commentedThis will be solved with #1869600: Refactor RDF mappings to be inline with the new Entity Field API.