Skip to content

ARTEMIS-1577 Address-settings policies not working with older clients#1787

Closed
clebertsuconic wants to merge 1 commit into
apache:masterfrom
clebertsuconic:compatibility
Closed

ARTEMIS-1577 Address-settings policies not working with older clients#1787
clebertsuconic wants to merge 1 commit into
apache:masterfrom
clebertsuconic:compatibility

Conversation

@clebertsuconic
Copy link
Copy Markdown
Contributor

This closes #1744

AtomicBoolean startedTX = new AtomicBoolean(false);

final SimpleString address = message.getAddressSimpleString();
final SimpleString address = context.getAddress(message);
Copy link
Copy Markdown
Contributor

@michaelandrepearce michaelandrepearce Jan 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not make the call to context.getAddress just the once, and re-use the result throughout the method call?

eg. this line could be before setPagingStore, and simply use address else where.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just going for the simple refactoring here.. every call to message.getAddressSimplString() to be calling context.getAddress()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your comment could apply to the previous version as well.. I didn't make any big changes here. nor did want to risk anything else.


if (duplicateIDBytes != null) {
cache = getDuplicateIDCache(message.getAddressSimpleString());
cache = getDuplicateIDCache(context.getAddress(message));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as other comment, re making one call to context.getAddress(message) and re-using the result

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelandrepearce caching here goes beyond the scope here.

I would need to change a lot of signatures to pass an address as a parameter.. or cache within each method used during routing.

We can do that.. but it goes beyond the scope of this fix.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries I can do separate pr later

@michaelandrepearce
Copy link
Copy Markdown
Contributor

+1 I’m happy as noted I can pr some bits later

@asfgit asfgit closed this in 9f77514 Jan 18, 2018
@clebertsuconic clebertsuconic deleted the compatibility branch May 15, 2018 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants