Porting a complex extension to Magento 2

.topleft.small.grey[Meet Magento Romania]
.topright.small.grey[29.10.2016]
# Porting a complex extension to Magento 2

## .integergreen[Fabian Schmengler]

.small.twitter.grey[@fschmengler]

---

# The Extension
## .huge.integergreen[IntegerNet_Solr]

---
class: middle
# Improved Search and Layered Navigation

- More relevant search results
- Fuzzy search
- Autosuggest window
- Filters with multiple selection
- Search categories and CMS pages
- Boosting of products and attributes
- Fetch category pages from Solr for improved performance

---

---
class: middle
# Metrics

- 2304 Logical Lines of Code
- 79 Classes (and 3 interfaces)
- 10 Class rewrites
- 12 Observers

---

# Our Goal:
## .huge[Reuse]

---

background-image: url(images/architecture1a.png)
class: center
# Extract independent library

---

# Extract independent library

---

# Extract independent library

---

# Architecture

---

# Architecture

---

# Architecture

---

# Architecture

---

# The Plan:

1. Decouple / Extract library
2. Implement M2 bridges
3. Glue together M2 module

---

---
class: center

# .huge[First: TESTS]
--

![Grumpy Programmer](images/grumpy.png)
## .red[this is **NOT** optional!]

- Make changes with confidence
- Receive fast feedback during development

---
# .green[Must have: Integration Tests]
- e.g. EcomDev_PHPUnit

# .orange[Nice to have: Functional Tests]
- e.g. Selenium

# .red[Useless: Unit Tests]
- Don't write unit tests for existing code!

---
class: middle

# Start easy

- Find classes with least interaction with Magento
- Eliminate dependencies

# Small steps
- Introduce intermediate solutions if necessary
- Deprecate them immediately

---
class: center, middle
background-image: url(images/mage.png)

---
class: middle
# Mage Levels

- `Mage::throwException()`
- `Mage::getStoreConfig()`
- `Mage::dispatchEvent()`
- `Mage::helper()`
- `Mage::getModel()`

---
class: middle
# Use own exceptions
```php
Mage::throwException($msg);
```

## .center[⇓]
```php
throw new IntegerNet_Solr_Exception($msg);
```

---
class: middle
# Inject configuration values
```php
public function __construct()
{
    $this->host = Mage::getStoreConfig('integernet_solr/server/host');
    $this->port = Mage::getStoreConfig('integernet_solr/server/port');
}
```
## .center[⇓]
```php
public function __construct($host, $port)
{
    $this->host = $host;
    $this->port = $port;
}
```

---
class: middle

# Better: Use Value Objects
- represent a value
- no identity
- immutable

---
# Configuration Value Objects
## Definition
```php
namespace IntegerNet\Solr\Config;
final class ServerConfig
{
    private $host, $port; // ...
    public function __construct($host, $port, ...)
    {
        $this->host = $host;
        $this->port = $port;
        // ...
    }
    public function getHost()
    {
        return $this->host;
    }
    public function getPort()
    {
        return $this->port;
    }
    // ...
}
```

---
# Configuration Value Objects
## Usage

- Read configuration in Magento module:
```php
    $serverConfig = new ServerConfig(
        Mage::getStoreConfig('integernet_solr/server/host'),
        Mage::getStoreConfig('integernet_solr/server/port'),
        // ...
    );
```
- Pass `$serverConfig` value object to library:
```php
$solr = new SolrResource($serverConfig, ...);
```

---
class: middle
# Interfaces for Helpers

```php
public function __construct()
{
    $this->helper = Mage::helper('integernet/solr');
}
```
```php
$this->helper->getUserQueryText();
```
## .center[⇓]
```php
public function __construct(UserQuery $userQuery)
{
    $this->userQuery = $userQuery;
}
```
```php
$this->userQuery->getText();
```

---
class: middle
# Introduce Interfaces
First define interface for existing code

```php
class IntegerNet_Solr_Helper_Data extends Mage_Core_Helper_Abstract
*    implements UserQuery
{
    public function getUserQueryText();
    ...
}
```
```php
namespace IntegerNet\Solr\Implementor;
*interface UserQuery
{
    /**
     * Returns query as entered by user
     *
     * @return string
     */
    public function getUserQueryText();
}
```

---
class: middle
# Dependency Injection
Then pass helpers as implementation of the interface
```php
class IntegerNet_Solr_Helper_Factory
{
    public function getSolrRequest()
    {
        return new SearchRequestFactory(
*           Mage::helper('integernet_solr'),  // constructor expects UserQuery
            ...
        )->createRequest();
    }
}
```
**Note:** No Dependency Injection framework involved!

---
class: middle
# Interface Segregation
```php
class IntegerNet_Solr_Helper_Data extends Mage_Core_Helper_Abstract
*    implements UserQuery, EventDispatcher, SearchUrl
{
    ...
}
```

