Switch to feed owner during manual import.

I'm also having issue like this. It would be nice if there's a solution/patch for this.

I'm working on this issue, but it involves a lot of changes. Work in progress in patch attached. Not functional yet.

This big change introduces the following:

• A feeds executable class, which contains all logic around the order of import tasks and related actions:
  • It takes care of switching accounts during each stage of the import.
  • It creates batch tasks. Child classes then dictate to which API these tasks are sent: either the batch API or the queue API.
  • As said before, it takes care of running import tasks in the right order: begin, fetch, parse, process, clean.
  • It dispatches events.
• A feeds batch class, that holds information on which stage and operations to run. For some stages, like the parse stage, multiple new batch tasks are created. During the parse stage a bunch of process tasks are created and one 'finish' task.
• The class FeedsBatchBatch passes the info to the batch API.
• The class FeedsQueueBatch passes the info to the queue API.
• FeedsImportHandler is refactored to pass the actual import tasks to the executable.
• FeedsRefresh is refactored to also pass the actual import tasks to the executable.

In a follow-up FeedsImportHandler::import() and FeedsImportHandler::pushImport() should also pass their logic to an executable, so that these too follow the same logic as batch imports and cron imports. Right now, no account switch happens in there.

Let's see which tests are failing after this refactoring. I'm also not sure yet if it already fixes the reported issue.

Here is also a test-only patch that should demonstrate the issue and thus fail.

Okay, it looks like FeedImportHandler::import() and FeedImportHandler::pushImport() needs to be updated in this issue as well.

Let's fix some coding standard issues first.

Let's see what this does. I'm not sure about the implementation of pushImport() yet. I'm also not sure if the implementation of FeedsExecutable::handleException() is correct.

Apparently, the parse state can be explicitly set to NULL, which happens in FeedsExecutable::doFetch():

$feed->setState(StateInterface::PARSE, NULL);

This is taken from FeedRefresh::doFetch(). I assume this is because the parse state needs to be reset after a second fetch.

This updates the unit tests FeedRefreshTest and FeedImportHandlerTest. It also adds an unit test for the class FeedsExecutable.

The test FieldValidationTest::testImportFieldWithAdminFilterFormatInUi() which was still failing has been corrected. The message after import is not "Created 2 nodes", but "Created 2 Articles". Because this test was updated, I also made a new test-only patch, to ensure this whole refactoring thing actually fixes an issue.

Points of #9 still need to be addressed.

I've spinned off the issue I found and fixed in #10 in #3086342: $feed->getState() can return NULL while it should always return an instance of StateInterface.

Besides this patch being a reroll because of that, this also fixes coding standard issues and corrects FeedsExecutable::handleException() (I checked how the previously existing FeedImportHandler::handleException() was implemented).

Needs at least kernel test coverage for FeedImportHandler::pushImport(). Perhaps I'll create a spin-off issue for this as well.

Two changes:

• Test coverage for FeedImportHandler::pushImport().
• FeedsQueueExecutable overrides ::handleException() so that it rethrows \RuntimeException instances. I'm not sure though why normally these exceptions should be catched.

Reroll, needed after fixing several deprecations in dev.

Using custom code that depends on these changes, I saw a need to have access to $params (relevant to the current stage) in FeedsExecutable::handleException(), so I'm adding extra context to this method.

Hi,

I used,
drupal 8.6.16
Feeds 8.x-3.x-dev
Feeds_Tamper 8.x-2.x-dev

Before applying the patch, the testing was passed

After applying the patch https://www.drupal.org/files/issues/2019-11-10/feeds-switch-owner-ui-281...

The testing gets success.

Thanks.

@thangaraj.moorthi
Thanks for testing! Unfortunately, running the automated tests locally give no new information. Automated tests are run in isolation, thus with only Feeds enabled. So your test results only match with the test results from the testbot, which also reported success.

