Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The sample ResizePicture event is processed twice. #2

Closed
paulvanbladel opened this issue Jun 7, 2019 · 11 comments
Closed

The sample ResizePicture event is processed twice. #2

paulvanbladel opened this issue Jun 7, 2019 · 11 comments

Comments

@paulvanbladel
Copy link

paulvanbladel commented Jun 7, 2019

It can be reproduced (but not clearly tested) with following test (put a break point in the when method of the picture class).
You will notice that the following code is executed twice:

case Events.ClassifiedAdPictureResized e:
                    Size = new PictureSize{Height = e.Height, Width = e.Width};
                    break;

Since the handling is an idempotent operation, there is no harm (that's why the test is not failing), but in case of non-idempotency it would give nasty results.

The test:

 [Fact]
        public void ShowMeTheProblem()
        {
            _classifiedAd.SetTitle(ClassifiedAdTitle.FromString("Test ad"));
            _classifiedAd.UpdateText(ClassifiedAdText.FromString("Please buy my stuff"));
            _classifiedAd.UpdatePrice(
                Price.FromDecimal(100.10m, "EUR", new FakeCurrencyLookup()));
            _classifiedAd.AddPicture(new Uri("http://localhost/storage/123.jpg"), new PictureSize(1200, 620));
            _classifiedAd.ResizePicture(_classifiedAd.Pictures.First().Id, new PictureSize(1209, 629));
            _classifiedAd.RequestToPublish();

            Assert.Equal(ClassifiedAd.ClassifiedAdState.PendingReview,
                _classifiedAd.State);
        }
@paulvanbladel
Copy link
Author

@alexeyzimarev .
I have the feeling that in the Entity class, inside the Apply method then when method should not be called, the 'binding' via the applier is already making sure that the when method is called.
My 2 cents...

 protected void Apply(object @event)
        {
            // When(@event);
            _applier(@event);
        }

@kuechlerm
Copy link

@paulvanbladel I see the same problem, but if you remove the When call, the Entity can not apply an event and reach a given state without the AggregateRoot. Normally this should be the way to go, but maybe it makes sense to use an Entity on its own in unit tests. Therefore, I would suggest:

// Entity.cs
[...]
protected void Apply(object @event)
{
    if (_applier == null)
        When(@event);  // for unit tests
    else
        _applier(@event); // Applier calls Entity.When
}

What do you think?

@paulvanbladel
Copy link
Author

@kuechlerm thanks for the reply. Well... not sure... Maybe the author of the book should formulate his exact intension. If a state change is only possible via the AR, not sure then that there can be a backdoor for unit testing.

@alexeyzimarev
Copy link
Collaborator

You are both right but @paulvanbladel has the point.

This is enough:

protected void Apply(object @event) => _applier(@event);

I don't think that testing entities in isolation make a lot of sense. In the end, entities should not be called otherwise than from the aggregate root. It's the aggregate itself who controls its consistency, not an individual entity.

@paulvanbladel
Copy link
Author

@alexeyzimarev a bit reluctant to ask but... everything fine with the missing chapter :)

@alexeyzimarev
Copy link
Collaborator

@paulvanbladel sorry about the delay. I worked on the code yesterday, introducing some testing patterns and figuring out if I can squeeze the micro-frontends topic to the chapter. It seems like micro-frontends would be a separate thing to publish. I've done the chapter code but it needs more tests. When I confirm it working, I would just need two or three paragraphs of text.

@paulvanbladel
Copy link
Author

@alexeyzimarev no worries/sorry Alex. I'm just very thankful you are adding an additional chapter.

@jorgeolive
Copy link

@alexeyzimarev , just one question. At several points through the book you mention about bounded context intercommunication that would be seen in further chapters. I guess that due to book length you had to shorten it, and seems that that part unfortunately got out of scope.

I guess that a good strategy would be to use a messaging service such as RabbitMQ and take the pub-sub route. I am wondering in that scenario, how would you manage sharing the actual event objects between bounded contexts, assuming we're not taking a shared kernel approach. Would you create a new type of events (eg: Integration Events) or the already seen Domain Events would be reused?

Thanks, I loved the book by the way.

@alexeyzimarev
Copy link
Collaborator

Some integration concerns are covered in Chapter 13. It is not published yet and it will be online only. You should have a link at the end of the book in your copy.

@alexeyzimarev
Copy link
Collaborator

The topic of using different event contracts inside and outside of the context is covered briefly as well. You are on the right track. Remember that persisting new state and publishing events must be done in a transaction. If you use event sourcing, you’re good. Make a reactor that will convert domain events to integration events and publish to the bus. Some people just use EventStore for integration as well. Persistent subscriptions are quite good, handle retries, can scale and you don’t need to worry about the checkpoint.

@gaurav-packt
Copy link
Collaborator

Thanks for sharing your feedback, Alexey.

With this I'm closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants