This patch is for the Drupal.org D7 upgrade. I can of course implement the mapping in a custom module, but to me it makes sense to offer a mapping as part of the machine_name module.

For sites that don't use Solr, the hook does nothing.

CommentFileSizeAuthor
#8 1822252.patch658 bytesgrendzy
#1 1822252.patch789 bytesgrendzy
#1 1822252.png25.06 KBgrendzy

Comments

grendzy’s picture

Status: Active » Needs review
StatusFileSize
new25.06 KB
new789 bytes

/node/N/devel/apachesolr after applying the patch:

senpai’s picture

Priority: Normal » Critical

Elevating to critical because we need this for D7 launch.

dww’s picture

I don't know enough about the innards of the Solr API to give this a meaningful review, but Senpai is asking for one, anyway. ;)

I guess this looks reasonable. I was confused as to how this hook knows what field names to operate on, but grendzy assured me it's operating on field types, not field names.

I can't RTBC this with a clear conscience not knowing the innards of Solr, but I don't see any obvious reason to set this to 'needs work', either...

Cheers,
-Derek

jhedstrom’s picture

This looks reasonable to me. I haven't tested it though.

c-logemann’s picture

It's difficult for me to test this patch. I need a RTBC or better an explicit OK of the "Drupal.org D7 upgrade initiative" for the special stable release:
#1647904: Machine Name needs a 1.0 stable release as part of the Drupal.org D7 Upgrade Initiative

senpai’s picture

Title: Support for apachesolr » Allow machine_name to provide field indexing to ApacheSolr
csevb10’s picture

Ok, in general, this is basically fine.

I don't think all the array elements are technically necessary or accurate, but they don't really cause an issue:

necessary/fine/agree:
- 'indexing_callback' => 'apachesolr_fields_default_indexing_callback',
- 'index_type' => 'string',
- 'facets' => FALSE,
- 'dependency plugins' => array('bundle', 'role'),
- 'multiple' => FALSE,

not sure these are necessary:
- 'map callback' => '', // default setting, except using FALSE
- 'facet mincount allowed' => FALSE, // default setting
- 'hierarchy callback' => FALSE, // default setting
- 'name_callback' => '', // not actually a key 'name callback' is, but don't need it here

grendzy’s picture

StatusFileSize
new658 bytes

Thanks for the feedback, here's a re-roll.

grendzy’s picture

If anyone's wondering specifically how this will be used on Drupal.org, the project node's short name was previously stored under ss_project_uri, and pulled by securitydrupalorg_sync_projects(). The new machine_name field replaces this of course, so with the patch Solr will map fields of this type to ss_[FIELD-NAME].

Although the patch was motivated by the d.o upgrade, it's potentially useful to anyone who uses Solr.

senpai’s picture

Status: Needs review » Reviewed & tested by the community

@grendzy, thanks for the re-roll. This one looks RTBC.

c-logemann’s picture

Status: Reviewed & tested by the community » Fixed

Automatically closed -- issue fixed for 2 weeks with no activity.