The most useful thing to test here is testing doing imports through the UI manually and in combination with several contributed projects that depend or integrate with Feeds, thus with as much projects that are listed on https://www.drupal.org/project/feeds/ecosystem (that have D8 versions). The purpose of this testing is to catch regressions: features provided by these contrib projects that worked with alpha6, but get broken after this patch is applied.

I’m testing this patch too on a live site, combined with a bunch of custom modules that integrate with Feeds. So far, any reports from my client don’t seem to be related to this patch.

Well Ok, Thank you @MegaChriz

Hi,

Before applying the patch, I tested it manually.
I had with more custom modules ie) Feeds_Tamper, Feeds_Personify, Commerce_feeds..
I'm tested on feeds tamper functionality and the import process is gets done successfully.

I used,
drupal 8.7.10
Feeds 8.x-3.x-dev
Feeds_Tamper 8.x-2.x-dev

I have to do some more testing.
will update that soon.

Thanks

While working on #3069752: Entities sometimes get removed/unpublished unexpectedly on cron I noticed some tests did not pass on Drupal 8.8 with this patch installed. Hopefully with the attached patch they do.
Also catched a deprecated call to getMock().

No functional changes were made. Only test fixes.

@MegaChriz

I used drupal 8.8.0-rc1,
feeds 8.x-3.x-dev

