General Mock object for simpletests (expected value/real value object model)

I can second that this is happening...I'm using rules to unpublish nodes, and they stay in the search index. I also call entity_save after changing the node title in a custom module, and the update does not propagate to the search index. I have yet to explore the module, but will report back if I come up with anything good...

So after some digging, I've figured out that it is getting past the "is_callable($status_callback)", and properly assigning status to 0 and inserting the record into the table "apachesolr_index_entities_node" with a status equal to 0. But what happens from then on? It seems like there are a bunch of nodes here with status = 0 but that nothing calls the table to remove the records from the index?

I think I fixed this (at least temporarily for my purposes) by adding a check for $status = 1, and sending the entity to be deleted from the index if not...

 if($status == 1){
    $indexer_table = apachesolr_get_indexer_table($type);

     .......
     (more unchanged code here)
     ......

      ->execute();
	}
	else{
	return apachesolr_entity_delete($entity, $type);
	}

Nodes leave the index as they should, and then when re-published, they come back....
Is this the best way to do this?

On it right now, looks critical

Could you put the module_load_include at the top of the function?

/**
 * Implements hook_entity_update().
 */
function apachesolr_entity_update($entity, $type) {
+  // Include the index file for the status callback
+  module_load_include('inc', 'apachesolr', 'apachesolr.index');
  if (apachesolr_entity_should_index($entity, $type)) {

Committed! Thanks!

Ok, Before we continue I'd like to get some simpletests first so we can actually figure out if everything works as expected. Care to do that?

I think that what was committed is right and disagree with #10.

My impression is that apachesolr_index_node_check_table is only there to catch problems that slip through. Right now I have a lot of nodes that it's trying to fix because they are never properly updated during node_save (which the patch in #8 fixes). Nodes that are properly matching between the two tables will already be indexed and don't need to be caught by this check function.

I just updated to dev and will reindex with the patch in #8. I have a big site with a lot of activity so should be able to tell pretty quickly if this still needs work.

Subscribe

Jody Lynn, possible to get back to us with an update?

It should be removed using apachesolr_index_delete_entity_from_index

So, for a test :

- Add a node and make sure it is published
- verify if this node exists in our indexer_table with status 1
- Run indexing (cron or the function)
- Check if the node was submitted
- Unpublish the node
- verify if this node exists in our indexer_table with status 0 and a new changed timestamp
- verify if the node was removed from solr

The only bad thing that happens in the module is that deleted nodes are not scheduled for deletion but are deleted immediately. We could improve this but this would be another issue

Afaik, no such thing is happening. apachesolr_index_get_entities_to_index only indexes entities with status 1 so we should probably add an extra if check in entity_update and when the status from the function call returns 0 it should remove the entity from the index using apachesolr_index_delete_entity_from_index.

This way we have an easy way to control if unpublished nodes should be indexed or not. One could easily return TRUE for the status callback so it indexes unpublished nodes.

Understood?

Seems to be good yes. This matches the use case I explained above I think

Before we can mark this as reviewed and ready I'd love to have some simpletests. Please chip in if you have time :-)

The dev (what was committed in #8) fixed my problem.

My problem was that nodes that started out as unpublished and then were edited and set to published were never added to the index. They would be attempted to be fixed on apachesolr_index_node_check_table but that would also fail to index them for some reason and so it would try the same list of nodes each cron run.

A test would be to create a node that's unpublished and a node that's published. Index and make sure only the published node appears in search. Then edit each one and reverse their statuses and index. The reverse should appear in search.

Sound about right for the tests description?

Sounds about right, now we just need someone to do it ;-)

Let's give the testbot some crunching :-)

I'll check in detail later, thanks!

Wow, I was absent for a little while. What a great attribution!

However, I should look more closely to this patch because it adds a bunch of code (and since the testing bot is green it seems like it is good code)
Do you think your mock object is able to handle all testing interactions towards solr without having a real solr?

johnennew - I commited the included patch for the status into the project (6.x-3.x & 7.x-1.x)but I am going to wait with the mock object a bit till we get some more reviewers. I do like the dynamics it allows.

This patch includes only the mock object

@johnennew - If possible I'd like to see all the live tests (or as many as possible) converted to this mock object. Would you be able to do that? Currently it checks if we have a live solr available and if so, it runs an additional set of tests. This makes the test different on a local platform compared to drupal.org. Preferably we want to get rid of this difference

I already renamed it, no worries - I prefer to keep the history where this came from. You can however change the issue summary already to reflect in more detail what is expected. Also look in the issue queue, there should be an ancient issue that tried to do the same.

Any news on the status of this mock object for testing? We are running a multisite setup, I got the sandbox version working of apachesolr_multisite in D7. I would like to extend the module with some simple tests. Do you think I could subclass this mockup object to allow it to return multisite test data?

kriboogh, be my guest to do that. We are still in the progress for converting most of the test suite to this mock object. This can take a considerable amount of time so any contribution is welcome

Going to categorize this as a feature request. It is certainly not a bug nor a blocker

postponing for now - let's get the module stable and get away from scope creep

The purpose of this issue changed as the conversation progressed. Updated the top level description so it still makes sense.