This module implements real-time commits of Drupal entities with the Apache Solr Search Integration project. Implementing real-time commits to Solr not only allows for more up to date search results, but also opens the possibilities for other displays, reports and analysis previously not considered.

Project page: http://drupal.org/sandbox/StevenWill/1912988
Git: git.drupal.org:sandbox/StevenWill/1912988.git

Comments

manarth’s picture

Status: Active » Reviewed & tested by the community

+1 for granting "Promote to full project" permission to StevenWill.

I came across this project whilst searching for a module for exactly this requirement. It's well-coded, well-documented, meets coding-standards, and even has tests. I'm certainly comfortable putting this into a production environment.

Oh, StevenWill has also added me as a co-maintainer on the project, so technically we could promote it to full-project straight away, but I'd prefer to see him get the credit, and the ability to freely contribute other projects :-)

klausi’s picture

Status: Reviewed & tested by the community » Needs work

Sorry for the delay, but you have not listed any reviews of other project applications in your issue summary as strongly recommended in the application documentation.

manual review:

  1. project page is too short, see https://drupal.org/node/997024 . What does the project actually do? Is that the "index immediately" feature from Search API Solr?
  2. apachesolr_realtime.install: empty file, so remove it.
  3. apachesolr_realtime_commit(): why do you need cURL and cannot use drupal_http_request()?
  4. apachesolr_realtime_reports(): all user facing text must run through t() for translation.
  5. apachesolr_realtime_reports(): this looks vulnerable to XSS exploits. $return->ss_name appears to be user provided text and must be sanitized before printing. The vulnerability is mitigated by the fact that Drupal's default registration validation prevents the creation of usernames that contain cross site scripting attacks. However, a contributed module may bypass that validation or alter the way usernames are loaded in a way that introduces an attack vector. I'm not sure if Solr or something else in between already sanitizes $return->ss_name, and I have not tried to exploit this, so please check that and report back. Please read https://drupal.org/node/28984 again.
manarth’s picture

Status: Needs work » Needs review

Hi Klausi,

Thank you for your comments and feedback. Here are the changes we've made to address them:

1. Rewritten the project page to help make the purpose of the module clearer.
2. Empty file removed.
3. Refactored to use drupal_http_request().
4. Refactored the report page to return a renderable array, with all translatable strings passed through t().
5. The value ss_name is now run through check_plain(). The title of the node is sanitised through using l().

PA robot’s picture

Multiple Applications
It appears that there have been multiple project applications opened under your username:

Project 1: https://drupal.org/node/1992544

Project 2: https://drupal.org/node/1944834

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

I'm a robot and this is an automated message from Project Applications Scraper.

PA robot’s picture

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

phanophite’s picture

Assigned: Unassigned » phanophite

assuming ownership for review

phanophite’s picture

Assigned: phanophite » Unassigned
Status: Needs review » Needs work

I think this project is ready. The security hole described by klausi seems to have been resolved with the use of check_plain(), but I think it could be used in all entries of the table. I'm not sure if those other fields would be considered user-provided.

Other than that the other issues mentioned by klausi have been resolved.

This module seems useful, but I would be leery about overhead if it's going to index every insert, update, or delete.

See details below. I performed the checks described here and have added the results below.

1. Basic application checks
1.1 Project application issue contains repository link & project page link. The repository link contains a colon(:) where a forward slash (/) after git.drupal.org.
1.2 Project is not a duplicate.
1.3 Multiple applications by user StevenWill were found, but the only other issue was closed.

2. Basic repository checks
2.1 Repository contains code.
2.2 Repository contains a version specific branch.
2.3 Project contains the minimum of handwritten code.

3. Security Review
3.1 Code appears to meet Drupal's security standards.

4. Licensing checks
4.1 Repository does not contain a LICENSE.txt file.
4.2 Repository does not contain any third party code.

5. Documentation checks
5.1 Project page gives details regarding the functionality of the module. The user can know what to expect when installing this module.
5.2 Project contains a helpful README.txt file that explains the functionality of the module.
5.3 Code contains a well balanced amount of comments.

6. Coding style and standards
6.1 Ran automated review using ventral.org. Review reported a few minor issues but nothing serious.

7. API and best practices review
7.1 Code makes use of Drupal APIs.

manarth’s picture

Status: Needs work » Reviewed & tested by the community

I think this project is ready.

Setting to RTBC.

Regarding the other entries in the table, the node title is going through implicit sanitisation via the l() function, and the other two fields - entity_id and bundle_name - are safe to use directly, they're not user-generated.

kscheirer’s picture

Status: Reviewed & tested by the community » Fixed

You have a typo in the module file, "immediatly".
I'm not sure what the new report in apachesolr_realtime_reports() is about - can you mention that on the project page?

Thanks for your contribution, StevenWill!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

----
Top Shelf Modules - Crafted, Curated, Contributed.

heacu’s picture

i just discovered this project and am now following with interest.

i wonder: is there any plan to provide a version for Apache Solr Search Integration 6.x-3.x, which is just a back port of Apache Solr Search Integration 7.x-1.x? if not, would you see any inherent obstacles to attempting such a back port?

kscheirer’s picture

This issue is about the application and will be closed shortly.

For questions about the module, you'll have better luck filing an issue in the module's queue!

Status: Fixed » Closed (fixed)

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

avpaderno’s picture

Issue summary: View changes