I'm not getting any getMock() deprecated errors, but having some other deprecations even after applying the patch.

 ------ -------------------------------------------------------------------------- 
  Line   src/Element/Uri.php                                                       
 ------ -------------------------------------------------------------------------- 
  28     Call to deprecated function file_stream_wrapper_uri_normalize():          
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                     
         \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface::normalizeUri()  
         instead.                                                                  
 ------ -------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------------------------------ 
  Line   src/Entity/Subscription.php                                                                                 
 ------ ------------------------------------------------------------------------------------------------------------ 
  104    Call to deprecated constant REQUEST_TIME: Deprecated in drupal:8.3.0 and is removed from drupal:9.0.0. Use  
         \Drupal::time()->getRequestTime();                                                                          
 ------ ------------------------------------------------------------------------------------------------------------ 

 ------ ------------------------------------------------------------------------------------ 
  Line   src/Plugin/Type/FeedsPluginManager.php                                              
 ------ ------------------------------------------------------------------------------------ 
  61     Instantiation of deprecated class Drupal\feeds\Plugin\Type\FeedsAnnotationFactory:  
         in Feeds 8.x-3.0-alpha6, will be removed before Feeds 8.x-3.0.                      
 ------ ------------------------------------------------------------------------------------ 

 ------ --------------------------------------------------------------------------------- 
  Line   tests/src/Functional/Plugin/Field/FieldFormatter/FeedsItemFormatterTestBase.php  
 ------ --------------------------------------------------------------------------------- 
  95     Call to deprecated function entity_get_display():                                
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                            
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                      
 ------ --------------------------------------------------------------------------------- 

 ------ --------------------------------------------------------------------------------- 
  Line   tests/src/Functional/Plugin/Field/FieldFormatter/FeedsItemGuidFormatterTest.php  
 ------ --------------------------------------------------------------------------------- 
  20     Call to deprecated function entity_get_display():                                
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                            
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                      
  43     Call to deprecated function entity_get_display():                                
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                            
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                      
 ------ --------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------- 
  Line   tests/src/Functional/Plugin/Field/FieldFormatter/FeedsItemImportedFormatterTest.php  
 ------ ------------------------------------------------------------------------------------- 
  20     Call to deprecated function entity_get_display():                                    
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                                
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                          
  38     Call to deprecated function entity_get_display():                                    
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                                
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                          
  68     Call to deprecated function entity_get_display():                                    
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                                
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                          
 ------ ------------------------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------------- 
  Line   tests/src/Functional/Plugin/Field/FieldFormatter/FeedsItemTargetEntityFormatterTest.php  
 ------ ----------------------------------------------------------------------------------------- 
  20     Call to deprecated function entity_get_display():                                        
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                                    
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                              
  50     Call to deprecated function entity_get_display():                                        
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                                    
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                              
 ------ ----------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------- 
  Line   tests/src/Functional/Plugin/Field/FieldFormatter/FeedsItemTargetIdFormatterTest.php  
 ------ ------------------------------------------------------------------------------------- 
  20     Call to deprecated function entity_get_display():                                    
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                                
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                          
  44     Call to deprecated function entity_get_display():                                    
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                                
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                          
 ------ ------------------------------------------------------------------------------------- 

 ------ ---------------------------------------------------------------------------------------- 
  Line   tests/src/Functional/Plugin/Field/FieldFormatter/FeedsItemTargetLabelFormatterTest.php  
 ------ ---------------------------------------------------------------------------------------- 
  20     Call to deprecated function entity_get_display():                                       
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                                   
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                             
  42     Call to deprecated function entity_get_display():                                       
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                                   
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                             
  64     Call to deprecated function entity_get_display():                                       
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                                   
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                             
  92     Call to deprecated function entity_get_display():                                       
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                                   
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                             
 ------ ---------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------- 
  Line   tests/src/Functional/Plugin/Field/FieldFormatter/FeedsItemUrlFormatterTest.php  
 ------ -------------------------------------------------------------------------------- 
  22     Call to deprecated function entity_get_display():                               
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                           
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                     
  37     Call to deprecated function entity_get_display():                               
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                           
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                     
  73     Call to deprecated function entity_get_display():                               
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                           
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                     
  87     Call to deprecated function entity_get_display():                               
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use                           
         EntityDisplayRepositoryInterface::getViewDisplay() instead.                     
 ------ -------------------------------------------------------------------------------- 

 ------ ---------------------------------------------------------------------------------------------- 
  Line   tests/src/Kernel/Entity/FeedTest.php                                                          
 ------ ---------------------------------------------------------------------------------------------- 
  177    Call to deprecated method setExpectedException() of class Drupal\KernelTests\KernelTestBase:  
         in drupal:8.8.0 and is removed from drupal:9.0.0.                                             
         Backward compatibility for PHPUnit 4 will no longer be supported.                             
  294    Call to deprecated method setExpectedException() of class Drupal\KernelTests\KernelTestBase:  
         in drupal:8.8.0 and is removed from drupal:9.0.0.                                             
         Backward compatibility for PHPUnit 4 will no longer be supported.                             
 ------ ---------------------------------------------------------------------------------------------- 

 ------ --------------------------------------------------------------------------------------- 
  Line   tests/src/Kernel/Feeds/Source/BasicFieldSourceTest.php                                 
 ------ --------------------------------------------------------------------------------------- 
  15     Usage of deprecated trait Drupal\Tests\taxonomy\Functional\TaxonomyTestTrait in class  
         Drupal\Tests\feeds\Kernel\Feeds\Source\BasicFieldSourceTest:                           
         in drupal:8.8.0 and is removed from drupal:9.0.0.                                      
         Use \Drupal\Tests\taxonomy\Traits\TaxonomyTestTrait instead.                           
 ------ --------------------------------------------------------------------------------------- 

 ------ ---------------------------------------------------------------- 
  Line   tests/src/Kernel/Feeds/Target/FileTest.php                      
 ------ ---------------------------------------------------------------- 
  67     Call to deprecated function file_default_scheme():              
         in drupal:8.8.0 and is removed from drupal:9.0.0. Use           
         \Drupal::config('system.file')->get('default_scheme') instead.  
 ------ ---------------------------------------------------------------- 

 ------ ---------------------------------------------------------------------------------------------- 
  Line   tests/src/Kernel/Feeds/Target/FileTestBase.php                                                
 ------ ---------------------------------------------------------------------------------------------- 
  179    Call to deprecated method setExpectedException() of class Drupal\KernelTests\KernelTestBase:  
         in drupal:8.8.0 and is removed from drupal:9.0.0.                                             
         Backward compatibility for PHPUnit 4 will no longer be supported.                             
 ------ ---------------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------------- 
  Line   tests/src/Unit/FeedExpireHandlerTest.php                                              
 ------ -------------------------------------------------------------------------------------- 
  97     Call to deprecated method setExpectedException() of class Drupal\Tests\UnitTestCase:  
         in drupal:8.8.0 and is removed from drupal:9.0.0.                                     
         Backward compatibility for PHPUnit 4 will no longer be supported.                     
 ------ -------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------------- 
  Line   tests/src/Unit/Feeds/Target/EntityReferenceTest.php                                   
 ------ -------------------------------------------------------------------------------------- 
  146    Call to deprecated method setExpectedException() of class Drupal\Tests\UnitTestCase:  
         in drupal:8.8.0 and is removed from drupal:9.0.0.                                     
         Backward compatibility for PHPUnit 4 will no longer be supported.                     
  165    Call to deprecated method setExpectedException() of class Drupal\Tests\UnitTestCase:  
         in drupal:8.8.0 and is removed from drupal:9.0.0.                                     
         Backward compatibility for PHPUnit 4 will no longer be supported.                     
 ------ -------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------------- 
  Line   tests/src/Unit/Feeds/Target/FileTest.php                                              
 ------ -------------------------------------------------------------------------------------- 
  111    Call to deprecated method setExpectedException() of class Drupal\Tests\UnitTestCase:  
         in drupal:8.8.0 and is removed from drupal:9.0.0.                                     
         Backward compatibility for PHPUnit 4 will no longer be supported.                     
 ------ -------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------------- 
  Line   tests/src/Unit/Plugin/QueueWorker/FeedRefreshTest.php                                 
 ------ -------------------------------------------------------------------------------------- 
  128    Call to deprecated method setExpectedException() of class Drupal\Tests\UnitTestCase:  
         in drupal:8.8.0 and is removed from drupal:9.0.0.                                     
         Backward compatibility for PHPUnit 4 will no longer be supported.                     
  167    Call to deprecated method setExpectedException() of class Drupal\Tests\UnitTestCase:  
         in drupal:8.8.0 and is removed from drupal:9.0.0.                                     
         Backward compatibility for PHPUnit 4 will no longer be supported.                     
  199    Call to deprecated method setExpectedException() of class Drupal\Tests\UnitTestCase:  
         in drupal:8.8.0 and is removed from drupal:9.0.0.                                     
         Backward compatibility for PHPUnit 4 will no longer be supported.                     
 ------ -------------------------------------------------------------------------------------- 

