Thank you Gus and David for the very prompt replies! I was planning on updating the test with some assertions (that is how I ran into this). It seems that today setting to null will be a no-op, the property will not be changed. I'm not 100% sure what component handles this, it may be Zk.
In my PR I have changed the 'set null value' behavior to be consistent with the 'set empty value' and updated all tests with assertions. If anyone disagrees please let me know here or on the PR itself [0]. best, alex [0] https://github.com/apache/solr/pull/1459 On Wed, Mar 15, 2023 at 3:01 PM Gus Heck <gus.h...@gmail.com> wrote: > Yeah it does look like that test should be improved. I would expect that > null and "" should be treated equivalently, but I don't recall what the > code actually does. We should get some asserts in there and decide if we > like the results of what we find. If not, fix it. > > On Wed, Mar 15, 2023 at 4:49 PM David Smiley <dsmi...@apache.org> wrote: > > > Interestingly, the part of that test which you commented on doesn't > appear > > to assert/validate anything; it just does stuff. > > I suspect Gus may know the answer to your question. > > > > ~ David Smiley > > Apache Lucene/Solr Search Developer > > http://www.linkedin.com/in/davidwsmiley > > > > > > On Wed, Mar 15, 2023 at 2:13 PM Alex Deparvu <stilla...@apache.org> > wrote: > > > > > Hi, > > > > > > I am working on v2 apis for managing alias properties as part of > > > SOLR-16393. > > > > > > I have a confusion I would love to clarify with the community related > to > > an > > > existing test (AliasIntegrationTest) that deletes an alias property. > > > My understanding is that setting a property to 'empty string' will > > > effectively remove it. So what happens when a property is set to null? > > > > > > What is the expectation for the following? The test currently lacks any > > > assertions [0]. > > > var setAliasProperty = > > > CollectionAdminRequest.setAliasProperty(aliasName); > > > setAliasProperty.addProperty("bar", null); > > > > > > My feeling was that setting it to null will remove it, but that does > not > > > seem to be the case [1], it seems to not change it at all. > > > > > > best, > > > alex > > > > > > [0] > > > > > > > > > https://github.com/apache/solr/blob/a2a39fb136a9338f4603748bf446038cf4155296/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java#L301 > > > > > > [1] > > > > > > > > > https://github.com/apache/solr/pull/1459/files#diff-e4ea077299d4865546c5a8bdf8ba618d1dbb06fc01bf95eeacde1231ff11fcd7R331 > > > > > > > > -- > http://www.needhamsoftware.com (work) > http://www.the111shift.com (play) >
