Skip to content

Add field to change HTTP port #13479

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jul 11, 2025
Merged

Add field to change HTTP port #13479

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c7798cd
Add field to change HTTP port
mugiwara-tech Jul 5, 2025
1ce282e
Change URI to include the user's HTTP port
mugiwara-tech Jul 6, 2025
157c573
Merge branch 'main' into task-for-issue-13463
mugiwara-tech Jul 6, 2025
df53f2f
Add URI validator and validate before returning URI for HTTP server
mugiwara-tech Jul 7, 2025
59c2e07
Merge branch 'task-for-issue-13463' of https://github.com/dbolate/jab…
mugiwara-tech Jul 7, 2025
9f04c77
Merge branch 'JabRef:main' into task-for-issue-13463
mugiwara-tech Jul 7, 2025
47aeffd
Update JabRef_en.properties to resolve issues for updated text for en…
mugiwara-tech Jul 7, 2025
92a0887
Merge branch 'task-for-issue-13463' of https://github.com/dbolate/jab…
mugiwara-tech Jul 7, 2025
987ed00
Merge branch 'main' into task-for-issue-13463
koppor Jul 10, 2025
bb33695
Change isUriValid to only accept host address and port number
mugiwara-tech Jul 10, 2025
a569a88
Construct URI the same way
koppor Jul 11, 2025
6ca44e7
Remove obsolete method
koppor Jul 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions jabgui/src/main/java/org/jabref/gui/preferences/general/GeneralTab.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public class GeneralTab extends AbstractPreferenceTabView<GeneralTabViewModel> i
@FXML private CheckBox remoteServer;
@FXML private TextField remotePort;
@FXML private CheckBox enableHttpServer;
@FXML private TextField httpServerPort;
@FXML private Button remoteHelp;
@Inject private FileUpdateMonitor fileUpdateMonitor;
@Inject private BibEntryTypesManager entryTypesManager;
Expand Down Expand Up @@ -143,6 +144,7 @@ public void initialize() {

Platform.runLater(() -> {
validationVisualizer.initVisualization(viewModel.remotePortValidationStatus(), remotePort);
validationVisualizer.initVisualization(viewModel.httpPortValidationStatus(), httpServerPort);
validationVisualizer.initVisualization(viewModel.fontSizeValidationStatus(), fontSize);
validationVisualizer.initVisualization(viewModel.customPathToThemeValidationStatus(), customThemePath);
});
Expand All @@ -152,6 +154,8 @@ public void initialize() {
remotePort.disableProperty().bind(remoteServer.selectedProperty().not());

enableHttpServer.selectedProperty().bindBidirectional(viewModel.enableHttpServerProperty());
httpServerPort.textProperty().bindBidirectional(viewModel.httpPortProperty());
httpServerPort.disableProperty().bind(enableHttpServer.selectedProperty().not());
}

@FXML
Expand Down
33 changes: 32 additions & 1 deletion jabgui/src/main/java/org/jabref/gui/preferences/general/GeneralTabViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ public class GeneralTabViewModel implements PreferenceTabViewModel {
private final StringProperty remotePortProperty = new SimpleStringProperty("");
private final Validator remotePortValidator;
private final BooleanProperty enableHttpServerProperty = new SimpleBooleanProperty();
private final StringProperty httpPortProperty = new SimpleStringProperty("");
private final Validator httpPortValidator;
private final TrustStoreManager trustStoreManager;

private final FileUpdateMonitor fileUpdateMonitor;
Expand Down Expand Up @@ -160,14 +162,28 @@ public GeneralTabViewModel(DialogService dialogService, GuiPreferences preferenc
Localization.lang("Network"),
Localization.lang("Remote operation"),
Localization.lang("You must enter an integer value in the interval 1025-65535"))));

