-
@andrzej.wojcik I made a (really short) change, that should fix the problem. From what I gathered, in
tigase.vhosts.VHostItemImpl.VHostItemWrapper#refresh
, during refreshing non-default item, there would be a call todefaults.getExtension(extClass)
which would create new instance of such class, but without any data from the repository, thus it would end-up with instance's defaulttrue
(hence it wasn't possible to disable it).I added a check (next to the existing ones that make sure that both
item
anddefaults
are present) with the same idea - making sure that all extensions of default vhost are also present (i.e. they should have been initialised during loading ofdefault
VHost with correct data from repository instead of class' default).Could you verify that it fits author's intention?
IMHO, there is still a problem with handling 'defaults' of boolean values and lack of clarity what "unchecked" mean - whether disabled or use default. tri-state value would fit here nicely (unfortunately, DataForms doesn't seem to support it).
-
Well, I see what you've tried to do, but I'm not sure that you've got that. Basically, now you check if a set of extensions matches. However, if the extension is missing in default and you as for it (call to
getExtension()
) it will be automatically created. Moreover, with your change, it may just stop refreshing asdefault
may never initialize it's extensions as they were initialized ingetExtension()
method, so now the sets of extensions may not match any more.Right now, I'm not sure if the extension refresh will work correctly.
As for merging defaults, each extension has its own custom logic for that and idea how to handle bool values. In case of brute force locker extension, it was assumed that it should be enabled always if default setting or vhost setting is enabled.
-
Thank you for your comment. I spend more time on this and made a couple of changes (@andrzej.wojcik please review https://github.com/tigase/tigase-server/pull/32).
In a nutshell:
- Items are refreshed only sometimes
- We don't set
defaultVHost
intigase.vhosts.VHostJDBCRepository#getItemInstance
VHostItems
withinWrappedVHostItem
are not wrapped anymore (there is no point)- names were refactored slightly to better reflect their intent.
Especially two first points are important - setting defaultVHost when getting item instance also triggered refreshing of default vhost, but it wasn't yet loaded from the database. On the other hand, actual item should be refreshed only after it's loaded from the database or once default vhost is set (to update it's effective configuration).
-
There is an NPE when default item hasn't been set and client tries to login.
java.lang.NullPointerException at tigase.vhosts.VHostItemImpl$VHostItemWrapper.lambda$getExtension$1(VHostItemImpl.java:1616) at java.base/java.util.concurrent.ConcurrentHashMap.computeIfAbsent(ConcurrentHashMap.java:1705) at tigase.vhosts.VHostItemImpl$VHostItemWrapper.getExtension(VHostItemImpl.java:1613) at tigase.server.xmppclient.ClientTrustManagerFactory.getManager(ClientTrustManagerFactory.java:119) at tigase.server.xmppclient.ClientConnectionManager.processCommand(ClientConnectionManager.java:746) at tigase.server.xmppclient.ClientConnectionManager.processPacket(ClientConnectionManager.java:129) at tigase.server.AbstractMessageReceiver$QueueListener.run(AbstractMessageReceiver.java:1401)
One solution would be:
- Load all elements from database, create
Items
from them; - Search for
default
element and add it to list of items; - Process/add remaining elements.
Other (quick fix):
diff --git a/src/main/java/tigase/vhosts/VHostJDBCRepository.java b/src/main/java/tigase/vhosts/VHostJDBCRepository.java index 9ce8b6de1..4a50933d2 100644 --- a/src/main/java/tigase/vhosts/VHostJDBCRepository.java +++ b/src/main/java/tigase/vhosts/VHostJDBCRepository.java @@ -150,6 +150,11 @@ public class VHostJDBCRepository } else { this.defaultVHost = defaultVHost; } + for (VHostItem it : allItems()) { + if (it instanceof VHostItemImpl.VHostItemWrapper && it != this.defaultVHost) { + ((VHostItemImpl.VHostItemWrapper) it).setDefaultVHost(this.defaultVHost, true); + } + } } @Override
- Load all elements from database, create
-
@wojtek I've pushed different solution to the NPE - basically I've moved creation of default NPE (hardcoded values) to the point where vhosts from database are already loaded. That should work correctly and without any issues as in most cases default VHost will be in the database.
-
@andrzej.wojcik While if got rid of NPE, it broke configuration after startup. I started Tigase, disabled BruteForceLocker for domain X and
default
domain and later on when I tried to enable it it was impossible.I'll crate a test case (most likely with some sort of TestExtension that would be enabled and used only in TTS).
Note: from #helpdeskpr-535 - at the moment of addition and calling
refresh()
does not update the values becausedefaultVHost
hasn't been set yet. -
OK, the issue is finally fixed:
- VHost is properly configured (updated configuring VHost from command) and change is properly propagated to Wrapper
- Added VHostRepository unit-test to test everything properly
- Changed unit-test execution from executing all methods in parallel to executing classes/suites in parallel
- Improved to-string method for VHost to return correct values…
commit:4302d76869913d9ead99e948ad378ec8c5715ca8 (@andrzej.wojcik feel free to take a look)
Type |
Bug
|
Priority |
Normal
|
Assignee | |
Version |
tigase-server-8.1.0
|
Spent time |
0
|
In the following code, even though
BruteForceLockerVHostExtension
is configured as disabled aboveextension.isEnabled();
returns defaulttrue