Problem/Motivation

The changes made to Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Json for #2640516: Support paging through multiple requests included backwards-incompatible API changes (protected method signature changes).

We have a custom module that includes a data_parser plugin that extends the Json plugin. After upgrading migrate_plus to 6.0.3 we saw errors when scanning our custom module code with PHPStan:

------ --------------------------------------------------------------------------------------------------------------------------- 
 Line   modules/custom/custom_module/src/Plugin/migrate_plus/data_parser/CustomJson.php  
------ --------------------------------------------------------------------------------------------------------------------------- 
  20     Method                                                                                                                     
         Drupal\custom_module\Plugin\migrate_plus\data_parser\CustomJson::getSourceData()         
         overrides method                                                                                                           
         Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Json::getSourceData()                                                  
         but misses parameter #2 $item_selector.                                                                                    
  21     Method                                                                                                                     
         Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Json::getSourceData()                                                  
         invoked with 1 parameter, 2 required.                                                                                      
 ------ --------------------------------------------------------------------------------------------------------------------------- 


 [ERROR] Found 2 errors

Steps to reproduce

Proposed resolution

Set a default value for the new parameter to preserve backwards compatibility.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork migrate_plus-3462520

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

joegraduate created an issue. See original summary.

joegraduate’s picture

joegraduate
Primary language English
Location Arizona, USA
commented
Issue summary: View changes
joegraduate’s picture

joegraduate
Primary language English
Location Arizona, USA
commented
Assigned: Unassigned » joegraduate
Related issues: +#2640516: Support paging through multiple requests
joegraduate’s picture

joegraduate
Primary language English
Location Arizona, USA
commented
Title: 6.0.3 included a backwards-incompatible API change » Fix backwards-incompatible Json data_parser API change included in 6.0.3.

joegraduate opened merge request !99

joegraduate’s picture

joegraduate
Primary language English
Location Arizona, USA
commented
Assigned: joegraduate » Unassigned
Status: Active » Needs review
heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Needs review » Needs work

NW for some minor feedback.

joegraduate’s picture

joegraduate
Primary language English
Location Arizona, USA
commented
Status: Needs work » Needs review

Applied suggestions from @lucashedding and a couple of other changes. I think this is ready for review again.

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Needs review » Reviewed & tested by the community

Nice enhancements!

  • heddn committed b799318c on 6.0.x authored by joegraduate 
    Issue #3462520 by joegraduate, heddn: Fix backwards-incompatible Json...
heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Reviewed & tested by the community » Fixed
huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented

@joegraduate, @heddn, Thank you! 🙏

jsacksick’s picture

jsacksick commented

FYI, that change broke all my migrations... After I reverted to 6.0.2, all my migrations ran successfully again...
Not really sure what information I can provide, somehow it seems no rows was detected by the migration.

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented

https://www.drupal.org/project/migrate_plus/releases/6.0.4 does include the BC fix now too.

jsacksick’s picture

jsacksick commented

After upgrading to 6.0.4, the rows count is 0 for all my migrations, reverting to 6.0.2 fixes the issue...

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented

@jsacksick, Could you please check whether the 3465782-using-slash-json-parser branch (MR 102) in #3465782: Using "/" item_selector in Json parser plugin config does not work as expected anymore fixes the issue you have?

joegl’s picture

joegl commented

6.0.4 does not fix this issue. Our custom plugin which extends Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Json::getSourceData is still broken:

PHP Fatal error: Declaration of Drupal\CustomJson\Plugin\migrate_plus\data_parser\CustomJson::getSourceData(string $url): array must be compatible with Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Json::getSourceData(string $url, string|int $item_selector = '')

It looks like the BC fix only introduces a default value if one is not specified. This would fix an issue with using the getSourceData method in code, but does not fix issues when the Json class is extended to a custom class which implements the getSourceData method. I don't think there is necessarily a backwards-compatible fix extending the class. Typically a change to a class definition would constitute at least a minor version increase to indicate the change is not backwards-compatible, (i.e., this release should have been 6.1.0, not 6.0.3).

Status: Fixed » Closed (fixed)

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