- Lean interfaces
- Better defined dependencies

## Single Responsibility
- Split helpers (later)

---
class:middle
# Build bridges for models

- "Implementor" interfaces define what library needs from Magento
- Bridge implementation delegates to actual Magento models

---
background-image: url(images/architecture2c.png)

## Remember?

---
class: middle, center
# .huge[Example]

---
### Service
```php
class ProductModification
{
    public function uppercaseDescription($sku) {
        $product = $this->productRepository->getProduct($sku);
        $product->setDescription(strtoupper($product->getDescription());
        $productRepository->saveProduct($product);
    }
}
```
### Interfaces
```php
interface ProductRepository {
    /** @return Product */
    public function getProduct($sku);
    public function saveProduct(Product $product);
}
interface Product {
    public function getDescription();
    public function setDescription($description);
}
```
---
### Bridge: Product
```php
class IntegerNet_Example_Model_Bridge_Product implements Product
{
    /** @var Mage_Catalog_Model_Product */
    protected $_product;
    
    public function __construct(Mage_Catalog_Model_Product $_product)
    {
        $this->_product = $_product;
    }
	
    public function getDescription()
    {
        return $this->_product->getDescription();
    }
	
    public function setDescription($description)
    {
        $this->_product->setDescription($description);
    }
    
    ...
    
}
```
---
### Bridge: Product (additional methods)
```php
class IntegerNet_Example_Model_Bridge_Product implements Product
{
    ...
    
*   /** @deprecated only use interface methods! */
    public function __call($method, $args)
    {
        return call_user_func_array(array($this->_product, $method), $args);
    }
	
    /** @return Mage_Catalog_Model_Product */
    public function getMagentoProduct()
    {
        return $this->_product;
    }
}
```

- only use within the Magento module!
- `__call()` useful during refactoring (`@deprecated`)

---

### Bridge: Product Repository
```php
class IntegerNet_Example_Model_Bridge_ProductRepository
    implements ProductRepository
{
    public function getProduct($sku)
    {
	    $id = Mage::getModel('catalog/product')->getIdBySku($sku);
	    $magentoProduct = Mage::getModel('catalog/product')->load($id);
*       return new IntegerNet_Example_Model_Bridge_Product($magentoProduct);
    }
	
	public function saveProduct(Product $product)
	{
*	    /** @var IntegerNet_Example_Model_Bridge_Product $product */
*		$product->getMagentoProduct()->save();
	}
}
```

- Here we know the concrete type of `$product`
- So we are allowed to use `getMagentoProduct()`

---
### Usage from Magento Controller
```php
class IntegerNet_Example_Adminhtml_ModifierController
    extends Mage_Adminhtml_Controller_Action
{
    public function uppercaseDescriptionAction()
	{
	    // Instantiation:
	    $modifier = new ProductModifier(
		    new IntegerNet_Example_Model_Bridge_ProductRepository()
		);
		
		// Call the Service:
		$modifier->uppercaseDescription($this->getParam('sku'));
		
		...
		
	}
}
```

- Tipp: move all intantiation into factory "helper"
- Less duplication
- Magento rewrite system can still be used

---
class: center, middle
# .huge[Divide and Conquer]

---
class: middle

# Split big classes

## Approach 1
Extract methods that don’t use any mutable 
attributes of `$this` to other classes

## Approach 2
Extract mutable state, grouped with the 
methods operating on this state

---

# Example

Big "Result" singleton

![before](images/extract-before.png)

---

# Example

Big "Result" singleton after first extraction

![after](images/extract-after.png)

---
class: middle

# Split big classes

- Keep as Facade (`@deprecated`)
- Delegate calls to new structure

##"Result" example:
- **Before:** 10 public methods, 221 LLOC
- **After:** 6 public methods, 31 LLOC

---
class: middle
# Rebuild Components

- Small independent units
- Plan, develop bottom up, replace old implementation
- New code => TDD, Unit Tests .green[**(Yay!)**]

---

# Example

- Query strings were escaped with a helper
- Used in several places in original code

## Introduced value object
```php
final class SearchString
{
    public function __construct($rawString) { ... }
    public function getRawString() { ... }
    public function getEscapedString() { ... }
    public static function escape ($string) { ... }
}
```

- Replaced strings that represent query string with SearchString instance

---
class: middle
background-image: url(images/notes.jpg)

.layer[
# Visualize class dependencies
- Use doxygen or pen and paper
- Who is talking to whom?
- Where are the boundaries?
- Where do we want them to be?
- Can we minimize the interfaces?
]

---
class: middle

# Avoid .orange["gold-plating"]
- No need for perfect implementation
- Focus on good abstraction instead
- Small and well-defined interface is priority

# When it's good enough, .red[stop.]

---
class: center, middle
#.huge[Before/After]
---

