gerlowskija commented on code in PR #4377: URL: https://github.com/apache/solr/pull/4377#discussion_r3214754974
########## solr/core/src/java/org/apache/solr/schema/SemVerField.java: ########## @@ -0,0 +1,268 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.schema; + +import java.util.Collection; +import org.apache.lucene.document.LongField; +import org.apache.lucene.document.LongPoint; +import org.apache.lucene.document.StoredField; +import org.apache.lucene.index.IndexableField; +import org.apache.lucene.queries.function.ValueSource; +import org.apache.lucene.queries.function.valuesource.LongFieldSource; +import org.apache.lucene.queries.function.valuesource.MultiValuedLongFieldSource; +import org.apache.lucene.search.MatchNoDocsQuery; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.SortedNumericSelector; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.BytesRefBuilder; +import org.apache.solr.common.SolrException; +import org.apache.solr.search.QParser; + +/** + * A field type for semantic versioning strings with up to 6 dot-separated parts (e.g. "2.3.1"). + * Each part must be in the range [0, 999]. + * + * <p>Internally stored as a {@code long} by encoding each part as: {@code part[i] * 1000^(5-i)}. + * For example, "2.3.1" encodes as {@code 2*1000^5 + 3*1000^4 + 1*1000^3 = 2_003_001_000_000}. + * + * <p>This encoding preserves version ordering, so range queries, sorting, and comparisons all work + * naturally. Missing trailing parts are treated as zero, so "2.3" and "2.3.0" are equivalent. + */ +public class SemVerField extends LongPointField { + + static final int MAX_PARTS = 6; + static final long PART_MULTIPLIER = 1000L; + static final int MAX_PART_VALUE = 999; + + static final long[] POWERS = new long[MAX_PARTS]; + + static { + POWERS[MAX_PARTS - 1] = 1L; + for (int i = MAX_PARTS - 2; i >= 0; i--) { + POWERS[i] = POWERS[i + 1] * PART_MULTIPLIER; + } + } + + public static String decodeDocValue(long value) { + return longToSemVer(value); + } + + static long parseSemVer(String semver) { + if (semver == null || semver.isEmpty()) { + throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Empty semver value"); + } + String[] parts = semver.split("\\.", -1); + if (parts.length < 1 || parts.length > MAX_PARTS) { + throw new SolrException( + SolrException.ErrorCode.BAD_REQUEST, + "Invalid semver '" + semver + "': must have 1 to " + MAX_PARTS + " dot-separated parts"); + } + long result = 0; + for (int i = 0; i < parts.length; i++) { + int val; + try { + val = Integer.parseInt(parts[i]); + } catch (NumberFormatException e) { + throw new SolrException( + SolrException.ErrorCode.BAD_REQUEST, + "Invalid semver part '" + parts[i] + "' in '" + semver + "'"); + } + if (val < 0 || val > MAX_PART_VALUE) { + throw new SolrException( + SolrException.ErrorCode.BAD_REQUEST, + "Semver part " + val + " out of range [0, " + MAX_PART_VALUE + "] in '" + semver + "'"); + } + result += val * POWERS[i]; + } + return result; + } + + static String longToSemVer(long value) { + if (value < 0) { + return Long.toString(value); + } + int[] parts = new int[MAX_PARTS]; + long remaining = value; + for (int i = 0; i < MAX_PARTS; i++) { + parts[i] = (int) (remaining / POWERS[i]); Review Comment: [0] Not sure how much this would matter in practice, but if the per-part limit was a power of 2, then we could do parsing and reconstitution with bit-shifting rather than the (relatively) more expensive multiplication and division. Might not be worth the hassle. It's really nice how the current base-10 setup is easy to scan in the tests haha 😛 ########## solr/core/src/test/org/apache/solr/schema/SemVerFieldTest.java: ########## @@ -0,0 +1,202 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.schema; + +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.common.SolrException; +import org.junit.BeforeClass; +import org.junit.Test; + +public class SemVerFieldTest extends SolrTestCaseJ4 { + + @BeforeClass + public static void beforeClass() throws Exception { + initCore("solrconfig-basic.xml", "schema-semver.xml"); + } + + @Test + public void testParseSemVer() { + assertEquals(0L, SemVerField.parseSemVer("0")); + assertEquals(1_000_000_000_000_000L, SemVerField.parseSemVer("1")); + assertEquals(2_003_001_000_000_000L, SemVerField.parseSemVer("2.3.1")); + assertEquals(1_002_003_004_005_006L, SemVerField.parseSemVer("1.2.3.4.5.6")); + assertEquals(999_999_999_999_999_999L, SemVerField.parseSemVer("999.999.999.999.999.999")); + assertEquals(SemVerField.parseSemVer("2.3"), SemVerField.parseSemVer("2.3.0")); + assertEquals(SemVerField.parseSemVer("2.3"), SemVerField.parseSemVer("2.3.0.0.0.0")); + } + + @Test + public void testLongToSemVer() { + assertEquals("0", SemVerField.longToSemVer(0L)); + assertEquals("1", SemVerField.longToSemVer(1_000_000_000_000_000L)); + assertEquals("2.3.1", SemVerField.longToSemVer(2_003_001_000_000_000L)); + assertEquals("1.2.3.4.5.6", SemVerField.longToSemVer(1_002_003_004_005_006L)); + assertEquals("0.0.0.0.0.1", SemVerField.longToSemVer(1L)); + } + + @Test + public void testRoundTrip() { + String[] versions = {"0", "1", "1.2.3", "999.999.999.999.999.999", "0.0.0.0.0.1"}; + for (String v : versions) { + long parsed = SemVerField.parseSemVer(v); + String formatted = SemVerField.longToSemVer(parsed); + assertEquals("Round trip for " + v, parsed, SemVerField.parseSemVer(formatted)); + } + } + + @Test + public void testParseSemVerErrors() { + assertThrows(SolrException.class, () -> SemVerField.parseSemVer("1.2.3.4.5.6.7")); + assertThrows(SolrException.class, () -> SemVerField.parseSemVer("")); Review Comment: [-0] Would be great to test the error messages here to make sure users are gonna get a decently helpful message. ########## solr/core/src/test/org/apache/solr/schema/SemVerFieldTest.java: ########## @@ -0,0 +1,202 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.schema; + +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.common.SolrException; +import org.junit.BeforeClass; +import org.junit.Test; + +public class SemVerFieldTest extends SolrTestCaseJ4 { + + @BeforeClass + public static void beforeClass() throws Exception { + initCore("solrconfig-basic.xml", "schema-semver.xml"); + } + + @Test + public void testParseSemVer() { + assertEquals(0L, SemVerField.parseSemVer("0")); + assertEquals(1_000_000_000_000_000L, SemVerField.parseSemVer("1")); + assertEquals(2_003_001_000_000_000L, SemVerField.parseSemVer("2.3.1")); + assertEquals(1_002_003_004_005_006L, SemVerField.parseSemVer("1.2.3.4.5.6")); + assertEquals(999_999_999_999_999_999L, SemVerField.parseSemVer("999.999.999.999.999.999")); + assertEquals(SemVerField.parseSemVer("2.3"), SemVerField.parseSemVer("2.3.0")); + assertEquals(SemVerField.parseSemVer("2.3"), SemVerField.parseSemVer("2.3.0.0.0.0")); + } + + @Test + public void testLongToSemVer() { + assertEquals("0", SemVerField.longToSemVer(0L)); + assertEquals("1", SemVerField.longToSemVer(1_000_000_000_000_000L)); + assertEquals("2.3.1", SemVerField.longToSemVer(2_003_001_000_000_000L)); + assertEquals("1.2.3.4.5.6", SemVerField.longToSemVer(1_002_003_004_005_006L)); + assertEquals("0.0.0.0.0.1", SemVerField.longToSemVer(1L)); + } + + @Test + public void testRoundTrip() { + String[] versions = {"0", "1", "1.2.3", "999.999.999.999.999.999", "0.0.0.0.0.1"}; + for (String v : versions) { + long parsed = SemVerField.parseSemVer(v); + String formatted = SemVerField.longToSemVer(parsed); + assertEquals("Round trip for " + v, parsed, SemVerField.parseSemVer(formatted)); + } + } + + @Test + public void testParseSemVerErrors() { + assertThrows(SolrException.class, () -> SemVerField.parseSemVer("1.2.3.4.5.6.7")); + assertThrows(SolrException.class, () -> SemVerField.parseSemVer("")); + assertThrows(SolrException.class, () -> SemVerField.parseSemVer("abc")); + assertThrows(SolrException.class, () -> SemVerField.parseSemVer("1.2.abc")); + assertThrows(SolrException.class, () -> SemVerField.parseSemVer("1000.0.0")); + assertThrows(SolrException.class, () -> SemVerField.parseSemVer("-1.0.0")); + assertThrows(SolrException.class, () -> SemVerField.parseSemVer("1.2.")); + } + + @Test + public void testIndexAndExactQuery() { + clearIndex(); + assertU(adoc("id", "1", "version", "1.2.3")); + assertU(adoc("id", "2", "version", "2.0.0")); + assertU(adoc("id", "3", "version", "1.2.3")); + assertU(commit()); + + assertQ("Exact match should find 2 docs", req("q", "version:1.2.3"), "//*[@numFound='2']"); + + assertQ( + "Exact match single doc", + req("q", "version:2.0.0"), + "//*[@numFound='1']", + "//result/doc[1]/str[@name='id'][.='2']"); + + assertQ("Trailing zeros are equivalent", req("q", "version:2.0"), "//*[@numFound='1']"); + } + + @Test + public void testRangeQuery() { + clearIndex(); Review Comment: [-0] Maybe put 'clearIndex' in an `@Before` so that any editors down the road don't have to remember to put it their own test methods? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
