ba-st / sagan Goto Github PK

In the PR
#60
the support for MySQL was discontinued since it was not being actively maintained, and relied at least partially on old dependencies.
This issue is to remember that should eventually add back support, with up to date dependencies and drivers.

Improve the travis CI setup , run the unitary tests against the RBDMS versions supported by travis CI.

https://downloads.mariadb.org/mariadb/+releases/

If update:with: is performed in a different session from the one used to read the original object, it fails as Glorp tries to do an insert instead of an update.

Problem is already documented in this test:
PooledRDBMSRepositoryProviderTest>>testUpdateWithOutsideTransactWillFail

Workaround for now is to re-read the object before updating it, within the same transaction to ensure same session is used.
For example, update method for a trade registration system:

updateTrade: originalTrade with: updatedTrade

	^trades transact: [:session |
		trades
			withOneMatching: [:trade :builder | builder does: trade equal: originalTrade]
			do: [:trade | trades update: trade with: updatedTrade]
			else: [ObjectNotFound signal: ('<1p> was not found.' expandMacrosWith: originalTrade)]]

RDBMSRepository>>update: aMutableDomainObject executing: aMonadycBlock

	self
		transact: [ :session | 
			| originalObject |

			originalObject := session refresh: aMutableDomainObject.
			session modify: originalObject in: [ aMonadycBlock value: originalObject ]
			].
	^ aMutableDomainObject

As seen above this method currently returns the aMutableDomainObject.
This is fine as long as we use a single session to the database within the same Smalltalk image.
It does not work well when using a session pool because the synchronizeWith: is sent to the original object after doing a refresh (not the same instance).
The method should return the result of the modify, which is the object returned in the method modify:in:

By default, Glorp is using proxies for all attributes in relationship mappings.
See method: RelationshipMapping>>initialize.

This is fine and we would normally leave this enabled and resolve the proxy during postFetch.
However, using a pooled session repository provider this is not possible unless objects are somehow shared between sessions or by other means (not sure how).

I am documenting this with more unit tests and will soon submit the changes. I will also add more information to this issue.

Workaround for now, without disabling a session pool, is use relationship mappings for attributes with use of proxies disabled.

So now it's safe to remove it from the build matrix. It's EOL was on November 8, 2018

It is common for an application to represent certain objects with a null value in a database field.
For example, when you use the null object pattern, it is straightfoward to think of a nullable field in the database table representing the null object with a NULL value and the non-null object with the field value, which could be a string, a number, a timestamp, etc.
One could then expect Sagan to provide a way of building the SQL command to query the database for registers where certain field is NULL or satisfies any other expression (most common use is with OR).
So, it would be nice for the matching criteria builder to add support for this.
And, it would be extra nice if mapping definitions provide a way of creating these kind of mappings in a simple way.

Support Gemstone protocol for transaction and indexes in specific repository classes.

This was briefly discussed between Gabriel and I. Other opinions are welcome.
If we model the storing, purging, updating and finding, it seems the repository could be implemented with a single class (goodbye hierarchy).

Two main points make me think this:

1. Currently the superclass defines subclass responsibilities which do not make sense for all subclasses (e.g. #configureMappingsIn:)
2. It defines common behavior which could be extracted to other objects which are easy to create (as I said, the actions for store, purge, update and access the repository elements)

I think this idea is worth trying and not too difficult to try in this early stage of development.
For example, the store action which is the same for all implementations, would be another object which when evaluated, it would perform the assertion for conflict check (which will be a collaboration with the conflict checking strategy which is already a different object) and then store the object.

It feels we could achieve same behavior with one less hierarchy and creating more reusable objects.
Not sure if transaction management gets in the way for all implementations other than InMemory variant, but I am sure it could be solved with more object composition.

Now you can provide an explanation since errors where explained by default with 'Something is in conflict with ' and that was not very user friendly.

Say one test uses 2 tables (X with a fk to Y).
If you run a second test that uses Y, glorp will try to recreate it (drop it and create it again), but as the constrain of the fk in table X still exists in the db, the drop fails.
One solution is that the tearDown of the tests explicitly drop the tables, so the repositories needs to have a protocol to do it.

See https://github.com/pharo-rdbms/Pharo-SQLite3/releases/tag/v2.2.6

When using more than one repository, during #configureMappingsIn: the first repository setups the session causing the descriptor system to perform a validation (see method: GlorpSession>>system:).
Due to how Sagan's configurable descriptor system works, this causes only the first repository's tables, class descriptors and mappings to be created. The rest only exist as definitions which are never evaluated because of lazy variable initializations.

https://www.postgresql.org/about/news/1976/

Currently the only thing not working in Pharo 6.1 are the sorting features, because the SortFunction hierarchy is different to the one in Pharo 7.

Maybe it makes sense to have a package loadable in Pharo 6.1 with the required extensions. The other tests are already working on Pharo 6.1

Currently, running tests with a specific db it's a bit unconfortable as to deploy a db using the scripts, one needs to remember which environment variables needs to be set.

This can be easily solved by adding a docker-compose that has the same set up as the scripts.

Remove from the build matrix the support for 32 bits versions of Pharo.
It makes the build take twice the time for not too much gain. We can't support old tech forever.

Make sure to use transact: in the same methods as the RDBMS implementation

Expose some parameters so the users can configure the session pool:

• maxIdleObjects (defaulting to 10)
• minIdleObjects (defaulting to 5)
• maxActiveObjects (defaulting to 12)

See

This requires changes in the underlying support:
Failed to connect to server: Client does not support authentication protocol requested by server; consider upgrading MySQL client

Would be great to make us of the Postgres UUID datatatype.