ongr-archive / connectionsbundle Goto Github PK
View Code? Open in Web Editor NEWDEPRECATED, USE API INSTEAD
License: MIT License
DEPRECATED, USE API INSTEAD
License: MIT License
"Panther" name not good and need better name. It confuses with other names used in code like "Storage". So we decided to rename it with "SyncStorage".
Is this still needed? I believe this has been replaced by pipeline model.
There is no way to configure shop id for command ongr:sync:provide magento
while ongr:sync:execute magento.product
requires shop id.
So provide
puts data into ongr_sync_storage
but execute
expects data in ongr_sync_storage_{shop_id}
.
Pipeline should be able to skip items. This can be achieved in mulitple ways:
stopPropagation()
and detecting it.Should we throw a .skip
event in that case?
code in question:
related discussion:
https://github.com/ongr-io/ConnectionsBundle/pull/81/files#r22380117
working set command looks like this:
app/console ongr:sync:provide:parameter last_sync_date --set=1 2000-01-01
--set must equal to 1 otherwise command complains that --set must be used when setting values
https://github.com/ongr-io/ConnectionsBundle/blob/master/Resources/doc/sync_storage.rst
command ongr:sync:execute [<pipeline_name>]
uses these events
ongr.pipeline.sync.execute.<pipeline_name>.source
ongr.pipeline.sync.execute.<pipeline_name>.consume
but example gives these
ongr.pipeline.data_sync.<pipeline_name>.source
ongr.pipeline.data_sync.<pipeline_name>.consume
DataSyncService
name and description is too abstract and it is not clear, where this service is being used. Maybe its SyncStarter
etc?
Because of this line: https://github.com/ongr-io/ConnectionsBundle/blob/master/Command/SyncStorageCreateCommand.php#L57
when creating storage, without any shop_id, storage for shop_id=0 is created, everywhere else, when no shop_id specified, default(shop_id=null) storage is created.
NEw license header should be added 2014-2015 see Content bundle.
Some classes have comments such as
Class LoadClassMetadataListener.
or incorrect definitions
Class to pass relations to compiler.
(not a compiler. There is a collection that keeps track of all relations)
Now used shop ids must be defined in parameters like this
parameters:
shop_ids:
- 0
But if configuration is not provided it default to shop ids and then ongr:sync:provide
console command puts data in not suffixed table but SyncExecuteSourceEventListener
never expect data in tables without suffix.
So the problem is that default configuration (no configuration) is invalid and it will not be clear what is wrong and i suggest either throwing errors if configuration is missing or changing default to something that works.
in Sync/DiffProvider/SyncJobs/JobsCleanupService.php, line 75
If provided by the user, the value of
$this->tableName
may allow an SQL injection attack. Avoid concatenating parameters to SQL query strings, and use parameter binding instead.
*
* @return string
*/
protected function generateCleanupQuery()
{
$query = 'DELETE FROM ' . $this->tableName . ' WHERE';
if (empty($this->shops)) {
return $query . ' `status` = ' . SyncJob::STATUS_DONE;
}
Also, escaping functionality exists here: https://github.com/ongr-io/ConnectionsBundle/blob/63948a849c8befb4ddeeb4981da43438e456021a/Sync/StorageManager/MysqlStorageManager.php#L86-L89. Make sure to deduplicate.
E.g. AbstractInitialSyncModifyEvent
should be AbstractInitialSyncModifyEventListener
.
File services.yml
should be split into functionally related service sets.
A lot of logic in catch block.
Some console commands have this code:
$output->writeln('');
$output->writeln(sprintf('<info>Job finished in %.2f s</info>', microtime(true) - $start));
$output->writeln(sprintf('<info>Memory usage: %.2f MB</info>', memory_get_peak_usage() >> 20));
This should be abstracted away as a CommandBenchmark object that takes $output
as an argument.
There are several places, where non-strict comparison is being made with false
, true
or null
. Change to strict equality check. See this report:
in Sync/DiffProvider/Binlog/BinlogDiffProvider.php, line 243
With booleans, only strict comparison (with
===
operator) should be used to lower bug risks and to improve performances.
/**
* {@inheritdoc}
*/
public function next()
{
if ($this->valid() == true) {
$this->setFromDate($this->current()->getTimestamp());
}
$this->getBinlogDecorator()->next();
}
Posted from SensioLabsInsight
Currently, BinLog parser is using decorator pattern with nested iterators.
Team pointed out, that this is a hacky solution and should be resolved by using more flat composition architecture.
When no sql relations defined in connector, ConnectionsBundle fails, because it expects at least one relation. Should relations be required?
example relation:
https://github.com/ongr-io/MagentoConnectorBundle/blob/master/Resources/config/services.yml#L7
Most of ONGR bundles use:
<logging>
<log type="coverage-text" target="php://stdout" showOnlySummary="true"/>
<!--<log type="coverage-html" target="Tests/app/build/coverage" title="Coverage" charset="UTF-8" yui="true" highlight="true"/>-->
</logging>
for logging settings.
I think this config should be updated accordingly.
Word "execute" is reserved for "execute" process as a whole for executing elements from SyncStorage.
"start" is a good name, because pipeline might have other methods for modifying, consuming and finalizing.
There are two injections possible in class MysqlStorageManager.
getFromDate()
setFromDate($fromDate)
https://github.com/ongr-io/ConnectionsBundle/blob/master/Resources/doc/sync_storage.rst
service @ongr_connections.sync.diff_provider.bin_log_diff_provider
does not exist and other documentation gives @ongr_connections.sync.diff_provider.binlog_diff_provider
Investigate and exterminate.
Namespace Service
is too abstract. Contents should be split into more functional namespaces like Util
and UrlInvalidator
.
https://github.com/ongr-io/ConnectionsBundle/blob/master/Resources/doc/sync_storage.rst
ongr_connections:
sync_storage:
mysql:
connection: default
table_name: ongr_sync_storage
should be
ongr_connections:
sync:
sync_storage:
mysql:
connection: default
table_name: ongr_sync_storage
Main load
method should be split into more smaller methods.
Page should be written, that has image of the whole import process + sync. It should include description of each stage briefly with command examples (maybe use sphinx import functionality to deduplicate code).
Possibly, Magento can be used as an example. Magento page itself will have only usage instructions. This page will be more in depth.
Further discussions are needed.
Add short and clear description, possibly copied from Resources/doc
.
Add warning of alpha stability.
Add link to further documentation as in ESB.
Initiate discussion.
syntax:
$this->logger && $this->logger->...;
change to:
if ($this->logger) {
$this->logger->...;
}
Is ImportItem really an event? It should probably reside together with import service classes.
When Connections bundle is in vendor directory DoctrineAnnotations.php is not i the place ESDoctrineTestCase expects, thus its impossible to use this test case outside of connections bundle.
After splitting service definitions to separate files, duplicate definitions where left, few of duplications found:
https://github.com/ongr-io/ConnectionsBundle/blob/master/Resources/config/binlog.yml#L7
and
https://github.com/ongr-io/ConnectionsBundle/blob/master/Resources/config/services.yml#L99
also
https://github.com/ongr-io/ConnectionsBundle/blob/master/Resources/config/extractor.yml#L6
and
https://github.com/ongr-io/ConnectionsBundle/blob/master/Resources/config/services.yml#L106
Need to check and remove all duplications.
Your project contains files with permissive permissions. In order to avoid opening a security breach, you should restrict execution rights on following files:
- .gitignore
- ONGRConnectionsBundle.php
- Resources/config/services.yml
- composer.json
chmod a-x '.gitignore' \
'ONGRConnectionsBundle.php' \
'Resources/config/services.yml' \
'composer.json'
Posted from SensioLabsInsight
There is Event
folder and Pipeline/Event
. We should either mover everything to Event
or distribute events to their own namespaces.
I would prefer second variant to avoid crowded folders.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.