theseion / Fuel

Fuel, the Smalltalk object serializer
https://theseion.github.io/Fuel
MIT License
27 stars 13 forks source link

Serializing special objects like nil, true, false together with classes when they have to be substituted #217

Closed dionisiydk closed 8 years ago

dionisiydk commented 8 years ago

Serializing special objects like nil, true, false together with classes not working when classes needs substitution. Following code is broken:

buffer := ByteArray streamContents: [ :s |
serializer := FLSerializer newDefault. serializer analyzer when: [ :o | o == UndefinedObject] substituteBy: [ :o | 100 ]. serializer serialize: {nil. UndefinedObject} on: s].

object := (FLMaterializer newDefault materializeFrom: buffer readStream) root.

And for booleans:

buffer := ByteArray streamContents: [ :s |
serializer := FLSerializer newDefault. serializer analyzer when: [ :o | o == True] substituteBy: [ :o | 100 ]. serializer serialize: {true. True} on: s].

object := (FLMaterializer newDefault materializeFrom: buffer readStream) root.

dionisiydk commented 8 years ago

Same for Character:

buffer := ByteArray streamContents: [ :s |

serializer := FLSerializer newDefault. serializer analyzer when: [ :o | o == Character] substituteBy: [ :o | 100 ]. serializer serialize: {$e. Character} on: s].

object := (FLMaterializer newDefault materializeFrom: buffer readStream) root.

marianopeck commented 8 years ago

Yes, this limit was kind of a design decision. Maybe we didn't expect someone wanting to substitute nil, true, false, characters, etc.

dionisiydk commented 8 years ago

But this case not about true, false and nil substitution but about classes substitution. Is it same?

tinchodias commented 8 years ago

I found the problem and fixed it. Now, there is so much time that I don't contribute with code to Fuel that I'm sure what's the best repository to commit! This is the fix:

Use encodeReferenceToClusterObjectClass: instead of encodeReferenceTo:, in the same way as FOObjectCluster>>clusterSerializeStepWith: does:

FLHookPrimitiveCluster>> clusterSerializeStepWith: aSerialization

super clusterSerializeStepWith: aSerialization.
aSerialization encoder encodeReferenceToClusterObjectClass: theClass.   
tinchodias commented 8 years ago

I was not sure if Fuel supported this, but I saw that #testPrivateExcludedAndWithConflicts reproduces more or less Denis' case but with a regular class instead of a "primivite class". BTW, we should crate the test now.

tinchodias commented 8 years ago

Very related with issue #209

tinchodias commented 8 years ago

Done. Released version 2.1.6 in sthub/Pharo/Fuel, with fix and tests. Now it needs to be integrated into pharo 6 (or 5 too?)

dionisiydk commented 8 years ago

Thank's.

2016-06-13 21:26 GMT+02:00 Martín Dias notifications@github.com:

Done. Released version 2.1.6 in sthub/Pharo/Fuel, with fix and tests. Now it needs to be integrated into pharo 6 (or 5 too?)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/theseion/Fuel/issues/217#issuecomment-225682348, or mute the thread https://github.com/notifications/unsubscribe/AHxaoJqeDDlM1BAETq7g8rlz_rorlonDks5qLa7hgaJpZM4I0JWJ .

tinchodias commented 8 years ago

i guess we can close this issue. Did you integrate the new version, Denis?

dionisiydk commented 8 years ago

Not yet. I was at vacation

2016-06-20 18:13 GMT+02:00 Martín Dias notifications@github.com:

i guess we can close this issue. Did you integrate the new version, Denis?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/theseion/Fuel/issues/217#issuecomment-227190261, or mute the thread https://github.com/notifications/unsubscribe/AHxaoPJIBbYxTcp_-irqubo4oQ3gA2A6ks5qNrwSgaJpZM4I0JWJ .

dionisiydk commented 8 years ago

I open issue 18545 https://pharo.fogbugz.com/f/cases/18545/Update-Fuel

2016-06-20 18:15 GMT+02:00 Denis Kudriashov dionisiydk@gmail.com:

Not yet. I was at vacation

2016-06-20 18:13 GMT+02:00 Martín Dias notifications@github.com:

i guess we can close this issue. Did you integrate the new version, Denis?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/theseion/Fuel/issues/217#issuecomment-227190261, or mute the thread https://github.com/notifications/unsubscribe/AHxaoPJIBbYxTcp_-irqubo4oQ3gA2A6ks5qNrwSgaJpZM4I0JWJ .

dionisiydk commented 8 years ago

In 60100

2016-06-20 18:19 GMT+02:00 Denis Kudriashov dionisiydk@gmail.com:

I open issue 18545 https://pharo.fogbugz.com/f/cases/18545/Update-Fuel

2016-06-20 18:15 GMT+02:00 Denis Kudriashov dionisiydk@gmail.com:

Not yet. I was at vacation

2016-06-20 18:13 GMT+02:00 Martín Dias notifications@github.com:

i guess we can close this issue. Did you integrate the new version, Denis?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/theseion/Fuel/issues/217#issuecomment-227190261, or mute the thread https://github.com/notifications/unsubscribe/AHxaoPJIBbYxTcp_-irqubo4oQ3gA2A6ks5qNrwSgaJpZM4I0JWJ .