[ 
https://issues.apache.org/jira/browse/SOLR-8778?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15176777#comment-15176777
 ] 

Steve Rowe commented on SOLR-8778:
----------------------------------

I don't think it's useful to have live setters at all on a CSVStrategy instance 
- they should be removed.

I'll make a patch that:
* deprecates the setters (to be removed in 7.0, sigh)
* switches the ctors to not call the setters
* switches all internal Solr usages to treat CSVStrategy instances as if they 
were immutable (i.e., don't call any setters)
* adds a deprecated private CSVStrategy subclass that throws an error from all 
setters, to be used for the pre-defined static strategies

> CSVStrategy's pre-defined static strategies should be immutable
> ---------------------------------------------------------------
>
>                 Key: SOLR-8778
>                 URL: https://issues.apache.org/jira/browse/SOLR-8778
>             Project: Solr
>          Issue Type: Bug
>            Reporter: Steve Rowe
>
> Removing some deprecated things in CSVStrategy (SOLR-8764) exposed a bug: 
> it's possible to redefine the public static 
> {{CSVStrategy.\{DEFAULT,EXCEL,TDF}_STRATEGY}} strategies, simply by calling 
> their setters.
> Right now that's happening in {{CSVParserTest.testUnicodeEscape()}}, where 
> the default unicode escape interpretation is changed from false to true.  And 
> then if that test happens to run before 
> {{CSVStrategyTest.testSetCSVStrategy()}}, which tests that the unicode escape 
> interpretation on the default strategy is set to false, then the latter will 
> fail.
> Example failures: 
> http://jenkins.thetaphi.de/job/Lucene-Solr-trunk-Linux/16079/ and 
> http://jenkins.thetaphi.de/job/Lucene-Solr-trunk-MacOSX/3126/



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to