# Bigger size

Logical Lines of Code

Classes

---

# Less complex units

Ø Cyclomatic complexity / method

.big[2.88 → 2.09]  .green[-0.79]
 
Ø Cyclomatic complexity / class

---
# Reactions of a Magento 1 developer

- wow, that's complicated
- unfamiliar code style, not Magento standard
- further development seems to take more effort

# After working with it for a few days
- got used to it faster than expected
- way better IDE integration
- more reliable thanks to automated tests
- higher code quality
---
class: middle, center, cover
# Step 2:
# .huge[M2 Bridge]

---
class: middle
# Code Convertion Tools

- Useful to get started with module XML files
- .red[Don't use them for actual code]

Unirgy ConvertM1M2:
- https://github.com/unirgy/convertm1m2

Magento Code Migration (official):
- https://github.com/magento/code-migration

---
class: middle
# Implement Interfaces
- Completely test driven
- Do not test (and use) services from the library yet
- **Unit tests** where you know which levers to pull in the core
- **Integration tests** (exploratory) if you still have to figure it out

---
class: middle, center, cover
# Step 3:
# .huge[Integrate]

---
class: middle
# Integrate library and Magento 2

- take remaining M1 module as example
- drive development with integration tests
- implement feature by feature
- prefer plugins over events and rewrites

---
class: middle
# Use the latest Magento version

- you don't want to support Magento 2.0 with a new extension
- lock module dependencies in composer.json
```js
"magento/catalog": "^101.0.0"
```

---
class: middle
#Magento 2 Module Development

- follow recommended Magento 2 development practices **.red[if possible]**
 - core code should not be taken as an example
 - refer to devdocs, Stack Exchange (there is a "`best-practice`" tag), presentations

- **.green[be pragmatic]**
 - service contracts are still incomplete
 - models and collections are more flexible

---
class: center
background-image: url(images/hey-did-you-just-use-the-object-manager.png)
# But never use the<br>.big.red[Object Manager!]
---
class: middle
#Dependency Injection - Interfaces
 - always depend on the interfaces
 - defin bridge as preference for implementor
---
class:middle
#Dependency Injection - Services

- `di.xml` also for library classes 
- types and preferences
- factories

---
class:middle
#Dependency Injection - Virtual Types

- prefer type specific dependencies over global preferences
 - virtual types are great

---
## Example: Product collection with disabled flat index
```xml
<type name="IntegerNet\Solr\Model\Indexer\ProductCollectionFactory">
  <arguments>
*   <argument name="collectionFactory" xsi:type="object">disabledFlatStateProductCollectionFactory
    </argument>
  </arguments>
</type>
<virtualType name="disabledFlatStateProductCollectionFactory"
    type="Magento\Catalog\Model\ResourceModel\Product\CollectionFactory">
  <arguments>
    <argument name="instanceName" xsi:type="string">disabledFlatStateProductCollection</argument>
  </arguments>
</virtualType>
<virtualType name="disabledFlatStateProductCollection"
    type="Magento\Catalog\Model\ResourceModel\Product\Collection">
  <arguments>
    <argument name="catalogProductFlatState" xsi:type="object">disabledFlatState</argument>
  </arguments>
</virtualType>
*<virtualType name="disabledFlatState" type="Magento\Catalog\Model\Indexer\Product\Flat\State">
* <arguments>
*   <argument name="isAvailable" xsi:type="boolean">false</argument>
* </arguments>
*</virtualType>
```
---
class: middle, center

#.huge[Questions]
##.huge[FAQ]

---
class: middle

# How long did this take?

- Original M1 module: ~300 hours
- Refactoring: ~150 hours
- M2 module: ~300 hours

---
class: middle

# .icon[![(trollface)](images/trollface.svg)] Can you replace Solr with ElasticSearch

- If you take the framework agnostic approach to the next level: .green[**yes**]
 - create the same boundary between library and search engine than between library and shop framework
 - write different adapters

--
- But why should we?
 - Solr is more advanced, ElasticSearch is easier to learn
 - We already learned Solr
 - .red[**YAGNI**]

---

# More useful resources

- **Eating ElePHPants** keynote (Larry Garfield on the Drupal 8 refactoring)

https://www.youtube.com/watch?v=5jqY4NNnc3I

- **SOLID MVC** presentation (Stefan Priebsch on framework agnostic domain code)
  
  https://www.youtube.com/watch?v=NdBMQsp_CpE

- **Mage2Katas** video tutorials (Vinai Kopp on TDD with Magento 2)

http://mage2katas.com/

---
background-image: url(images/fs-large.jpg)

I'm here, talk to me :-)
]
        
.popup-more[  
Read more in our Blog

[integer-net.com/m1m2](https://www.integer-net.com/magento-1-magento-2-shared-code-extensions/)

Shared Code for Extensions (7 articles)
]