Let me know if you need any clarification.

@Thangaraj Moorthi
It's normal that deprecation warnings for Drupal 8.8 are there. Feeds will support Drupal 8.7 for at least a few months, so D8.8 deprecations cannot be fixed right now. All warnings look unrelated to the patch, even the one that can be fixed right now (about deprecated constant REQUEST_TIME). Deprecation fixes can be handled in: #3042774: Drupal 9 Deprecated Code Report.

Ok Thank you @MegaChriz

Thanks for your work on this guys.

Looks like @Thangaraj Moorthi was able to test this one out and it worked for him outside of the Drupal 8.8 deprecations which we aren't worrying about now.

Is this sufficiently reviewed or is there more testing and review needed?

@jamesdixon
I asked @damondt if he would check if his contributed Feeds modules still work and he hopes to try it tomorrow. From Slack, today:

OK, @megachriz I got time allocated tomorrow to test "Switch to feed owner during manual import." with some feeds extension modules

I think especially his contributed module Feeds Halt Imports could be influenced by this patch (based on the name of the module, I haven't tried the module myself yet). On the other hand, when briefly expecting its code I don't see right away something that would potentially break.

Cool, lets see how @damondt's tests go.

I have this patch running on several live sites for about two months now and did not encounter any issues that are related to this patch. I also walked through all the code one more time to see if I perhaps could find something that could have impact. I only found some spelling errors or small mistakes in a code comments.

Committed #20 with a few changes in interface texts and code comments.