httpPortValidator = new FunctionBasedValidator<>(
httpPortProperty,
input -> {
try {
int portNumber = Integer.parseInt(httpPortProperty().getValue());
return RemoteUtil.isUserPort(portNumber);
} catch (NumberFormatException ex) {
return false;
}
},
ValidationMessage.error("%s".formatted(Localization.lang("You must enter an integer value in the interval 1025-65535"))));
this.trustStoreManager = new TrustStoreManager(Path.of(preferences.getSSLPreferences().getTruststorePath()));
}

public ValidationStatus remotePortValidationStatus() {
return remotePortValidator.getValidationStatus();
}

public ValidationStatus httpPortValidationStatus() {
return httpPortValidator.getValidationStatus();
}

@Override
public void setValues() {
selectedLanguageProperty.setValue(workspacePreferences.getLanguage());
Expand Down Expand Up @@ -210,6 +226,7 @@ public void setValues() {
remotePortProperty.setValue(String.valueOf(remotePreferences.getPort()));

enableHttpServerProperty.setValue(remotePreferences.enableHttpServer());
httpPortProperty.setValue(String.valueOf(remotePreferences.getHttpPort()));
}

@Override
Expand Down Expand Up @@ -276,6 +293,12 @@ public void storeSettings() {
remoteListenerServerManager.stop();
}

getPortAsInt(httpPortProperty.getValue()).ifPresent(newPort -> {
if (remotePreferences.isDifferentHttpPort(newPort)) {
remotePreferences.setHttpPort(newPort);
}
});

HttpServerManager httpServerManager = Injector.instantiateModelOrService(HttpServerManager.class);
// stop in all cases, because the port might have changed
httpServerManager.stop();
Expand Down Expand Up @@ -308,6 +331,10 @@ public boolean validateSettings() {
validator.addValidators(remotePortValidator);
}

if (enableHttpServerProperty.getValue()) {
validator.addValidators(httpPortValidator);
}

if (fontOverrideProperty.getValue()) {
validator.addValidators(fontSizeValidator);
}
Expand Down Expand Up @@ -442,6 +469,10 @@ public BooleanProperty enableHttpServerProperty() {
return enableHttpServerProperty;
}

public StringProperty httpPortProperty() {
return httpPortProperty;
}

public void openBrowser() {
String url = "https://themes.jabref.org";
try {
Expand Down
5 changes: 4 additions & 1 deletion jabgui/src/main/resources/org/jabref/gui/preferences/general/GeneralTab.fxml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@
</HBox>

<Label styleClass="sectionHeader" text="%HTTP Server" />
<CheckBox fx:id="enableHttpServer" text="%Enable HTTP Server (e.g., for JabMap)" />
<HBox alignment="CENTER_LEFT" spacing="10.0">
<CheckBox fx:id="enableHttpServer" text="%Enable HTTP Server (e.g., for JabMap)" />
Copy link
Member

@palukku palukku Jul 6, 2025

Choose a reason for hiding this comment

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

Maybe also add on port to the end of the text to clarify that the integer value refers to a port

Copy link
Contributor Author

@mugiwara-tech mugiwara-tech Jul 6, 2025

Choose a reason for hiding this comment

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

What about line 88? Would the id name httpServerPort be enough to clarify that's it's a port?

Copy link
Member

@palukku palukku Jul 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

But that information is not visible to the user, i mean the text that is being displayed to the user. So it would be better if it looked more like the one above for the single instance
image

Copy link
Contributor Author

@mugiwara-tech mugiwara-tech Jul 6, 2025

Choose a reason for hiding this comment

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

Oh! Okay, I understand now.

<TextField fx:id="httpServerPort" maxWidth="100.0" HBox.hgrow="ALWAYS" />
</HBox>

<Label styleClass="sectionHeader" text="%Libraries"/>
<GridPane hgap="10.0" vgap="4.0">
Expand Down
4 changes: 4 additions & 0 deletions jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ public class JabRefCliPreferences implements CliPreferences {
// Remote
private static final String USE_REMOTE_SERVER = "useRemoteServer";
private static final String REMOTE_SERVER_PORT = "remoteServerPort";
private static final String HTTP_SERVER_PORT = "httpServerPort";
private static final String ENABLE_HTTP_SERVER = "enableHttpServer";

private static final String AI_ENABLED = "aiEnabled";
Expand Down Expand Up @@ -639,6 +640,7 @@ public JabRefCliPreferences() {
defaults.put(USE_REMOTE_SERVER, Boolean.TRUE);
defaults.put(REMOTE_SERVER_PORT, 6050);
defaults.put(ENABLE_HTTP_SERVER, Boolean.FALSE);
defaults.put(HTTP_SERVER_PORT, 23119);

defaults.put(EXTERNAL_JOURNAL_LISTS, "");
defaults.put(USE_AMS_FJOURNAL, true);
Expand Down Expand Up @@ -1250,10 +1252,12 @@ public RemotePreferences getRemotePreferences() {
remotePreferences = new RemotePreferences(
getInt(REMOTE_SERVER_PORT),
getBoolean(USE_REMOTE_SERVER),
getInt(HTTP_SERVER_PORT),
getBoolean(ENABLE_HTTP_SERVER));

EasyBind.listen(remotePreferences.portProperty(), (_, _, newValue) -> putInt(REMOTE_SERVER_PORT, newValue));
EasyBind.listen(remotePreferences.useRemoteServerProperty(), (_, _, newValue) -> putBoolean(USE_REMOTE_SERVER, newValue));
EasyBind.listen(remotePreferences.httpPortProperty(), (_, _, newValue) -> putInt(HTTP_SERVER_PORT, newValue));
EasyBind.listen(remotePreferences.enableHttpServerProperty(), (_, _, newValue) -> putBoolean(ENABLE_HTTP_SERVER, newValue));

return remotePreferences;
Expand Down
22 changes: 20 additions & 2 deletions jablib/src/main/java/org/jabref/logic/remote/RemotePreferences.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ public class RemotePreferences {
private final IntegerProperty port;
private final BooleanProperty useRemoteServer;

private final IntegerProperty httpPort;
private final BooleanProperty enableHttpServer;

public RemotePreferences(int port, boolean useRemoteServer, boolean enableHttpServer) {
public RemotePreferences(int port, boolean useRemoteServer, int httpPort, boolean enableHttpServer) {
this.port = new SimpleIntegerProperty(port);
this.useRemoteServer = new SimpleBooleanProperty(useRemoteServer);
this.httpPort = new SimpleIntegerProperty(httpPort);
this.enableHttpServer = new SimpleBooleanProperty(enableHttpServer);
}

Expand Down Expand Up @@ -60,6 +62,22 @@ public void setUseRemoteServer(boolean useRemoteServer) {
this.useRemoteServer.setValue(useRemoteServer);
}

public int getHttpPort() {
return httpPort.getValue();
}

public IntegerProperty httpPortProperty() {
return httpPort;
}

public void setHttpPort(int httpPort) {
this.httpPort.setValue(httpPort);
}

public boolean isDifferentHttpPort(int otherHttpPort) {
return getHttpPort() != otherHttpPort;
}

public boolean enableHttpServer() {
return enableHttpServer.getValue();
}
Expand All @@ -79,7 +97,7 @@ public static InetAddress getIpAddress() throws UnknownHostException {

public @NonNull URI getHttpServerUri() {
try {
return new URI("http://" + RemotePreferences.getIpAddress().getHostAddress() + ":23119");
return new URI("http://" + RemotePreferences.getIpAddress().getHostAddress() + ":" + getHttpPort());
Copy link
Member

@koppor koppor Jul 6, 2025

Choose a reason for hiding this comment

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

Do you think, you are able to add another validator for checking that an URI can be constructed?

Extract this line into a new method. The validator calls it and if there is an exception, the host is invalid.

Copy link
Contributor Author

@mugiwara-tech mugiwara-tech Jul 6, 2025

Choose a reason for hiding this comment

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

I'll attempt to do that.

Copy link
Contributor Author

@mugiwara-tech mugiwara-tech Jul 6, 2025

Choose a reason for hiding this comment

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

Do you mean attempt to test an HTTP connection or just making sure the IP Address is in the correct format along with everything else?

Copy link
Member

@koppor koppor Jul 9, 2025

Choose a reason for hiding this comment

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

just ensuring that new URI( does not throw any exception - no self-implemented checks, just re-use waht's offered by JAva

Copy link
Contributor Author

@mugiwara-tech mugiwara-tech Jul 10, 2025

Choose a reason for hiding this comment

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

I'm confused since new URI throws the URISyntaxException - before my changes. I changed the code a few days ago to have a method isUriValid, but I'm not sure if it is necessary. Unfortunately, I believe I need some guidance on this.

Copy link
Member

@koppor koppor Jul 10, 2025

Choose a reason for hiding this comment

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

Maybe, its too complicated. Just add a validator. Don't use isValid here - just construct the URL

However, construct the URL in isValid and here THE SAME WAY.

THERE: With multiple parameters to URI constructor (which is better than the existing code)
HERE: Stirng concatination (existing code)

Copy link
Contributor Author

@mugiwara-tech mugiwara-tech Jul 10, 2025

Choose a reason for hiding this comment

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

So do not use isValid here - which I'm assuming is above the return statement on line 100 (or line 110 with recent push with my changes). But construct the URL in isValid and here the same way - confused by this.

However, what I believe is meant by there is the declaration of the isValid method - I changed this to only accept two params.

Based on my recent push, I put the validator above the returned URI - which includes the construction of the concatenated string in the param.

Just making sure I'm understanding this correctly.

} catch (UnknownHostException | URISyntaxException e) {
LOGGER.error("Could not create HTTP server URI. Falling back to default.", e);
try {
Expand Down
23 changes: 22 additions & 1 deletion jablib/src/test/java/org/jabref/logic/remote/RemotePreferencesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,31 @@ class RemotePreferencesTest {

@BeforeEach
void setUp() {
preferences = new RemotePreferences(1000, true, false);
preferences = new RemotePreferences(1000, true, 3000, false);
}

@Test
void getPort() {
assertEquals(1000, preferences.getPort());
}

@Test
void getHttpPort() {
assertEquals(3000, preferences.getHttpPort());
}

@Test
void setPort() {
preferences.setPort(2000);
assertEquals(2000, preferences.getPort());
}

@Test
void setHttpPort() {
preferences.setHttpPort(4000);
assertEquals(4000, preferences.getHttpPort());
}

@Test
void useRemoteServer() {
assertTrue(preferences.useRemoteServer());
Expand All @@ -43,8 +54,18 @@ void isDifferentPortTrue() {
assertTrue(preferences.isDifferentPort(2000));
}

@Test
void isDifferentHttpPortTrue() {
assertTrue(preferences.isDifferentHttpPort(4000));
Copy link

@trag-bot trag-bot bot Jul 11, 2025

Choose a reason for hiding this comment

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

Using assertTrue with a boolean condition instead of asserting the actual values. Should use assertEquals to compare the actual HTTP port values for better test clarity and error messages.

}

@Test
void isDifferentPortFalse() {
assertFalse(preferences.isDifferentPort(1000));
}

@Test
void isDifferentHttpPortFalse() {
assertFalse(preferences.isDifferentHttpPort(3000));
Copy link

@trag-bot trag-bot bot Jul 11, 2025

Choose a reason for hiding this comment

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

Using assertFalse with a boolean condition instead of asserting the actual values. Should use assertEquals to compare the actual HTTP port values for better test clarity and error messages.

}
}